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

sec1: can't decode secp256k1 pem #1116

Closed
ozwaldorf opened this issue Jun 24, 2023 · 4 comments
Closed

sec1: can't decode secp256k1 pem #1116

ozwaldorf opened this issue Jun 24, 2023 · 4 comments

Comments

@ozwaldorf
Copy link

ozwaldorf commented Jun 24, 2023

With a simple setup to generate a secp256k1 key pair, encode it to a pem, and then immediately decode the same pem and assert the secret keys are the same:

use fastcrypto::{
    secp256k1::{Secp256k1KeyPair, Secp256k1PublicKey},
    traits::KeyPair,
};
use pkcs8::{der::EncodePem, ObjectIdentifier};
use rand::rngs::ThreadRng;
use sec1::{DecodeEcPrivateKey, EcParameters, EcPrivateKey};

#[test]
fn encode_decode() {
    let pair = Secp256k1KeyPair::generate(&mut ThreadRng::default());
    let secret = &pair.private();
    let public: Secp256k1PublicKey = secret.into();

    let pem = EcPrivateKey {
        private_key: secret.as_ref(),
        parameters: Some(EcParameters::NamedCurve(
            ObjectIdentifier::new("1.3.132.0.10").unwrap(),
        )),
        public_key: Some(public.as_ref()),
    }
    .to_pem(pkcs8::LineEnding::LF)
    .unwrap();

    let doc = pkcs8::SecretDocument::from_sec1_pem(&pem).expect("decode sec1");
    let info: EcPrivateKey = doc.decode_msg().expect("decode der");

    assert_eq!(info.private_key, secret.as_ref());
}

It fails to decode the pem's der contents, which panics with the message:

'encode_decode' panicked at 'decode der: Error { kind: Value { tag: Tag(0x02: INTEGER) }, position: None }'

Manually testing via openssl ec, it can correctly decode the pem output, here's an example:

  • pem output:
-----BEGIN EC PRIVATE KEY-----
MFQCAQEEIDVZwPuscn3ruM06I2UJLs2CSbnvDvzCwQSzmPh+OgLRoAcGBSuBBAAK
oSQDIgADaai3wces5+EydeR55gNNUYIz+HZ/wOS8mnvcwwiKwYk=
-----END EC PRIVATE KEY-----
  • openssl ec output:
read EC key
Private-Key: (256 bit)
priv:
    35:59:c0:fb:ac:72:7d:eb:b8:cd:3a:23:65:09:2e:
    cd:82:49:b9:ef:0e:fc:c2:c1:04:b3:98:f8:7e:3a:
    02:d1
pub:
    03:69:a8:b7:c1:c7:ac:e7:e1:32:75:e4:79:e6:03:
    4d:51:82:33:f8:76:7f:c0:e4:bc:9a:7b:dc:c3:08:
    8a:c1:89
ASN1 OID: secp256k1

I'm not sure if this usage is correct, but couldn't find a test or example that decodes a sec1 pem file, and it was pretty poorly documented otherwise.

@tarcieri
Copy link
Member

tarcieri commented Jun 24, 2023

I'm not sure if this usage is correct

It's definitely not. Specifically this part:

let doc = pkcs8::SecretDocument::from_sec1_pem(&pem).expect("decode sec1");

I'm not sure how you managed to discover this compiles, but it's leveraging a default impl of SEC1 support for types which impl the PKCS#8 traits, i.e. you're mixing SEC1 and PKCS#8. SEC1 is described independently of PKCS#8, and PKCS#8 wraps it, with the blanket impl intended to provide PKCS#8 support to types which impl the sec1 traits.

Have a look at the elliptic-curve crate's decoder example: https://github.com/RustCrypto/traits/blob/master/elliptic-curve/src/secret_key.rs#L238-L244

Sorry this isn't better documented, but right now the elliptic-curve crate is the main downstream consumer, so it's going to be a bit difficult to use the crate without it. Have a look there for examples, however, it's all extensively tested there.

@tarcieri
Copy link
Member

tarcieri commented Jun 24, 2023

To describe this a little better, pkcs8::SecretDocument::from_sec1_pem converts a raw SEC1 private key into a PKCS#8-encoded private key by adding the PKCS#8 header and storing the serialized DER document.

The next thing you try to do is decode the PKCS#8-encoded key as a sec1::EcPrivateKey when you call doc.decode_msg(), and that's encountering a parse error because it's running into the PKCS#8 header.

If you really want to work directly with the SEC1 format, it'd be good to avoid using the pkcs8 crate entirely, and working entirely with the sec1 crate.

@ozwaldorf
Copy link
Author

ozwaldorf commented Jun 24, 2023

Thanks for the explanation, the error makes a lot more sense now. I had initially tried to just call EcPrivateKey::from_pem, as a reverse to the method to_pem, but it had failed to satisfy lifetime requirements for decode, and I was unable to use it. The downstream example is very useful though, and I should be able to use the same logic that does. Thanks!

@tarcieri
Copy link
Member

tarcieri commented Jun 24, 2023

Unfortunately EcPrivateKey has a lifetime and borrows from its input, so it's not able to use the on-the-fly PEM deserializer.

In other crates in this repo we've made types like EcPrivateKey generic with owned/ref aliases so you could use the on-the-fly deserializer on the owned type (which skips an intermediate Base64 decode to a Vec<u8> containing DER), but that didn't happen for sec1 unfortunately. I should open an issue. Edit: opened #1117

@tarcieri tarcieri closed this as completed Jul 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants