Permit Trace+Debug logging in all non-SGX builds #5375
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
verbose_runtime_option@72660 aka 20230707.5 vs main ewma over 20 builds from 72383 to 72651 Click to see tablemain
verbose_runtime_option
|
eddyashton
added
auto-backport
|
@eddyashton I am worried about bundling this in with items of configuration such as directories, IPs, domain names and ports, which may be node-specific. Even if the security policy correctly captures the configuration (which isn't a given, since existing implementations typically do not allow measuring mounts, except for the empty special case), it will differ by node, and effectively require that each node addition be preceded by governance to add the corresponding measurement. It seems to me that the requirements for this bit of configuration are that:
A generic solution would be to create an attested configuration, which must come from somewhere that's measured (ie. the container right now, I don't believe there are other practical options). That's still relatively easy to get wrong. A less generic, but low impact solution, is to make this a CLI argument (either directly, or add a CLI enclave log-level cap). CLI arguments are well captured by the security policy, it is difficult to get that wrong. |
|
@achamayou I've updated this PR, and the option is now passed on the CLI rather than in the JSON configuration file. Please re-review. |
We should also take the opportunity to |
achamayou
reviewed
Jul 6, 2023
achamayou
approved these changes
Jul 6, 2023
|
@achamayou I've renamed to separate the CMake option and the preprocessor definition. CMake's |
…to verbose_runtime_option
|
@eddyashton a changelog entry is probably worthwhile for this change. |
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
achamayou
added
auto-backport
ghost
pushed a commit
that referenced
this pull request
Jul 7, 2023
(cherry picked from commit cd33041)
ghost
mentioned this pull request
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We previously (#2404, and related follow-ups) made it so that logging verbosity was controlled at compile time. This is necessary for SGX, but not for other platforms. This PR loosens that behaviour, restoring a run-time
logging.enclave_levelconfiguration option that allows the enclave's verbosity to be controlled at launch. We still restrict the options which are available (and which are compiled into the binary), but only on SGX. On other platforms, we think it is safe to have a single build, which contains debug logging, and reliably disable that at launch (in an attested config).This gives a few big wins for Virtual and SNP:
--enclave-log-leveltosandbox.shto get verbose enclave logging (previously only available in thesgx_unsafeartifact).VERBOSE_LOGGINGon these platforms doesn't change the compile definitions, only the args passed to e2e tests. So you can quickly enable verbose logging to get more debug information, without a slow rebuild.To avoid too many annoying changes/duplicated checks, I've retained the original definition names. This means there's potential confusion - there's aVERBOSE_LOGGINGCMake option and aVERBOSE_LOGGINGC++ preprocessor definition, but the latter is always-present unless we're on SGX. It may be worth renaming the latter toUNSAFE_LOGGINGor something to avoid that confusion.VERBOSE_LOGGINGis the CMake option, which also affects the runtime arg used by the e2e tests. If we're building for SGX and don't haveVERBOSE_LOGGING, we add the compile definitionCCF_DISABLE_VERBOSE_LOGGING, which is the inverse of the previously preproc defVERBOSE_LOGGINGETA - This option is now passed as a CLI arg to
cchost, where it should be covered by attestation, rather than in a potentially-unattested config file