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

Support elliptic-curve keys #100

Closed
tl-alex-butler opened this issue Oct 12, 2021 · 14 comments · Fixed by #110 or #219
Closed

Support elliptic-curve keys #100

tl-alex-butler opened this issue Oct 12, 2021 · 14 comments · Fixed by #110 or #219
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@tl-alex-butler
Copy link

It would be great to have support for -----BEGIN EC PRIVATE KEY----- keys.

They currently do not work.

const EXAMPLE_KEY: &str = "-----BEGIN EC PRIVATE KEY-----\n\
    MIHcAgEBBEIBhqphIGu2PmlcEb6xADhhSCpgPUulB0s4L2qOgolRgaBx4fNgINFE\n\
    mBsSyHJncsWG8WFEuUzAYy/YKz2lP0Qx6Z2gBwYFK4EEACOhgYkDgYYABABwBevJ\n\
    w/+Xh6I98ruzoTX3MNTsbgnc+glenJRCbEJkjbJrObFhbfgqP52r1lAy2RxuShGi\n\
    NYJJzNPT6vR1abS32QFtvTH7YbYa6OWk9dtGNY/cYxgx1nQyhUuofdW7qbbfu/Ww\n\
    TP2oFsPXRAavZCh4AbWUn8bAHmzNRyuJonQBKlQlVQ==\n\
    -----END EC PRIVATE KEY-----\n\
    ";
PrivateKey::from_pem_str(EXAMPLE_KEY).expect("ec pems should work");
thread panicked at 'ec pems should work: InvalidPemLabel { label: "EC PRIVATE KEY" }'
@CBenoit CBenoit added enhancement New feature or request help wanted Extra attention is needed labels Oct 21, 2021
@CBenoit CBenoit changed the title Support EC PRIVATE KEY pems Support elliptic-curve keys Oct 21, 2021
@CBenoit
Copy link
Member

CBenoit commented Oct 21, 2021

Thank you for the report.

picky::key::PrivateKey is rejecting this PEM because it simply doesn't support elliptic curves yet. This can be seen in the PrivateKeyValue enum.

I understand you don't just want to decode PEM's content, but also use the elliptic-curve keys for actual signatures, so I changed issue's title and assigned enhancement label.

Elliptic curves are on my wish list, but I currently don't have the bandwidth to work on that. Of course, PRs are welcome until I find the time (help wanted).

Have a good day!

@RRRadicalEdward
Copy link
Collaborator

@CBenoit I will do it in my spare time

@CBenoit
Copy link
Member

CBenoit commented Nov 2, 2021

@CBenoit I will do it in my spare time

Thank you!

@CBenoit
Copy link
Member

CBenoit commented Dec 16, 2021

The sentence This PR partially closes #100. closed this issue automatically. It’s not yet fully supported (cryptographic operations are missing) so I re-open.

@CBenoit
Copy link
Member

CBenoit commented Mar 4, 2022

#132 added support for ECDSA p256 and p384

@dpogorzelski
Copy link
Contributor

Hey :)
I think a PrivateKey::generate_ec to compliment the existing PrivateKey::generate_rsa would be handy :)

@dpogorzelski
Copy link
Contributor

dpogorzelski commented May 5, 2022

It seems like Csr::generate will fail if supplied key is EC (generated via ring). Example:

let algo = &ring::signature::ECDSA_P256_SHA256_ASN1_SIGNING;
let rng = ring::rand::SystemRandom::new();

let pkcs8_bytes = ring::signature::EcdsaKeyPair::generate_pkcs8(&algo, &rng).unwrap();
let leaf_key = PrivateKey::from_pkcs8(&pkcs8_bytes).unwrap();

let csr = Csr::generate(
    DirectoryName::new_common_name(namespace.as_str()),
    &leaf_key,
    SignatureAlgorithm::Ecdsa(HashAlgorithm::SHA2_256),
)
.unwrap();

signature error: Key error: EC error: EC keypair cannot be built when curve type is not provided by private key

@dpogorzelski
Copy link
Contributor

dpogorzelski commented May 6, 2022

Just for the record I was able to repro this and find a reason for it. Find the attached code snippet
On the left side an EC key generated by the ring crate, on the right one generated by openssl.
Seems like picky expects the curve info to be present in the private key block although this info is always present in the private_key_algorithm block.

image

@flavio any specific reason for this?

@dpogorzelski
Copy link
Contributor

dpogorzelski commented May 6, 2022

@dpogorzelski
Copy link
Contributor

dpogorzelski commented May 9, 2022

I did some additional testing and it seems like openssl is unable to recognize the public key in the signed leaf certificate due to:

Subject Public Key Info:
            Public Key Algorithm: id-ecPublicKey
            Unable to load Public Key
8045FC0201000000:error:03000072:digital envelope routines:X509_PUBKEY_get0:decode error:crypto/x509/x_pubkey.c:458:
8045FC0201000000:error:03000072:digital envelope routines:X509_PUBKEY_get0:decode error:crypto/x509/x_pubkey.c:458
140459232052896:error:100AE081:elliptic curve routines:EC_GROUP_new_by_curve_name:unknown group:ec_curve.c:2075:
140459232052896:error:100D7010:elliptic curve routines:ECKEY_PUB_DECODE:EC lib:ec_ameth.c:206:
140459232052896:error:0B07707D:x509 certificate routines:X509_PUBKEY_get:public key decode error:x_pubkey.c:164:

this is unrelated to the merge request above as it happens also in 7.0.0-rc.2, keys generated via openssl and ring both present the same problem.

@CBenoit
Copy link
Member

CBenoit commented May 9, 2022

For any leaf certificate you built with picky (and ring for generating the key pair), you get this issue? The openssl error is not entirely clear to me, but I would guess the EC public key encoding might be wrong somewhere
Could you give me the commands used to get this error, please?

@dpogorzelski
Copy link
Contributor

dpogorzelski commented May 10, 2022

For any leaf certificate you built with picky (and ring for generating the key pair), you get this issue?

yes, i tried to use keys generated by openssl too and the outcome was the same

The openssl error is not entirely clear to me, but I would guess the EC public key encoding might be wrong somewhere Could you give me the commands used to get this error, please?

openssl x509 -in certfile.cert -text -noout

You can simulate correct ec based cert generation with the following bash scripts if you need a working reference:

generate_root_ca.sh:

#!/bin/bash
set -e

openssl ecparam -name prime256v1 -genkey -noout -out root_ca.key
openssl req -new -x509 -key root_ca.key -out root_ca.crt -days 10958 -config <(
    cat <<-EOF
[req]
distinguished_name = req_distinguished_name
default_md = sha256
req_extensions = v3_req
prompt = no

[req_distinguished_name]
O = foobar
CN = foobar

[v3_req]
subjectKeyIdentifier=hash
basicConstraints=critical,CA:TRUE
keyUsage=critical,keyCertSign,cRLSign
EOF
) -extensions v3_req

generate_leaf_cert.sh:

#!/bin/bash
set -e

openssl ecparam -name prime256v1 -genkey -noout -out leaf.key
openssl req -new -key leaf.key -out leaf.csr -config <(
    cat <<-EOF
[req]
distinguished_name = req_distinguished_name
req_extensions = v3_req
prompt = no

[req_distinguished_name]
O = foobar
CN = Dummy leaf

[v3_req]
keyUsage = keyEncipherment, dataEncipherment
extendedKeyUsage = serverAuth
EOF
)

openssl x509 -req -in leaf.csr -CA root_ca.crt -CAkey root_ca.key -out leaf.crt -days 1 -sha256 -CAcreateserial -extensions v3_req -extfile <(
    cat <<-EOF
[req]
distinguished_name = req_distinguished_name
req_extensions = v3_req
prompt = no

[req_distinguished_name]
O = foobar
CN = Dummy leaf

[v3_req]
keyUsage = keyEncipherment, dataEncipherment
extendedKeyUsage = serverAuth
EOF
)

Just run:

./generate_root_ca.sh
./generate_leaf_cert.sh
openssl x509 -in leaf.crt -text -noout

PS: just for the record prime256v1 and SECP256R1 are the same thing

@CBenoit
Copy link
Member

CBenoit commented May 10, 2022

Thank you for the detailed steps, this helps 👍

@damooo
Copy link

damooo commented Mar 30, 2023

#132 added support for ECDSA p256 and p384

Hello @CBenoit , currently JwsAlg still doesn't allow ec algorithms even for signing. It would be helpfull if atleast signing is supported through ec algs.

impl TryFrom<SignatureAlgorithm> for JwsAlg {
type Error = SignatureError;
fn try_from(v: SignatureAlgorithm) -> Result<Self, Self::Error> {
match v {
SignatureAlgorithm::RsaPkcs1v15(HashAlgorithm::SHA2_256) => Ok(Self::RS256),
SignatureAlgorithm::RsaPkcs1v15(HashAlgorithm::SHA2_384) => Ok(Self::RS384),
SignatureAlgorithm::RsaPkcs1v15(HashAlgorithm::SHA2_512) => Ok(Self::RS512),
unsupported => Err(SignatureError::UnsupportedAlgorithm {
algorithm: format!("{:?}", unsupported),
}),
}
}
}
impl TryFrom<JwsAlg> for SignatureAlgorithm {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
7 participants
@dpogorzelski @CBenoit @pacmancoder @damooo @RRRadicalEdward @tl-alex-butler and others