Skip to content

feat: add fips aligned security policy without CBC #5265

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

Open
wants to merge 38 commits into
base: main
Choose a base branch
from

Conversation

johubertj
Copy link
Contributor

@johubertj johubertj commented Apr 22, 2025

Release Summary:

  • Created an additional security policy (fips compliant) security_policy_20250416 that removes CBC cipher suites.

Description of changes:

  • Removed CBC (Cipher Block Chaining) cipher suites due to their security weaknesses of being vulnerable to Padding Oracle Attacks.

  • Uses secure AEAD ciphers (immune to Padding Oracle Attacks), since AEAD ciphers don't have padding

Testing:

  • Added security policy 20250416 to the list of policies being tested in s2n_security_policies_test.c

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 Apr 22, 2025
@johubertj johubertj changed the title added new security policy w/ new cipher_suites feat: update default fibs policy Apr 22, 2025
@johubertj johubertj self-assigned this Apr 22, 2025
@johubertj johubertj requested a review from jmayclin April 23, 2025 00:03
@lrstewart lrstewart changed the title feat: update default fibs policy feat: update default fips policy May 7, 2025
@johubertj johubertj marked this pull request as ready for review May 7, 2025 23:41
@johubertj johubertj requested a review from lrstewart May 7, 2025 23:41
@lrstewart lrstewart added the do_not_merge PR might needs something before merging, even if approved and passing label May 8, 2025
Copy link
Contributor

@lrstewart lrstewart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use the "policy" utility in ./bin to generate a before and after report of the default_fips policy? A diff between the before and after would also help verify this change is correct.

Also, do you need to update the security policy documentation under ./docs?

@lrstewart
Copy link
Contributor

lrstewart commented May 8, 2025

Oh, and this PR definitely needs a Release Notes section. And I'm also not sure why it doesn't have a Testing section.

@johubertj johubertj changed the title feat: update default fips policy feat: add fips aligned security policy without CBC May 13, 2025
@johubertj johubertj requested a review from lrstewart May 14, 2025 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do_not_merge PR might needs something before merging, even if approved and passing s2n-core team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants