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

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

Open
fwojtan opened this issue Jan 9, 2023 · 0 comments
Open

Comments

@fwojtan
Copy link

fwojtan commented Jan 9, 2023

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!

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

1 participant