mikkyang / rust-jwt

JSON Web Token library for Rust
MIT License
176 stars 38 forks source link

.sign() does not always produce a valid signature for PKeyWithDigest #88

Open fwojtan opened 1 year ago

fwojtan commented 1 year ago

Hi,

I've discovered that some of my tests for a project occasionally fail due to the JWTs they are using having an invalid signature. I think I've tracked down the bug to some inconsistent behaviour in the sign() implementation for PKeyWithDigest<Private>.

Versions rust-jwt 0.16.0 rust-openssl 0.10.45 (With OpenSSL 1.1.f installed on WSL2)

Steps to reproduce Using the following script with an EC private key produces an 'Invalid Signature' error in the first few hundred iterations, where I had expected it to complete after printing sigok 1000 times.

use std::{fs, path::Path};

use jwt::{PKeyWithDigest, RegisteredClaims, SignWithKey, Token, Unverified, VerifyWithKey};
use openssl::{hash::MessageDigest, pkey::PKey, x509::X509};

fn main() {
    let crate_root = Path::new(env!("CARGO_MANIFEST_DIR"));

    for _ in 1..1000 {
        let signing_crt =
            fs::read_to_string(crate_root.join("signing.crt")).unwrap();
        let signing_key = PKeyWithDigest {
            digest: MessageDigest::sha256(),
            key: PKey::private_key_from_pem(
                fs::read_to_string(crate_root.join("signing.key"))
                    .unwrap()
                    .as_bytes(),
            )
            .unwrap(),
        };

        let raw_token = Token::new(
            jwt::Header {
                algorithm: jwt::AlgorithmType::Es256,
                type_: None,
                key_id: None,
                content_type: None,
            },
            jwt::Claims::new(RegisteredClaims {
                issuer: Some("test".to_string()),
                subject: Some("test".to_string()),
                audience: Some("test".to_string()),
                expiration: None,
                not_before: None,
                issued_at: None,
                json_web_token_id: Some("test".to_string()),
            }),
        )
        .sign_with_key(&signing_key)
        .unwrap();

        let raw_token_str = raw_token.as_str();

        let jwt: Token<jwt::Header, jwt::Claims, Unverified> =
            Token::parse_unverified(raw_token_str).unwrap();

        let public_key = PKeyWithDigest {
            digest: MessageDigest::sha256(),
            key: X509::stack_from_pem(signing_crt.as_bytes())
                .unwrap()
                .first()
                .unwrap()
                .public_key()
                .unwrap(),
        };

        let result = jwt.verify_with_key(&public_key);

        if let Err(e) = result {
            println!("{:?}", e);
            return;
        }

        print!("sigok.. ");
    }
}

Impact Every so often (very rough guess of ~50-200 calls on average) a call to .sign_with_key(&key) produces an invalid signature which later fails verification.

Suspected cause der_to_jose in openssl.rs produces the signature as a Vec of bytes by calling rust-openssl's .to_vec() on the .r() and .s() parts of the signature. As far as I understand, the r and s parts of the signature require a fixed length in bytes, but I think it's possible that the .to_vec() call is dropping any leading zeros - it calls down to num_bytes and then num_bits, which says it only returns the number of significant bits.

I don't have enough Rust or OpenSSL context to be confident providing a fix, but I suspect to_vec_padded or copying into a fixed length array would work.

Please let me know if there's any other details that would be helpful! Thanks!