cryptography icon indicating copy to clipboard operation
cryptography copied to clipboard

Allow explicit elliptic curve parameters

Open smlu opened this issue 3 years ago • 15 comments

For the purpose of verifying MRTD (biometric passport) trust-chain, I'm using cryptography to verify signatures of CSCA and DSC x509 certificates issued by some countries . These certificates don't use named curve EC parameters but use explicit parameters to define the curve. At present state the cryptography library fails to initialize EC public key with explicit EC parameters. I think the library would benefit to also cover this case.

Through some testing I've noticed that the only thing it'd have to be done for the above case to work is move _mark_asn1_named_ec_curve 2 lines above, just before calling _ec_key_curve_sn function in _EllipticCurvePublicKey.__init__. This change successfully extracts curve name from explicit parameters. Probably the same could be done for _EllipticCurvePrivateKey.__init__.

Here's my patch: pymrtd

For anyone wanting to test this you can find some of the CSCA/DSC certificates in the official ICAO PKD repository here: https://download.pkd.icao.int. To test the above patch you can use my script pkdext.py to extract and verify CSCA/DSC certificates from ldif files.

Duplicates #4135, #5105

smlu avatar Dec 19 '20 12:12 smlu

Unfortunately accessing the certificates from ICAO requires agreeing to a terms & conditions that, to my non-lawyer, read is incompatible with an open source license, so we can't use them (or a derived work of them!) in a test case. For that reason I'm not accessing them.

Notwithstanding that issue. There are two ways to use explicit curve parameters:

  1. Explicitly specify values that are equivilant to a named curve
  2. To generate ad-hoc curves

(1) is silly, and implementing it correctly is error prone. (2) is wholly unacceptable and generates security risk. It's not possible for me to tell which use case ICAO has due to the licensing issues.

So long as I have a vote in this project, we will never implement (2). (2) puts EC on the path towards parameter complexity that DSA/FFDH suffer from, and which makes them very easy to misuse in protocols. And that's ignoring the part where supporting arbitrary curves is impossible to do in an optimized and constant-time fashion.

(1) I am also strongly opposed to, but am open to being persuaded on. My concern is that (a) it's easy to mis-implement (I believe OpenSSL historically has had bugs in their implementation where they'd use a non-optimized/non-constant-time implementation of the curve), (b) there's no point.

Why does ICAO use explicit curve parameters? Have you considered asking them to use named curves like literally everybody else does?

alex avatar Dec 19 '20 16:12 alex

Hey @alex, for test cases you could also use German PKI (CSCA, master list): https://www.bsi.bund.de/EN/Topics/ElectrIDDocuments/securPKI/securCSCA/Root_Certificate/cscaGermany_node.html. Quick glance over web page doesn't reveal any special ToS you would have to accept in order to use their PKI. You can use above mentioned pkdext.py to extract CSCAs from master list (.ml).

Another way is also to generate self signed x509 certificates with explicit EC parameters, for example using OpenSSL:

openssl ecparam -name prime256v1 -genkey -param_enc explicit -out p256_key.pem 
openssl req -x509 -sha256 -key p256_key.pem -out explicit_p256_params.cer

I've just tested such certificate with the patch I listed above and it works.
For the possible solution I'd go with what OpenSSL offers as it's currently being used in the library as the primary backend. I don't see at the moment if allowing explicit parameters by marking them as named curve (via _mark_asn1_named_ec_curve) would pose any serious threat. In any case I believe the call to _sn_to_elliptic_curve should prevent any non-standard curve to be used.

Why does ICAO use explicit curve parameters? Have you considered asking them to use named curves like literally everybody else does?

The biometric passport standard ICAO 9303 part 12 section 4.4.3 Elliptic Curve DSA (ECDSA) requires the countries to explicitly define EC parameters in their certificates and prohibits using named curves or implicit parameters. I'm guessing the reason for this could be that any country can define their own ECC standard and verification systems are able to process such custom curves. Note that the standard is 5 years old. It's possible that no one at the time had thought of CVE-2020-0601 :)

smlu avatar Dec 19 '20 19:12 smlu

Hi there,

Just wondering any update on this issue please. I'm getting the same error by initialize EC public key with unnamed curves.

ifindthanh avatar Apr 27 '22 03:04 ifindthanh

No updates -- supporting this remains controversial because custom curves have proven to be a vector for numerous security issues in EC implementations and are generally considered a design mistake at this point. Unfortunately they exist in a few places, but to get support in cryptography will likely require one of those users stepping up and working with us to determine an acceptable API that minimizes risk to typical named curve users.

reaperhulk avatar Apr 27 '22 14:04 reaperhulk

@alex, is there a way to go from explicit parameters to a curve name they represent in such keys? If so, why couldn't we handle such parametrized keys by transforming them back to named-curve's ones and processing from there in usual way?

psvz avatar Oct 20 '22 16:10 psvz

@psvz OpenSSL can do this mapping in recent versions (I believe 3.0.x only), but older versions do not. We explicitly disable it (https://github.com/pyca/cryptography/blob/b70f16e9d8f15b05624fd1cfca29d44141f6aa9b/src/cryptography/hazmat/backends/openssl/ec.py#L45-L54) at the moment.

Do you have a specific use case where explicitly encoding named curve values occurs?

reaperhulk avatar Oct 20 '22 17:10 reaperhulk

@reaperhulk - thanks for quick reply. I think compatibility use case offered by openssl wiki last few para is good enough. I do not see security implications provided the match to a valid curve is established - I'd be grateful for any evidence to the contrary.

I would think a good library should strive for generality; and reading the exchange above I do not quite see how the pushback is substantiated. ICAO is a long-standing maintainer of PKI for biometric passports around the globe. I totally support their requirement for parametrized keys - just because unnecessary abstraction layer in a form of curve naming is just that - unnecessary layer. Not to mention that challenging state bureaucracies around the globe is a bit... counterproductive?

psvz avatar Oct 20 '22 19:10 psvz

I needed functionality like what's being asked about here in my AsyncSSH package (https://github.com/ronf/asyncssh/blob/develop/asyncssh/crypto/ec_params.py), since SSH identifies curves by a string or OID, and not explicit EC parameters.

That module provides a function called lookup_ec_curve_by_params() which takes the params (p, a, b, point, and n) and returns an EC curve name if there's a match to a named curve, or raises ValueError otherwise. Right now, it only lists 4 EC curves, since those are the only ones I've seen supported by other SSH implementations, but it would be very easy to add new values at any time.

This approach has worked well for me, in rare cases where someone passes in a key with explicit EC params. It gets converted to a named curve when AsyncSSH loads the key, and the key is used from there as if it always referenced one of the supported named curves.

ronf avatar Oct 20 '22 23:10 ronf

@reaperhulk and I reached consensus here: We are willing to support explicit curve parameters in the cases where they actually match a known named curve.

However, because we do not have confidence in OpenSSL, we're only willing to do this once we migrate to our own parser for keys. Once we have that, we can move forward.

alex avatar Nov 01 '23 17:11 alex

For the folks who had originally requested this, do you need explicit curve support when loading public keys, or private keys?

alex avatar Jan 24 '24 12:01 alex

I'd primarily need explicit curve support for public keys (certificate trustchain verification).

smlu avatar Jan 24 '24 13:01 smlu

For PKCS#8, I need this for both public & private keys. For PKCS#1, I couldn't find support in other tools for public EC keys (either named curve or explicit parameters), so I only ended up needing it for private keys there.

See the link above for the function I implemented for this.

ronf avatar Jan 24 '24 13:01 ronf

Ok. We're at the point where we can look into implementing it for public keys, but not yet private keys.

alex avatar Jan 25 '24 01:01 alex

Do you know which key formats you'd be looking to support? Explicit parameters wouldn't apply to keys already in SSH format, so I think the main use case would be PKCS#8 DER and PEM. If you later add private keys, it would be good to support both PKCS#1 and PKCS#8 (both DER and PEM).

ronf avatar Jan 25 '24 04:01 ronf

We'd be supporting the same formats we do now.

On Wed, Jan 24, 2024 at 11:52 PM Ron Frederick @.***> wrote:

Do you know which key formats you'd be looking to support? Explicit parameters wouldn't apply to keys already in SSH format, so I think the main use case would be PKCS#8 DER and PEM. If you later add private keys, it would be good to support both PKCS#1 and PKCS#8 (both DER and PEM).

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

-- All that is necessary for evil to succeed is for good people to do nothing.

alex avatar Jan 25 '24 11:01 alex