rustls / rcgen

Generate X.509 certificates
Other
341 stars 105 forks source link

KeyPair::from_pem detects incorrect algorithm? #193

Open brocaar opened 11 months ago

brocaar commented 11 months ago

How to reproduce

Test certificates

Working certificates (-sha256 option)

openssl req -x509 \
                        -sha256 -days 356 \
                        -nodes \
                        -newkey rsa:4096 \
                        -subj "/CN=example.com" \
                        -keyout rootCA.key -out rootCA.crt

Failing certificates (-sha512 option)

openssl req -x509 \
                        -sha512 -days 356 \
                        -nodes \
                        -newkey rsa:4096 \
                        -subj "/CN=example.com" \
                        -keyout rootCA.key -out rootCA.crt

Code

use std::fs::read_to_string;

fn main() {
    let private_key = read_to_string("rootCA.key").unwrap();
    let private_key = rcgen::KeyPair::from_pem(&private_key).unwrap();
    println!("KeyPair alg: {:?}", private_key.algorithm());

    let cert = read_to_string("rootCA.crt").unwrap();
    let params = rcgen::CertificateParams::from_ca_cert_pem(&cert, private_key).unwrap();

    println!("Params alg: {:?}", params.alg);

    if let Err(e) = rcgen::Certificate::from_params(params) {
        println!("Error: {:#}", e);
    } else {
        println!("All good :-)")
    };
}

Test results

If using the -sha256 certificate files, the output is:

KeyPair alg: PKCS_RSA_SHA256
Params alg: PKCS_RSA_SHA256
All good :-)

If using the -sha512 certificate files, the output is:

KeyPair alg: PKCS_RSA_SHA256
Params alg: PKCS_RSA_SHA512
Error: The provided certificate's signature algorithm is incompatible with the given key pair

Is this expected?

est31 commented 11 months ago

Hmmm that's interesting. SHA-512 is supposed to be supported.

brocaar commented 11 months ago

Could this be coming from https://github.com/rustls/rcgen/blob/main/rcgen/src/key_pair.rs#L163?

if let Ok(rsakp) = RsaKeyPair::from_pkcs8(pkcs8) {
    (KeyPairKind::Rsa(rsakp, &signature::RSA_PKCS1_SHA256), &PKCS_RSA_SHA256)
}

I do not know the code-base well enough, but I read this as if RsaKeyPair::from_pkcs8(...) passes, it must be PKCS_RSA_SHA256 and I'm not sure if that is true...

est31 commented 11 months ago

mhh yeah good point, you have to use from_der_and_sign_algo instead in this instance.

brocaar commented 11 months ago

Wouldn't it be possible to auto-detect this?

On Mon, Dec 4, 2023, 21:41 est31 @.***> wrote:

mhh yeah good point, you have to use from_der_and_sign_algo instead in this instance.

— Reply to this email directly, view it on GitHub https://github.com/rustls/rcgen/issues/193#issuecomment-1839525638, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABIM6PKWDRBTIZWJBESE5DYHY7QZAVCNFSM6AAAAABAGGJSXWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZZGUZDKNRTHA . You are receiving this because you authored the thread.Message ID: @.***>

brocaar commented 11 months ago

My current work-around is:

use std::fs::read_to_string;

fn main() {
    let cert = read_to_string("rootCA.crt").unwrap();
    let private_key_s = read_to_string("rootCA.key").unwrap();

    let private_key = rcgen::KeyPair::from_pem(&private_key_s).unwrap();
    println!("KeyPair alg: {:?}", private_key.algorithm());

    let params = rcgen::CertificateParams::from_ca_cert_pem(&cert, private_key).unwrap();

    println!("Params alg: {:?}", params.alg);

    let private_key = rcgen::KeyPair::from_pem_and_sign_algo(&private_key_s, params.alg).unwrap();
    println!("KeyPair alg: {:?}", private_key.algorithm());

    let params = rcgen::CertificateParams::from_ca_cert_pem(&cert, private_key).unwrap();
    println!("Params alg: {:?}", params.alg);

    if let Err(e) = rcgen::Certificate::from_params(params) {
        println!("Error: {}", e);
    } else {
        println!("All good :-)")
    };
}

This outputs:

KeyPair alg: PKCS_RSA_SHA256
Params alg: PKCS_RSA_SHA512
KeyPair alg: PKCS_RSA_SHA512
Params alg: PKCS_RSA_SHA512
All good :-)

Hopefully the automatic algorithm detection can be fixed.

est31 commented 11 months ago

That's also a nice approach... I'm not sure ring's APIs allow auto-detection like the one we need, outside of starting trial encryptions/decryptions, which are time-intensive.