-
Notifications
You must be signed in to change notification settings - Fork 744
feat: output utility for security policy #5502
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
base: main
Are you sure you want to change the base?
Conversation
tls/policy/s2n_policy_builder.c
Outdated
/* TODO: Currently outputs the base_policy. This needs to be updated once builder implements a "finalized" policy field */ | ||
const char *policy_name = "unknown"; | ||
for (size_t i = 0; security_policy_selection[i].version != NULL; i++) { | ||
if (security_policy_selection[i].security_policy == builder->base_policy) { | ||
policy_name = security_policy_selection[i].version; | ||
break; | ||
} | ||
} |
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 goal here isn't to print existing, hard-coded policies (except to share code with bin/policy.c). The goal is to print the newly constructed policies, to see the results of the builder. That means you cannot rely on security_policy_selection. The new policy won't be in security_policy_selection.
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.
Ah right. If so I don't think the name
field will be useful. I removed the field in the next revision, and updated the snapshots alongside.
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.
Probably out of scope for this format, but I wonder if builder policies should still have a name that's generated with a hash of the contents or something. That way it'd be easy to determine if two separate policies built with the builder are the same or not. 🤔
Release Summary:
Resolved issues:
Description of changes:
Adds three new utility functions for outputting security policy information:
s2n_security_policy_write_length()
: Get the exact buffer size neededs2n_security_policy_write_bytes()
: Write policy data to a user buffers2n_security_policy_write_fd()
: Write policy data to a file descriptorWe define a
S2N_POLICY_FORMAT_DEBUG_V1
format that replaces the policy snapshot utility implementation. Future formats can be added asV2
,V3
, etc.The
name
field was removed from output to ensure compatibility with custom security policies from policy builder.Call-outs:
I ran the following command to regenerate all the snapshots and verify that the new policy output utility is working:
As a result, all snapshots were updated.
Testing:
Added unit tests for happy paths as well as negative paths. We have snapshot test that checks for exact content match.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.