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

certificate_signature_preferences field contains unnecessary information #4435

Open
jmayclin opened this issue Feb 22, 2024 · 2 comments
Open

Comments

@jmayclin
Copy link
Contributor

Problem:

The certificate_signature_preferences field could be specific more succintly.

/* When this field is set, the endpoint will ensure that the signatures on
* the certificates in the peer's certificate chain are in the specified
* list. Note that s2n-tls does not support the signature_algorithms_cert
* extension. Unlike the signature_preferences field, this information is
* never transmitted to a peer.
*/
const struct s2n_signature_preferences *certificate_signature_preferences;

This field can contains things like s2n_ecdsa_secp256r1_sha256. However, having this included in the signature preferences might be confusing to readers because the secp256r1 part of the signature is never validated. Only the signature type (ECDSA) and the digest (SHA256) are validated.

This is also present when trying to distinguish between

    &s2n_rsa_pss_pss_sha512,
    &s2n_rsa_pss_rsae_sha512,

Given only a single certificate and it's signature, you can't tell whether the signing key had an OID of rsaEncryption or rsassaPss, so we can't distinguish between s2n_rsa_pss_pss_sha512 and s2n_rsa_pss_rsae_sha512.

Solution:

Some possible solutions

  1. define a new type to use for cert signature preferences that only include the signature and the digest.
  2. enforce that only TLS 1.2 style signatures are specified in the signature scheme
@lrstewart
Copy link
Contributor

lrstewart commented Feb 22, 2024

However, having this included in the signature preferences might be confusing to readers because the secp256r1 part of the signature is never validated.

Why is one of the possible solutions not to start enforcing the secp256rl part of the signature scheme?

Given only a single certificate and it's signature, you can't tell whether the signing key had an OID of rsaEncryption or rsassaPss, so we can't distinguish between s2n_rsa_pss_pss_sha512 and s2n_rsa_pss_rsae_sha512.

We have the full certificate chain though, don't we? Why can't we consider more than one certificate at a time?

@jmayclin
Copy link
Contributor Author

Why is one of the possible solutions not to start enforcing the secp256rl part of the signature scheme?

Given a goal of enforcing "key preferences", enforcing the key type in the IANA signature scheme would be necessary but not sufficient. IANA signature schemes are generally poorly suited to representing signature and key preferences because (nonexhaustively)

  1. They don't include size information about RSA keys certs
  2. They suffer from combinatoric qualities. For example there would be ~ 24 rsassaPss schemes.|rsa sizes| * |rsa OID types| * |digest sizes|

We have the full certificate chain though, don't we?

We do have the full certificate chain, but only after validation, and all of its requisite expensive crypto operations have completed. If we are going to reject a cert, we should do that as early as possible to avoid throwing away work. Implementing path-aware validation before X509_verify has completed would mean that s2n-tls has to figure out the potential path. While this is solvable, it's complexity that I would prefer not to deal with.

@goatgoose goatgoose added priority/medium Rank 3 and removed priority/high Rank 2 labels May 2, 2024
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

3 participants