rcgen icon indicating copy to clipboard operation
rcgen copied to clipboard

KeyPair::from_pem detects incorrect algorithm?

Open brocaar opened this issue 2 years ago • 6 comments

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?

brocaar avatar Dec 04 '23 15:12 brocaar

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

est31 avatar Dec 04 '23 16:12 est31

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...

brocaar avatar Dec 04 '23 17:12 brocaar

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

est31 avatar Dec 04 '23 21:12 est31

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 avatar Dec 04 '23 23:12 brocaar

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.

brocaar avatar Dec 05 '23 09:12 brocaar

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.

est31 avatar Dec 05 '23 09:12 est31