-
Notifications
You must be signed in to change notification settings - Fork 96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CVM: Updated traces to log by default, unless trace is tagged with CVM_CONFIDENTIAL #515
base: main
Are you sure you want to change the base?
CVM: Updated traces to log by default, unless trace is tagged with CVM_CONFIDENTIAL #515
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a lot of changes here in crates that aren't included as part of OpenHCL. Those should all be reverted, as they aren't going to exist in CVMs.
… for those marked as CVM_CONFIDENTIAL
4a5d652
to
c58accd
Compare
pub fn confidential_event_filter<S: Subscriber>() -> impl Filter<S> { | ||
FilterFn::new(move |m| m.fields().field("CVM_ALLOWED").is_some()) | ||
FilterFn::new(move |m| m.fields().field("CVM_CONFIDENTIAL").is_some().not()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely not. The design here was intentional. We can not allow un-audited tracing statements to be logged in a CVM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The auditing will be done during code review correct? Given that most of the traces are not confidential, it is easier to maintain if we change it to behave this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is all the tracing statements that already exist that may log sensitive or confidential information. The auditing needs to be opt-in on a per-statement level. I agree that this would be easier, but it is also an unacceptable level of risk to meet our CVM confidentiality guarantees.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filter inversion
No description provided.