Skip to content
Please note that GitHub no longer supports Internet Explorer.

We recommend upgrading to the latest Microsoft Edge, Google Chrome, or Firefox.

Learn more
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

Connection error when using EC client certificate with explicit parameters and TLS1.2 #7045

Closed
kleptog opened this issue Aug 23, 2018 · 10 comments
Closed

Comments

@kleptog
Copy link

@kleptog kleptog commented Aug 23, 2018

This is a regression, this worked fine in the OpenSSL 1.0 series, but appears broken in all OpenSSL 1.1 (including the current master).

Steps to reproduce:

openssl version
>>> OpenSSL 1.1.0f  25 May 2017
 
# Generate server key
openssl genrsa -out server.key
openssl req -new -key server.key -out server.csr -subj "/CN=server" -batch
openssl x509 -req -signkey server.key -in server.csr -out server.crt
 
# Generate client key
openssl ecparam -name secp256k1 -genkey -param_enc explicit -out ec_client.key
openssl req -new -key ec_client.key -out ec_client.csr -subj "/CN=client" -batch
openssl x509 -req -in ec_client.csr -signkey ec_client.key -out ec_client.crt
 
# Start server
openssl s_server -key server.key -cert server.crt -CAfile ec_client.crt -verify 1 &
 
# Start client (TLS1_2 fails)
openssl s_client -key ec_client.key -cert ec_client.crt -tls1_2
### OUTPUT ###
CONNECTED(00000003)
depth=0 CN = server
verify error:num=18:self signed certificate
verify return:1
depth=0 CN = server
verify return:1
depth=0 CN = client
verify return:1
ERROR
shutting down SSL
139974347023616:error:1409441A:SSL routines:ssl3_read_bytes:tlsv1 alert decode error:../ssl/record/rec_layer_s3.c:1399:SSL alert number 50
---
Certificate chain
 0 s:/CN=server
   i:/CN=server
---
Server certificate
CONNECTION CLOSED

Switching to TLS1.1 or 1.0 works just fine. Using named curve settings also makes it work.

When testing with master an extra error message is dropped:

139669262665088:error:1414D17A:SSL routines:tls12_check_peer_sigalg:wrong curve:ssl/t1_lib.c:1023:

This is the key it generated that fails:

Certificate:
    Data:
        Version: 1 (0x0)
        Serial Number:
            cf:e1:c6:62:e8:a7:9c:6f
    Signature Algorithm: ecdsa-with-SHA256
        Issuer: CN = client
        Validity
            Not Before: Aug 23 14:16:21 2018 GMT
            Not After : Sep 22 14:16:21 2018 GMT
        Subject: CN = client
        Subject Public Key Info:
            Public Key Algorithm: id-ecPublicKey
                Public-Key: (256 bit)
                pub:
                    04:d3:e8:a5:8d:16:26:e9:97:93:36:53:33:7c:4b:
                    b6:da:cc:ec:ab:58:f7:5e:5e:f9:d3:29:64:f0:cb:
                    76:6c:55:67:c7:53:ab:d0:b0:03:59:69:0f:29:38:
                    07:df:37:f9:8e:d4:2e:bd:16:e1:3a:6c:65:bc:27:
                    1b:86:77:15:17
                Field Type: prime-field
                Prime:
                    00:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:
                    ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:fe:ff:
                    ff:fc:2f
                A:    0
                B:    7 (0x7)
                Generator (uncompressed):
                    04:79:be:66:7e:f9:dc:bb:ac:55:a0:62:95:ce:87:
                    0b:07:02:9b:fc:db:2d:ce:28:d9:59:f2:81:5b:16:
                    f8:17:98:48:3a:da:77:26:a3:c4:65:5d:a4:fb:fc:
                    0e:11:08:a8:fd:17:b4:48:a6:85:54:19:9c:47:d0:
                    8f:fb:10:d4:b8
                Order: 
                    00:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:
                    ff:fe:ba:ae:dc:e6:af:48:a0:3b:bf:d2:5e:8c:d0:
                    36:41:41
                Cofactor:  1 (0x1)
    Signature Algorithm: ecdsa-with-SHA256
         30:46:02:21:00:de:f4:b5:f2:1a:e2:26:ec:f3:52:48:ee:c0:
         d2:b4:db:ae:72:d0:85:4b:1c:bb:51:77:2a:7b:11:60:c7:50:
         36:02:21:00:f0:cf:64:e2:ed:05:86:2a:ec:45:59:0c:41:93:
         32:8f:d8:d0:5e:16:d4:ed:0e:3d:9e:98:4f:60:d6:ca:12:68
-----BEGIN CERTIFICATE-----
MIIBsDCCAVUCCQDP4cZi6KecbzAKBggqhkjOPQQDAjARMQ8wDQYDVQQDDAZjbGll
bnQwHhcNMTgwODIzMTQxNjIxWhcNMTgwOTIyMTQxNjIxWjARMQ8wDQYDVQQDDAZj
bGllbnQwgfUwga4GByqGSM49AgEwgaICAQEwLAYHKoZIzj0BAQIhAP//////////
//////////////////////////7///wvMAYEAQAEAQcEQQR5vmZ++dy7rFWgYpXO
hwsHApv82y3OKNlZ8oFbFvgXmEg62ncmo8RlXaT7/A4RCKj9F7RIpoVUGZxH0I/7
ENS4AiEA/////////////////////rqu3OavSKA7v9JejNA2QUECAQEDQgAE0+il
jRYm6ZeTNlMzfEu22szsq1j3Xl750ylk8Mt2bFVnx1Or0LADWWkPKTgH3zf5jtQu
vRbhOmxlvCcbhncVFzAKBggqhkjOPQQDAgNJADBGAiEA3vS18hriJuzzUkjuwNK0
265y0IVLHLtRdyp7EWDHUDYCIQDwz2Ti7QWGKuxFWQxBkzKP2NBeFtTtDj2emE9g
1soSaA==
-----END CERTIFICATE-----
@davidben

This comment has been minimized.

Copy link
Contributor

@davidben davidben commented Aug 23, 2018

Explicit parameters are forbidden by RFC 5480, section 2.1.1.
https://tools.ietf.org/html/rfc5480#section-2.1.1

 o specifiedCurve, which is of type SpecifiedECDomain type (defined
   in [X9.62]), allows all of the elliptic curve domain parameters
   to be explicitly specified.  This choice MUST NOT be used.  See
   Section 5, "ASN.1 Considerations".

I know of no implementation other than OpenSSL that parses the non-compliant specifiedCurve form in public keys.

Although, OpenSSL does parse it, so I'm guessing instead it's tripping the logic to enforce the curve is one of the advertised ones. There, RFC 8422 does not include a provision for the client to advertise support for non-named curves so it's understandable that OpenSSL would reject them. The older RFC 4492 does have an arbitrary_explicit_prime_curves advertisement, but I don't believe OpenSSL ever supported those. Probably (though I don't know for sure off-hand) OpenSSL 1.0.x was missing a check that 1.1.x added.

All this explicit curve business was done when ECC was much younger and we were less sure how it would be used. Now we know better and the analogous problems with DHE negotiation in TLS demonstrate that arbitrary parameters were a clear mistake. (They are difficult to advertise and difficult to make performant.) Use the named encoding.

@mattcaswell

This comment has been minimized.

Copy link
Member

@mattcaswell mattcaswell commented Aug 23, 2018

Right - as @davidben says this isn't allowed by the RFCs. There was some half-hearted, broken, explicit curve support that exists in 1.0.2 but was removed by commit 2235b7f. That change brings OpenSSL in line with the standards and was an explicit decision. That I suppose is a breaking change, but 1.0 -> 1.1 is breaking anyway.

So, the up shot is - this won't be fixed.

@t-j-h

This comment has been minimized.

Copy link
Member

@t-j-h t-j-h commented Aug 23, 2018

Perhaps an FAQ entry would make sense for this?
Why don't curves in explicit parameter form work in 1.1+

Most of the body of the FAQ is in these comments.

@mattcaswell

This comment has been minimized.

Copy link
Member

@mattcaswell mattcaswell commented Aug 23, 2018

Perhaps an FAQ entry would make sense for this?

Maybe...but as far as I can recall this is the first question on this in the two years since the commit was made. The dodgy 1.0.2 explicit parameter support only ever applied to client certs anyway - it was unimplemented for server certs. Probably doesn't qualify as "frequent", for an FAQ!

@kleptog

This comment has been minimized.

Copy link
Author

@kleptog kleptog commented Aug 23, 2018

Ok, but in that case the "openssl ec -param_enc" option should be removed as well so you can't even generate such invalid keys/certificates.

Also, it does work, as long as you're only using TLS1.1 or lower. If you say that it shouldn't work, perhaps support should be ripped out entirely? That would make it more obvious.

It's kind annoying because we have several hundred certificates like this because prior to 1.1 "explicit parameters" was the default for EC certificate generation.

@mattcaswell

This comment has been minimized.

Copy link
Member

@mattcaswell mattcaswell commented Aug 23, 2018

Ok, but in that case the "openssl ec -param_enc" option should be removed as well so you can't even generate such invalid keys/certificates.

Well, the situation is a bit more murky at the ecparam level. That app supports X9.62 parameters - where explicit parameters are allowed. The IETF based standards are built on top of X9.62 but specify that you can't use the explicit parameters bit. So while the app allows you to create them, you're not supposed to use them in any standard related to IETF (i.e. RFCs). That doesn't preclude their use in other settings. I don't know if there are any such settings that people are using openssl for.

Also, it does work, as long as you're only using TLS1.1 or lower. If you say that it shouldn't work, perhaps support should be ripped out entirely? That would make it more obvious.

It doesn't work because of the TLSv1.2 signature algorithm checking which doesn't exist in TLSv1.1. and below. We could disable it in those protocols as well but, since it would be a breaking change, we can't do it until we move to OpenSSL 1.2.0 which is unscheduled at the moment. It's also probably a non-trivial change. Removing it from TLSv1.2 made sense because that was a simplifying change. I'm not sure how much enthusiasm there would be for adding some extra complexity into TLSv1.1 and below just for this.

@davidben

This comment has been minimized.

Copy link
Contributor

@davidben davidben commented Aug 23, 2018

To the 1.1. vs 1.2 business, supported_{curves,groups} exists prior to TLS 1.2 as well. If I've configured OpenSSL to only accept such-and-such curves (because, say, I consider the others to be insecure), I would probably want them to be rejected consistently at both versions.

@kleptog

This comment has been minimized.

Copy link
Author

@kleptog kleptog commented Aug 28, 2018

I wouldn't put too much stock in that it wasn't reported earlier. It's a difficult problem to track down, because the server produces no error message (fixed in 1.1.1) and the client merely says "TLS alert: decode error". The problem mysteriously vanishes when the key/certificates are recreated. There are enough vague TLS failure reports that it's possible people didn't realise this was the problem.

In any case, now it's clear it's not going to be fixed we are working on fixing all the key/certificates in the field. The attached script does the conversion, I hope it is useful for other people.

#!/usr/bin/python
#
# The issue is that code on OpenSSL 1.0 generated EC keys with explicit
# parameters and they are actually not legal, OpenSSL 1.1+ servers will
# reject them.  This script takes a key and fixes it (after verifying it is
# the expected curve).
#
# The key will work with existing certificates, but the certificate will
# also need to be regenerated to completely resolve the problem.  You only
# need this script if you want to preserve the key fingerprint.  (Otherwise
# you may as well generate a new key.)


import os
import sys

import OpenSSL

from cryptography.hazmat.backends.openssl import backend
from cryptography.hazmat.primitives.asymmetric import ec
from cryptography.hazmat.primitives import serialization

curve = ec.SECP256K1()


def verify_curve(group, private_key):
    def get_params(group):
        # Yes, it leaks
        ctx = backend._lib.BN_CTX_new()
        p = backend._lib.BN_new()
        a = backend._lib.BN_new()
        b = backend._lib.BN_new()

        assert backend._lib.EC_GROUP_get_curve_GFp(group, p, a, b, ctx) == 1

        return (backend._bn_to_int(p), backend._bn_to_int(a), backend._bn_to_int(b))

    key_group = OpenSSL.crypto._lib.EC_KEY_get0_group(
        OpenSSL.crypto._lib.EVP_PKEY_get1_EC_KEY(private_key._pkey))

    assert get_params(group) == get_params(key_group), \
        "%s != %s" % (get_params(group), get_params(key_group))


def convert_key(private_key):
    # Get the group of the curve we want
    group = OpenSSL.crypto._lib.EC_KEY_get0_group(
        OpenSSL.crypto._lib.EC_KEY_new_by_curve_name(
            backend._elliptic_curve_to_nid(curve)))

    verify_curve(group, private_key)

    # Set the group of the key we loaded
    OpenSSL.crypto._lib.EC_KEY_set_group(
        OpenSSL.crypto._lib.EVP_PKEY_get1_EC_KEY(private_key._pkey),
        group)

    # Now it has a group we can simply use cryptography to write out the right version
    serialized_private = private_key.to_cryptography_key().private_bytes(
        encoding=serialization.Encoding.PEM,
        format=serialization.PrivateFormat.PKCS8,
        encryption_algorithm=serialization.NoEncryption()
    )
    return serialized_private


if len(sys.argv) == 1:
    print "convert.py [filename]"
    print "Converts EC keys with explicit parameters to named parameters"
    sys.exit(1)

filename = sys.argv[1]

# Read private key using OpenSSL, because cryptography won't accept explicit parameters
private_key = OpenSSL.crypto.load_privatekey(OpenSSL.crypto.FILETYPE_PEM, open(filename).read())

try:
    # Check key
    private_key.to_cryptography_key()
except NotImplementedError:
    serialized_private = convert_key(private_key)
    open(filename + ".new", "w").write(serialized_private)
    print "Key updated"

    os.rename(filename, filename + ".old")
    os.rename(filename + ".new", filename)
@thierryba

This comment has been minimized.

Copy link

@thierryba thierryba commented Oct 14, 2019

I tried to generate a key with the SECP256K1 curve but for me it fails whatever I do. I just can't use it and it says "wrong curve". Is that curve really supported for TLS 1.2?

@richsalz

This comment has been minimized.

Copy link
Contributor

@richsalz richsalz commented Oct 14, 2019

Please don't use a closed issue to start a new thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.