-
Notifications
You must be signed in to change notification settings - Fork 717
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
Add new PQ TLS Policies #4327
Add new PQ TLS Policies #4327
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -166,6 +166,20 @@ S2N_RESULT s2n_security_rule_validate_policy(const struct s2n_security_rule *rul | |
"curve", curve->name, i + 1)); | ||
} | ||
|
||
const struct s2n_kem_preferences *kem_prefs = policy->kem_preferences; | ||
RESULT_ENSURE_REF(kem_prefs); | ||
for (size_t i = 0; i < kem_prefs->tls13_kem_group_count; i++) { | ||
const struct s2n_kem_group *kem_group = kem_prefs->tls13_kem_groups[i]; | ||
const struct s2n_ecc_named_curve *curve = kem_group->curve; | ||
Comment on lines
+172
to
+173
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are there any FIPS restrictions on the pq kems, or just the classical ones? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIST published new Key Derivation standards in 2020 that allow simple concatenation of a FIPS shared secret with a non-FIPS shared secret to still be considered allowed by FIPS, which is what the draft TLS 1.3 RFC does today. So no FIPS requirements apply to the PQ KEM's, just the FIPS ECDHE curves which are already FIPS approved.
From Section 2: https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-56Cr2.pdf
From section 3.3 of the draft Hybrid TLS 1.3 RFC: https://datatracker.ietf.org/doc/html/draft-ietf-tls-hybrid-design-09#section-3.3 |
||
RESULT_ENSURE_REF(curve); | ||
bool is_valid = false; | ||
RESULT_ENSURE_REF(rule->validate_curve); | ||
RESULT_GUARD(rule->validate_curve(curve, &is_valid)); | ||
RESULT_GUARD(s2n_security_rule_result_process(result, is_valid, | ||
error_msg_format_name, rule->name, policy_name, | ||
"curve", curve->name, i + 1)); | ||
} | ||
|
||
bool is_valid = false; | ||
RESULT_ENSURE_REF(rule->validate_version); | ||
RESULT_GUARD(rule->validate_version(policy->minimum_protocol_version, &is_valid)); | ||
|
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.
These RSA Key Exchange ciphers cause
PQ-TLS-1-2-2023-12-13
to fail theS2N_FIPS_140_3
rule checks in s2n as they exist today, but it seems that s2n's checks might be stricter than necessary.See this discussion about how RSA Key exchange ciphers are technically still allowed by FIPS if necessary, but no longer recommended and proposed a separate
S2N_FIPS_140_3_LEGACY_SUPPORT
rule.And this NIST doc that deprecates PKCSv1.5 padding.
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.
We didn't end up including the "LEGACY_SUPPORT" version of the rule because csosto-pk was right that the FIPS docs are pretty clear that RSA kex / PKCSv1.5 is not allowed after 2023, regardless of circumstances. So I don't think the rule is stricter than necessary for RSA kex.
It's 2024 now ;)