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

Shouldn't be able to call sign_oneshot twice. #2225

Open
gaetan-sbt opened this issue Apr 24, 2024 · 0 comments
Open

Shouldn't be able to call sign_oneshot twice. #2225

gaetan-sbt opened this issue Apr 24, 2024 · 0 comments

Comments

@gaetan-sbt
Copy link

gaetan-sbt commented Apr 24, 2024

We shouldn't be able to call EVP_DigestSign twice with the same signer. Calling sign_oneshot twice fails with a panic with 'ED25519' (see test code below).

After openssl commit 3fc2b7d this should fail for all signing algorithms.

To prevent users from using sign_oneshot twice (which is always a mistake), we could change the sign_oneshot function to take ownership of signer. Since that breaks backward compatibilty we should probably consider deprecating sign_oneshot and replace it with a function that takes ownership.

More information about EVP_DigestSign is available on this openssl issue.

WDYT?


Failing test case

use openssl::{bn::BigNum, hash::MessageDigest, pkey::{self, PKey}, rsa::Rsa, sign::Signer};

fn test_issue_23075() {
    let key =
        BigNum::from_hex_str("f5e5767cf153319517630f226876b86c8160cc583bc013744c6bf255f5cc0ee5")
            .unwrap()
            .to_vec();
    let msg = BigNum::from_hex_str(
    "08b8b2b733424243760fe426a4b54908632110a66c2f6591eabd3345e3e4eb98fa6e264bf09efe12ee50f8f54e9f77b1e355f6c50544e23fb1433ddf73be84d879de7c0046dc4996d9e773f4bc9efe5738829adb26c81b37c93a1b270b20329d658675fc6ea534e0810a4432826bf58c941efb65d57a338bbd2e26640f89ffbc1a858efcb8550ee3a5e1998bd177e93a7363c344fe6b199ee5d02e82d522c4feba15452f80288a821a579116ec6dad2b3b310da903401aa62100ab5d1a36553e06203b33890cc9b832f79ef80560ccb9a39ce767967ed628c6ad573cb116dbefefd75499da96bd68a8a97b928a8bbc103b6621fcde2beca1231d206be6cd9ec7aff6f6c94fcd7204ed3455c68c83f4a41da4af2b74ef5c53f1d8ac70bdcb7ed185ce81bd84359d44254d95629e9855a94a7c1958d1f8ada5d0532ed8a5aa3fb2d17ba70eb6248e594e1a2297acbbb39d502f1a8c6eb6f1ce22b3de1a1f40cc24554119a831a9aad6079cad88425de6bde1a9187ebb6092cf67bf2b13fd65f27088d78b7e883c8759d2c4f5c65adb7553878ad575f9fad878e80a0c9ba63bcbcc2732e69485bbc9c90bfbd62481d9089beccf80cfe2df16a2cf65bd92dd597b0707e0917af48bbb75fed413d238f5555a7a569d80c3414a8d0859dc65a46128bab27af87a71314f318c782b23ebfe808b82b0ce26401d2e22f04d83d1255dc51addd3b75a2b1ae0784504df543af8969be3ea7082ff7fc9888c144da2af58429ec96031dbcad3dad9af0dcbaaaf268cb8fcffead94f3c7ca495e056a9b47acdb751fb73e666c6c655ade8297297d07ad1ba5e43f1bca32301651339e22904cc8c42f58c30c04aafdb038dda0847dd988dcda6f3bfd15c4b4c4525004aa06eeff8ca61783aacec57fb3d1f92b0fe2fd1a85f6724517b65e614ad6808d6f6ee34dff7310fdc82aebfd904b01e1dc54b2927094b2db68d6f903b68401adebf5a7e08d78ff4ef5d63653a65040cf9bfd4aca7984a74d37145986780fc0b16ac451649de6188a7dbdf191f64b5fc5e2ab47b57f7f7276cd419c17a3ca8e1b939ae49e488acba6b965610b5480109c8b17b80e1b7b750dfc7598d5d5011fd2dcc5600a32ef5b52a1ecc820e308aa342721aac0943bf6686b64b2579376504ccc493d97e6aed3fb0f9cd71a43dd497f01f17c0e2cb3797aa2a2f256656168e6c496afc5fb93246f6b1116398a346f1a641f3b041e989f7914f90cc2c7fff357876e506b50d334ba77c225bc307ba537152f3f1610e4eafe595f6d9d90d11faa933a15ef1369546868a7f3a45a96768d40fd9d03412c091c6315cf4fde7cb68606937380db2eaaa707b4c4185c32eddcdd306705e4dc1ffc872eeee475a64dfac86aba41c0618983f8741c5ef68d3a101e8a3b8cac60c905c15fc910840b94c00a0b9d0"
).unwrap().to_vec();
    let sig = BigNum::from_hex_str("0aab4c900501b3e24d7cdf4663326a3a87df5e4843b2cbdb67cbf6e460fec350aa5371b1508f9f4528ecea23c436d94b5e8fcd4f681e30a6ac00a9704a188a03").unwrap().to_vec();
    let pkey = openssl::pkey::PKey::private_key_from_raw_bytes(&key, pkey::Id::ED25519).unwrap();

    let mut signer = Signer::new_without_digest(&pkey).unwrap();
    assert_eq!(signer.sign_oneshot_to_vec(&msg).unwrap(), sig);

    assert_eq!(signer.sign_oneshot_to_vec(&msg).unwrap(), sig);
}

fn main() {
    test_issue_23075();
}

Here is the Cargo.toml:

[package]
name = "test-openssl"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
openssl = { version = "=0.10.64", features = ["vendored"] }
openssl-sys = "0.9.102"

Error message:

thread 'main' panicked at src/main.rs:17:49:
called `Result::unwrap()` on an `Err` value: ErrorStack([Error { code: 50331836, library: "digital envelope routines", function: "EVP_DigestSign", reason: "final error", file: "crypto/evp/m_sigver.c", line: 585 }])
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
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