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

x509-cert: Error decoding CSR signature bytes #1041

Closed
jstayton opened this issue May 4, 2023 · 16 comments
Closed

x509-cert: Error decoding CSR signature bytes #1041

jstayton opened this issue May 4, 2023 · 16 comments
Assignees

Comments

@jstayton
Copy link

jstayton commented May 4, 2023

Hey 👋🏻

I'm using the new RequestBuilder from #1034 (/cc @baloo), and when I submit the CSR to my CA/RA, it says "error decoding signature bytes".

Here's the CSR:

-----BEGIN CERTIFICATE REQUEST-----
MIIBKzCB2QIBADBbMRAwDgYDVQQDDAdUZXN0IENOMRAwDgYDVQQLDAdUZXN0IE9V
MQ8wDQYDVQQKDAZUZXN0IE8xCzAJBgNVBAYTAlVTMRcwFQYKCZImiZPyLGQBAQwH
dGVzdDEyMzBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABG35X3bvpU597hzLMuzp
pzA2/Winaetja2HLawhRBA3QY9+hJPNDe5ZZp31nOCobHvvXoDo33BpME+7RU6jS
CJCgHDAaBgkqhkiG9w0BCQcxDRMLcGFzc3dvcmQxMjMwCgYIKoZIzj0EAwIDQQBW
dW5pVyO/PPDICokwFmkAxOIN2+SS0S/oXqIzAE9tiTU9VYM6baduD4uMwbWTKX1U
4ypmFSwiNpqTNxfHrZaM
-----END CERTIFICATE REQUEST-----
Certificate Request:
    Data:
        Version: 0 (0x0)
        Subject: CN=Test CN, OU=Test OU, O=Test O, C=US/UID=test123
        Subject Public Key Info:
            Public Key Algorithm: id-ecPublicKey
                Public-Key: (256 bit)
                pub:
                    04:6d:f9:5f:76:ef:a5:4e:7d:ee:1c:cb:32:ec:e9:
                    a7:30:36:fd:68:a7:69:eb:63:6b:61:cb:6b:08:51:
                    04:0d:d0:63:df:a1:24:f3:43:7b:96:59:a7:7d:67:
                    38:2a:1b:1e:fb:d7:a0:3a:37:dc:1a:4c:13:ee:d1:
                    53:a8:d2:08:90
                ASN1 OID: prime256v1
                NIST CURVE: P-256
        Attributes:
            challengePassword        :password123
    Signature Algorithm: ecdsa-with-SHA256
         56:75:6e:69:57:23:bf:3c:f0:c8:0a:89:30:16:69:00:c4:e2:
         0d:db:e4:92:d1:2f:e8:5e:a2:33:00:4f:6d:89:35:3d:55:83:
         3a:6d:a7:6e:0f:8b:8c:c1:b5:93:29:7d:54:e3:2a:66:15:2c:
         22:36:9a:93:37:17:c7:ad:96:8c

For comparison, here's the exact same CSR generated by another package/language, which my CA/RA accepts just fine:

-----BEGIN CERTIFICATE REQUEST-----
MIIBMjCB2QIBADBbMRAwDgYDVQQDEwdUZXN0IENOMRAwDgYDVQQLEwdUZXN0IE9V
MQ8wDQYDVQQKEwZUZXN0IE8xCzAJBgNVBAYTAlVTMRcwFQYKCZImiZPyLGQBARMH
dGVzdDEyMzBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABIKpiOJCyfVpfOfIs8zS
aAoolrKOwUJtjxjoSrm56sRdURBqiK2Lpa34Y6+2eF0+cy1/W2wuojhP2rK1QtMx
8zGgHDAaBgkqhkiG9w0BCQcxDRMLcGFzc3dvcmQxMjMwCgYIKoZIzj0EAwIDSAAw
RQIgXn+YbSEJMFDLCHqTKdNbPRyE1XAywUh67i7XQ8ljiy4CIQDqoIgsJkzRNFYA
9AwXniUpUymXo9GqgwCQDRGsRTlnlA==
-----END CERTIFICATE REQUEST-----
Certificate Request:
    Data:
        Version: 0 (0x0)
        Subject: CN=Test CN, OU=Test OU, O=Test O, C=US/UID=test123
        Subject Public Key Info:
            Public Key Algorithm: id-ecPublicKey
                Public-Key: (256 bit)
                pub:
                    04:82:a9:88:e2:42:c9:f5:69:7c:e7:c8:b3:cc:d2:
                    68:0a:28:96:b2:8e:c1:42:6d:8f:18:e8:4a:b9:b9:
                    ea:c4:5d:51:10:6a:88:ad:8b:a5:ad:f8:63:af:b6:
                    78:5d:3e:73:2d:7f:5b:6c:2e:a2:38:4f:da:b2:b5:
                    42:d3:31:f3:31
                ASN1 OID: prime256v1
                NIST CURVE: P-256
        Attributes:
            challengePassword        :password123
    Signature Algorithm: ecdsa-with-SHA256
         30:45:02:20:5e:7f:98:6d:21:09:30:50:cb:08:7a:93:29:d3:
         5b:3d:1c:84:d5:70:32:c1:48:7a:ee:2e:d7:43:c9:63:8b:2e:
         02:21:00:ea:a0:88:2c:26:4c:d1:34:56:00:f4:0c:17:9e:25:
         29:53:29:97:a3:d1:aa:83:00:90:0d:11:ac:45:39:67:94

The only thing noticeably different is that the second signature is a bit longer.

Any help is appreciated! Thanks.

@tarcieri
Copy link
Member

tarcieri commented May 4, 2023

The discrepancy seems to be a fixed-width non-ASN.1 signature (ecdsa::Signature) versus an ASN.1 DER-encoded signature (ecdsa::der::Signature).

Can you provide a code example? I think you just need to switch the signature type.

Also perhaps it's possible we impl'd the relevant traits incorrectly. I would hope this would be a compile error rather than generating an invalid certificate.

@jstayton
Copy link
Author

jstayton commented May 4, 2023

Yes, sorry, should have included that initially...

use p256::{
    ecdsa::{Signature, SigningKey},
    pkcs8::{LineEnding, ObjectIdentifier},
};
use rand_core::OsRng;
use std::error::Error;
use std::str::FromStr;
use x509_cert::{
    attr::Attribute,
    builder::{Builder, RequestBuilder},
    der::{self, asn1, EncodePem},
    name::Name,
};

fn main() -> Result<(), Box<dyn Error>> {
    let signing_key = SigningKey::random(&mut OsRng);

    let subject = Name::from_str("CN=Test CN,OU=Test OU,O=Test O,C=US,UID=test123")?;

    let builder = RequestBuilder::new(subject, &signing_key)?;

    let mut cert_req = builder.build::<Signature>()?;

    let mut values = asn1::SetOfVec::new();
    values.add(asn1::Any::new(
        der::Tag::PrintableString,
        "password123".as_bytes(),
    )?)?;

    let mut attributes = asn1::SetOfVec::new();
    attributes.add(Attribute {
        oid: ObjectIdentifier::new_unwrap("1.2.840.113549.1.9.7"),
        values,
    })?;

    cert_req.info.attributes = attributes;

    let cert_req_pem = cert_req.to_pem(LineEnding::LF)?;

    println!("{}", cert_req_pem);

    Ok(())
}

Are you saying I just need to change that p256::ecdsa::Signature to p256::ecdsa::DerSignature?

That gives me this CSR:

-----BEGIN CERTIFICATE REQUEST-----
MIIBMzCB2QIBADBbMRAwDgYDVQQDDAdUZXN0IENOMRAwDgYDVQQLDAdUZXN0IE9V
MQ8wDQYDVQQKDAZUZXN0IE8xCzAJBgNVBAYTAlVTMRcwFQYKCZImiZPyLGQBAQwH
dGVzdDEyMzBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABLHmVD/JyBl9vSQ5kSq7
S9gQuNi7fkZ7XXm30ojwcY9XVwsne0aTxp+4z2d0vPF0IRolzZP23A98dHOnGl5D
YlOgHDAaBgkqhkiG9w0BCQcxDRMLcGFzc3dvcmQxMjMwCgYIKoZIzj0EAwIDSQAw
RgIhAPxu+qxj3I9Snp3xYHxmxftmEVAEi237bsuzX/f/VyN9AiEAtr7FyZq6rpMG
mRw37+oAHzPADzY/afKqpca/UggHUUs=
-----END CERTIFICATE REQUEST-----
Certificate Request:
    Data:
        Version: 0 (0x0)
        Subject: CN=Test CN, OU=Test OU, O=Test O, C=US/UID=test123
        Subject Public Key Info:
            Public Key Algorithm: id-ecPublicKey
                Public-Key: (256 bit)
                pub:
                    04:b1:e6:54:3f:c9:c8:19:7d:bd:24:39:91:2a:bb:
                    4b:d8:10:b8:d8:bb:7e:46:7b:5d:79:b7:d2:88:f0:
                    71:8f:57:57:0b:27:7b:46:93:c6:9f:b8:cf:67:74:
                    bc:f1:74:21:1a:25:cd:93:f6:dc:0f:7c:74:73:a7:
                    1a:5e:43:62:53
                ASN1 OID: prime256v1
                NIST CURVE: P-256
        Attributes:
            challengePassword        :password123
    Signature Algorithm: ecdsa-with-SHA256
         30:46:02:21:00:fc:6e:fa:ac:63:dc:8f:52:9e:9d:f1:60:7c:
         66:c5:fb:66:11:50:04:8b:6d:fb:6e:cb:b3:5f:f7:ff:57:23:
         7d:02:21:00:b6:be:c5:c9:9a:ba:ae:93:06:99:1c:37:ef:ea:
         00:1f:33:c0:0f:36:3f:69:f2:aa:a5:c6:bf:52:08:07:51:4b

The signature seems to be a tad longer now. My CA/RA isn't giving the same "error decoding signature bytes", but it's still not liking it.

Thanks for your quick response!

@baloo
Copy link
Member

baloo commented May 4, 2023

$ openssl req -in /tmp/foo.csr  -verify -noout
Certificate request self-signature verify failure

Yeah, it's reproducible here. I'll have a look.

@baloo
Copy link
Member

baloo commented May 4, 2023

alright, so this will produce a correctly signed CSR:

use p256::{
    ecdsa::{DerSignature, SigningKey},
    pkcs8::{LineEnding, ObjectIdentifier},
};
use rand_core::OsRng;
use std::error::Error;
use std::str::FromStr;
use x509_cert::{
    attr::Attribute,
    builder::{Builder, RequestBuilder},
    der::{self, asn1, EncodePem},
    name::Name,
};

fn main() -> Result<(), Box<dyn Error>> {
    let signing_key = SigningKey::random(&mut OsRng);

    let subject = Name::from_str("CN=Test CN,OU=Test OU,O=Test O,C=US,UID=test123")?;

    let builder = RequestBuilder::new(subject, &signing_key)?;

    let mut cert_req = builder.build::<DerSignature>()?;

    //let mut values = asn1::SetOfVec::new();
    //values.add(asn1::Any::new(
    //    der::Tag::PrintableString,
    //    "password123".as_bytes(),
    //)?)?;

    //let mut attributes = asn1::SetOfVec::new();
    //attributes.add(Attribute {
    //    oid: ObjectIdentifier::new_unwrap("1.2.840.113549.1.9.7"),
    //    values,
    //})?;

    //cert_req.info.attributes = attributes;

    let cert_req_pem = cert_req.to_pem(LineEnding::LF)?;

    println!("{}", cert_req_pem);

    Ok(())
}

The reason it still fails with DerSignature is because you got a structure signed, then changed the structure.

If you want to add the password extension, you'll have to use the add_extension API of the builder before calling build (which signs the CSR).

I fixed the examples in #1043

@baloo
Copy link
Member

baloo commented May 4, 2023

If you want to add the password extension, you'll have to use the add_extension API of the builder before calling build (which signs the CSR).

Disregard that, this is not an x509v3 extension, we'll have to come up with something.

@jstayton
Copy link
Author

jstayton commented May 4, 2023

Ah, yeah, looking at the attributes you commented-out, that makes sense now. Thanks for taking a look!

Does it make sense to allow them to be passed into RequestBuilder::new, or have an add_attribute?

@baloo
Copy link
Member

baloo commented May 4, 2023

yeah, an add_attribute would be the deal, but it's needs to be stored in a temporary structure and reordered according to DerOrd in finalize().
Could probably use some helpers not to have to deal with low level structure (not have to deal with SetOfVec like you had).

I don't have time to do that right now, but I'd be happy to take a look in a couple weeks.

@baloo baloo self-assigned this May 4, 2023
@tarcieri
Copy link
Member

tarcieri commented May 4, 2023

@baloo you can build an intermediate Vec and then turn it into SetOfVec when you're ready to finalize.

Using this TryFrom impl will eagerly sort the Vec using DerOrd: https://docs.rs/der/latest/der/asn1/struct.SetOfVec.html#impl-TryFrom%3CVec%3CT,+Global%3E%3E-for-SetOfVec%3CT%3E

@baloo
Copy link
Member

baloo commented May 4, 2023

oh nice, I missed that!

@baloo
Copy link
Member

baloo commented May 11, 2023

@jstayton I just pushed a x509-cert v0.2.2-pre.1 that makes use of a SignatureBitStringEncoding trait as marker for signature encoding.
I'd love if you give it a shot!

@jstayton
Copy link
Author

Everything still works on my end with v0.2.2-pre.1. Just to confirm, that's unrelated to adding support for attributes, right?

@tarcieri
Copy link
Member

@jstayton that particular change should make using ecdsa::Signature instead of ecdsa::der::Signature a compile error

@baloo
Copy link
Member

baloo commented Jul 3, 2023

@jstayton I've pushed #1137 with what I had in mind. Sorry it took a little while longer than I thought. Any chance you could give it a shot?

tarcieri pushed a commit that referenced this issue Jul 10, 2023
@jstayton
Copy link
Author

Hey @baloo – Sorry for the delay in getting back to you. I just gave it a try and it worked great! Thanks for following through with that.

I'd be grateful for a new release with that when you get a chance. Thanks!

@baloo
Copy link
Member

baloo commented Jul 21, 2023

Awesome! Thank you.

I guess I could proceed with a 0.2.3 release, there are a couple pending fixup commits.

Is there anything missing in that issue btw? Or can I close it?

@jstayton
Copy link
Author

👍🏻 I think that's it here.

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

3 participants