Parsing a PKCS#8 ECDSA signing keypair is confusing and inefficient

[briansmith]

First, in general people find the API confusing. We have not done a good job providing insight into the design of the API, and also the API is admittedly needs to be improved.

It is common for a user to want to support multiple curves and multiple digest algorithms when parsing a PKCS#8 document. The current API only allows the user to specify a single curve and a single digest algorithm when asking ring to parse a PKCS#8 document.

Another issue: To support multiple digest algorithms, some users, Rustls most notably, have resorted to awkward ways of using EcdsaKeyPair so that a single key pair can be used with multiple digest algorithms. They should be able to parse an EcdsaSigningKey once and then use it with multiple digest algorithms (but see below).

Accordingly, instead of only allowing the user to provide a single EcdsaSigningAlgorithm to indicate which algorithm combination is to be supported, we should let them provide multiple EcdsaSigningAlgorithms. And instead of Fixed vs. ASN1 in EcdsaSigningAlgorithm, we should instead allow the user to choose the format at signing time.

Further, when multiple provided EcdsaSigningAlgorithms have the same curve identifier, but different digest algorithms, a single EcdsaKeyPair should be returned, and that EcdsaKeyPair should store all the digest algorithms that are allowed to be used. I suggest we add a field to EcdsaKeyPair that is a bitmap, where nth bit is set iff the AlgorithmID of the digest algorithm has value n. At signing time, the user will pass in a digest algorithm (and the format Fixed vs ASN1) and the EcdsaKeyPair will refuse to sign with the given digest algorithm if it wasn't whitelisted at the time the EcdsaKeyPair was constructed.

As to the question of why EcdsaSigningAlgorithm should continue to identify both the curve and the digest algorithm, instead of having separate Curve and digest::Algorithm parameters: We need to be able to enforce the requirement "A hash function that provides a lower security strength than is associated with the bit length of n shall not be used" from FIPS-186-5. Note that ring may provide combinations of algorithms that do not meet FIPS/NIST requirements like this one, for compatibility with existing systems; these combinations should ideally be clearly-named as legacy-only.

For example, we need a way for the user to "I want to use P-256 with SHA-256, SHA-384, or SHA-512/256, and I want to use P-384 with SHA-384 but not SHA-256, but no other combinations." If the PKCS#8 document contains a P-256 key then the resulting EcdasKeyPair would allow sign() to be called with &digest::SHA256 or &digest::SHA384, but if the PKCS#8 document contains a P-384 key then calling sign with &digest::SHA256 should fail with an error.

In summary, we should:

• Replace all the ECDSA_{x}_{y}_FIXED_SIGNING with ECDSA_{x}_{y}_SIGNING, removing the signature format (Fixed VS ASN1). This is a breaking change.
• Add a variant of EcdsaKeyPair::sign that takes the digest algorithm and signature format as parameters. This one variant should verify that the digest algorithm was allowed at the time the key was constructed.
• Add a new PKCS#8 parsing API that allows multiple EcdsaSigningAlgorithms to be provided as explained above.
• Reimplement the existing APIs in terms of the new API. If this is hard to do, we may just drop the old APIs.
• Deprecate (perhaps in a separate follow-up PR) the existing key construction/parsing APIs.

This is a Semver-breaking change.

[andrewbaxter]

So if I understood that correctly, to summarize

• There are multiple combinations of parameters (curve + algorithm), some of which are safe and some of which aren't
• Right now the safe combinations are enumerated as ECDSA_* etc, along with usage method (fixed, asn1)
• Private keys only contain part of the parameters (curve) so the additional information is required to be supplied when deserializing (in this case the full set of parameters)
• The full parameters and private key data are stored together in the keypair so there's no chance of mix ups

So far that sounds reasonable to me.

The issues caused by pairing up the curve + algorithm are:

• The same key may be used with multiple algorithms which currently isn't straightforwardly possible due to the pairing
• Responsibilities are often split where the user loads the key and passes it to a library that uses the key, but in order to load the key the user needs to know what algorithm the library expects to use.

The idea proposed here is (roughly)

1. Keep the enumerated combinations
2. When parsing, the user presents a set of combinations. This could, for instance, be a default set like default_signing() or something?
3. Based on the curve in the key, the subset of presented combinations are stored with the key
4. When the key is used, an algorithm is presented, and that's checked against the stored list of combinations

You also mentioned splitting them, I assume like:

1. When parsing, no extra input. Curve is read and stored in key
2. When signing, the algorithm is presented and checked to see if it could be safely used with the curve

The disadvantage to this approach is that it's implicit in accepting combinations, there's no difference between using a FIPS compliant combination vs a non-compliant legacy combination, but the latter must still be supported due to existing uses.

The proposed solution seems smart to me (from my very outside view). My only concern is I wonder how much information a user needs in order to deserialize key pairs. If a default algorithm set could be used that's fine, but if they need to know more about how the key will be eventually used by a library in order to deserialize it it leaks library implementation details.

I think the pattern of "user gets pem, needs to deserialize it, then pass it to a library" is common. Right now rustls gets around it by just taking a pem (string?) and doing the deserialization itself, but if the cert needs to be reused it presents a less type safe interface since it pushes you to pass around untyped bytes (are those bytes from a pem? der? pcks8? sec1? etc).

Another option might be to have sign reject less secure combinations and a sign_allow_legacy or sign_dangerous for if you need those? It would shift responsibility to the library, but I'm not sure if that's reasonable in the typical contexts for needing legacy combinations.

[briansmith]

• There are multiple combinations of parameters (curve + algorithm), some of which are safe and some of which aren't

It isn't necessarily about safe vs. unsafe, but "allowed" vs. "not allowed," depending on which rules one is required to follow. In particular, when one is required to follow NIST/FIPS rules then certain combinations are not allowed for whatever reason.

When parsing, the user presents a set of combinations. This could, for instance, be a default set like default_signing() or something?

We do not provide things that would be like the default_signing() in ring currently. Basically, we expect the library to decide (probably by offering some configurability) which algorithms to support. Partly, this is to let the user choose the policies they are trying to conform too. Partly, this is a code size optimization, as any algorithms that the user doesn't provide to ring as an "allowed algorithm" will automatically be discarded at link time.

When parsing, no extra input. Curve is read and stored in key When signing, the algorithm is presented and checked to see if it could be safely used with the curve

Close. When parsing, one would supply what {curve,digest algorithm} combinations are allowed. When signing, ring would verify that the digest algorithm used was one of the ones that were mentioned being allowed at parsing time.

My only concern is I wonder how much information a user needs in order to deserialize key pairs. If a default algorithm set could be used that's fine, but if they need to know more about how the key will be eventually used by a library in order to deserialize it it leaks library implementation details.

In your library, you can maintain this "default algorithm set" yourself within your library, if you don't want the user to have to configure which algorithms are allowed. But usually if you are dealing with things like PKCS#8 you will need to have some user configuration about which algorithms are allowed, because different users have different policies they need to conform to.

[andrewbaxter]

Ah okay, makes sense. Thanks!