Skip to content

Conversation

jouho
Copy link
Contributor

@jouho jouho commented Sep 10, 2025

Release Summary:

Resolved issues:

Description of changes:

Adds three new utility functions for outputting security policy information:

  1. s2n_security_policy_write_length(): Get the exact buffer size needed
  2. s2n_security_policy_write_bytes(): Write policy data to a user buffer
  3. s2n_security_policy_write_fd() : Write policy data to a file descriptor

We define a S2N_POLICY_FORMAT_DEBUG_V1 format that replaces the policy snapshot utility implementation. Future formats can be added as V2, 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:

./tests/policy_snapshot/generate.sh build/bin/policy ./tests/policy_snapshot/snapshots

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.

@github-actions github-actions bot added the s2n-core team label Sep 10, 2025
@jouho jouho marked this pull request as ready for review September 11, 2025 22:55
Comment on lines 329 to 336
/* 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;
}
}
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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. 🤔

@jouho jouho changed the title feat: verbose output utility for policy builder feat: output utility for policy builder Sep 17, 2025
@jouho jouho changed the title feat: output utility for policy builder feat: output utility for security policy Sep 17, 2025
@jouho jouho requested a review from goatgoose September 17, 2025 18:33
@jouho jouho requested a review from lrstewart September 18, 2025 00:20
@jouho jouho requested a review from lrstewart September 18, 2025 21:45
@jouho jouho requested a review from goatgoose September 19, 2025 20:56
@jouho jouho requested a review from goatgoose September 19, 2025 22:46
@jouho jouho requested a review from lrstewart October 7, 2025 00:07
@jouho jouho requested a review from lrstewart October 7, 2025 23:04
@jouho jouho requested a review from goatgoose October 21, 2025 00:21
@jouho jouho requested a review from goatgoose October 21, 2025 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants