Skip to content
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

Runtime instead of compile time support check for curves #4515

Open
lrstewart opened this issue Apr 24, 2024 · 0 comments
Open

Runtime instead of compile time support check for curves #4515

lrstewart opened this issue Apr 24, 2024 · 0 comments

Comments

@lrstewart
Copy link
Contributor

Problem:

Our lists of ecc curves have to use #if EVP_APIS_SUPPORTED whenever they include s2n_ecc_curve_x25519. That's a lot of conditional compilation that shouldn't be necessary. See

const struct s2n_ecc_named_curve *const s2n_ecc_pref_list_20200310[] = {
#if EVP_APIS_SUPPORTED
&s2n_ecc_curve_x25519,
#endif
&s2n_ecc_curve_secp256r1,
&s2n_ecc_curve_secp384r1,
};
/* Curve p256 is at the top of the list in order to minimize HRR */
const struct s2n_ecc_named_curve *const s2n_ecc_pref_list_20230623[] = {
&s2n_ecc_curve_secp256r1,
#if EVP_APIS_SUPPORTED
&s2n_ecc_curve_x25519,
#endif
&s2n_ecc_curve_secp384r1,
};

Solution:

WillChilds-Klein did a similar fix for PQ in #4100.
The curves will need an is_supported method to check for runtime support. Then they can always be included on the lists, but skipped at runtime if not usable.

  • Does this change what S2N sends over the wire? If yes, explain.
  • Does this change any public APIs? If yes, explain.
  • Which versions of TLS will this impact?

Requirements / Acceptance Criteria:

What must a solution address in order to solve the problem? How do we know the solution is complete?

  • RFC links: Links to relevant RFC(s)
  • Related Issues: Link any relevant issues
  • Will the Usage Guide or other documentation need to be updated?
  • Testing: How will this change be tested? Call out new integration tests, functional tests, or particularly interesting/important unit tests.
    • Will this change trigger SAW changes? Changes to the state machine, the s2n_handshake_io code that controls state transitions, the DRBG, or the corking/uncorking logic could trigger SAW failures.
    • Should this change be fuzz tested? Will it handle untrusted input? Create a separate issue to track the fuzzing work.

Out of scope:

Is there anything the solution will intentionally NOT address?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants