RustCrypto / traits

Collection of cryptography-related traits
590 stars 191 forks source link

elliptic-curve: SEC1 private key export not loadable with OpenSSL #1706

Open czocher opened 1 month ago

czocher commented 1 month ago

Hello, I'm not sure if this is something I misunderstand or some other issue, but when I export a PEM of p256 to a file, openssl is not able to recognize the file when loading with:

openssl ec -in key.pem -pubout

This is how I generate the key:

use std::fs;

use p256::{
    elliptic_curve::{zeroize::Zeroizing, SecretKey},
    NistP256,
};

pub fn generate_jwt_private_signing_key() -> Zeroizing<String> {
    let mut random = rand::thread_rng();
    let key: SecretKey<NistP256> = SecretKey::random(&mut random);
    key.to_sec1_pem(p256::pkcs8::LineEnding::default()).unwrap()
}

fn main() {
    let key = generate_jwt_private_signing_key();
    fs::write("key.pem", key).unwrap()
}

The result of openssl ec -in key.pem -pubout:

read EC key
Could not find private key from key.pem
80C2396E037F0000:error:1608010C:STORE routines:ossl_store_handle_load_result:unsupported:crypto/store/store_result.c:151:
unable to load Key

On the other hand, if I use openssl to write the private key, the result is what is expected:

$ openssl ecparam -name prime256v1 -genkey -noout -out key.pem
$ openssl ec -in key.pem -pubout
read EC key
writing EC key
-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAELh80ZmtMD7DauXYY+HS7ngISWgvp
4Y2wwXk8XhHY/6m4lPfHv3mLRK2oCZeyFQyi5Gxhbmc+l6jDGXwy+mhC8w==
-----END PUBLIC KEY-----
tarcieri commented 1 month ago

It appears the curve OID (in context sensitive field 0) is missing from the key generated by to_sec1_pem.

Here is the OpenSSL-generated key:

$ openssl asn1parse -in key.pem
    0:d=0  hl=2 l= 119 cons: SEQUENCE
    2:d=1  hl=2 l=   1 prim: INTEGER           :01
    5:d=1  hl=2 l=  32 prim: OCTET STRING      [HEX DUMP]:E76BD09F83B1A7555A435C2069EE3BA1AB8982BE8DE2C56AC01F377693A1630D
   39:d=1  hl=2 l=  10 cons: cont [ 0 ]
   41:d=2  hl=2 l=   8 prim: OBJECT            :prime256v1
   51:d=1  hl=2 l=  68 cons: cont [ 1 ]
   53:d=2  hl=2 l=  66 prim: BIT STRING

Here is the elliptic_curve::SecretKey-generated one:

$ openssl asn1parse -in key.pem
    0:d=0  hl=2 l= 107 cons: SEQUENCE
    2:d=1  hl=2 l=   1 prim: INTEGER           :01
    5:d=1  hl=2 l=  32 prim: OCTET STRING      [HEX DUMP]:31B211BC55E02841DECAE770DFB18DAF364BEE678F168BC6C3A9DA54DC9381F5
   39:d=1  hl=2 l=  68 cons: cont [ 1 ]
   41:d=2  hl=2 l=  66 prim: BIT STRING
czocher commented 1 month ago

I understand that this is in fact a bug in the to_sec1_pem implementation?

tarcieri commented 1 month ago

Correct. I'm working on a fix. It's unfortunately a bit tricky to test as it's in code generic over curves, but requires a concrete curve implementation to test.

czocher commented 1 month ago

Another issue I see is that if this will be changed both was (i.e. to_sec1_pem as well as from_sec1_pem) and the same check that openssl does will be implemented in from_sec1_pem then it is in fact a breaking API change since all the previously exported keys will be considered invalid?

tarcieri commented 1 month ago

PR open: #1707

It's not technically a breaking change: before it didn't check the OID whatsoever, now it only checks it if present. Though it is a breaking API change as it relies on the AssociateOid trait, which the old version didn't bound on, but this is part of a breaking release cycle anyway.

It could be changed to always require the OID, although that would require some changes to the PKCS#8 parser which relies on the ability to handle an empty OID currently.

(Sidebar: I am surprised so many people are using SEC1 keys. They're a legacy format that has been replaced by PKCS#8)

tarcieri commented 1 month ago

I merged that PR, but I'll need to add some tests it works for a concrete curve to p256 to confirm this is closed.

I can try to figure out some stopgap tests in elliptic-curve itself too.

czocher commented 1 month ago

(Sidebar: I am surprised so many people are using SEC1 keys. They're a legacy format that has been replaced by PKCS#8)

Unfortunately that's due to various integration needs on my side ;).