jedisct1 / rust-minisign Goto Github PK

It appears that signature verification when prehashing in use doesn't work. Every verification fails with "Signature verification failed".

I wrote the following trivial demo program which takes a pubkey and path on argv and attempts to verify it:

use std::fs::File;
use std::io::Read;

fn main() {
    let mut args = std::env::args();
    args.next();
    let public_key = args.next().unwrap();
    let path = args.next().unwrap();

    println!("public key is {:?}", public_key);

    let pk = minisign::PublicKey::from_base64(&public_key).unwrap();

    let mut sig_path = path.clone();
    sig_path.push_str(".minisig");

    let mut sig_file = File::open(sig_path).unwrap();

    let mut signature = Vec::new();
    sig_file.read_to_end(&mut signature).unwrap();
    let signature =
        minisign::SignatureBox::from_string(String::from_utf8_lossy(&signature).as_ref()).unwrap();

    let mut f = File::open(path).unwrap();

    println!(
        "verify? {:?}",
        minisign::verify(&pk, &signature, &mut f, true, false)
    );
}

If you sign the file with minisign -S -m /path/to/file, it verifies fine. If you sign it with minisign -S -H -m /path/to/file, verification fails every time. I'm not familiar enough with libsodium to explain away the differences between the Rust and C implementations.

Hello 👋

When upgrading from 0.7.3 to 0.7.5 we've started getting an Wrong password for that key error when using no/empty passwords on Windows (100% reliably) when calling sign(). We also saw this happening on Linux after a while but i think it wasn't reliably reproducible.
Speaking of reproduction, i couldn't reproduce it in a standalone rust project yet, only in Tauri's CLI but i wanted to reach out already anyway. -> I will update this issue once i am able to repro it but i have to focus on some other stuff for a while.

In case you wanna look into it without me providing a minimal repro, the relevant function is here: https://github.com/tauri-apps/tauri/blob/4dd4893d7d166ac3a3b6dc2e3bd2540326352a78/tooling/cli/src/helpers/updater_signature.rs#L103 with the password being either None (so that minisgn asks for the pw) or an empty String (if CI env var is set) - both variants show the same behavior.

Thank you

P.S. For reference the PR where we locked minisign to 0.7.3: tauri-apps/tauri#7197

When using the key pair struct returned by KeyPair::generate_encrypted_keypair to create a signature, verification using the public key fails with PError { kind: Verify, err: "Could not verify signature with the provided public key ID: 2265E6986983A09D" }.

When converting the secret key to a string and then loading it again first, signature creation and validation works as expected.

See this gist for short example:
https://gist.github.com/Laegluin/b07abe10bfabb01b16903c9e1b5aef50

This project is great, but one minor problem is that it reimplements lots of stuff without exposing it. For example, libsodium is reimplemented, but what if other parts of my projects also need libsodium? I would like to be able to compile this crate with another implementation of libsodium (libsodium-sys?).

I am trying run it in wasm, and return :

error[E0425]: cannot find function `prompt_password` in this scope
  --> C:\Users\77236\.cargo\registry\src\github.com-1ecc6299db9ec823\minisign-0.7.1\src\helpers.rs:77:15
   |
77 |     let pwd = prompt_password(prompt)?;
   |               ^^^^^^^^^^^^^^^ not found in this scope

I ran into errors running this in WASM on wasmer. The problem is something I've run into a few times which is that getrandom with the js feature enabled does not work. I need to enable custom which I can do but there is no way for me to disable the js feature that this crate enables.

I am opening an issue rather than a PR because I can't find a tidy way to fix this.

I changed the Cargo.toml from

[target.'cfg(all(any(target_arch = "wasm32", target_arch = "wasm64"), target_os = "unknown"))'.dependencies]
getrandom = { version = "0.2", optional = false, default-features = false, features = ["js"] }

[target.'cfg(not(all(any(target_arch = "wasm32", target_arch = "wasm64"), target_os = "unknown")))'.dependencies]
getrandom = { version = "0.2", optional = false, default-features = false }

[dependencies]
scrypt = { version = "0.11.0", default-features = false }

to

[dependencies]
scrypt = { version = "0.11.0", default-features = false }
getrandom = { version = "0.2", default-features = false }

[target.'cfg(any(windows, unix))'.dependencies]
rpassword = "7.3.1"

[features]
wasm_js = ["getrandom/js"]
wasm_custom = ["getrandom/custom"]

Which I think will work for everyone, but it also means that everyone targeting WASM has to explicitly opt-in to a feature. I'm okay with that for myself and this solves my problem. Any objections to this suggestion?

The abbreviation "pk" seems error-prone; it could abbreviate either "public key" or "private key". Does it seem reasonable to consider expanding it to something like "pubkey" instead?

I'd be happy to provide a patch for this, but I wanted to file an issue for discussion first.

Hello! Thanks for maintaining this crate!!

While implementing things for wapm, I was running in to some issues, so I decided to make a test case. After making the test case, I noticed a new issue:

#[cfg(test)]
mod test {
    #[test]
    fn sanity_check() {
        let public_key = "untrusted comment: minisign public key 3013F76F7B1F0672
RWRyBh97b/cTMGoJsnKDGmleEpeCGBg9AMxJYeSYsIXYo09Hq6mg7irv";
        let private_key = "untrusted comment: minisign encrypted secret key
RWRTY0IyUIpMvYs4tqNv2cTpBMGYhg2sCN8aCzICPKRNlK19UR4AAAACAAAAAAAAAEAAAAAAArdUKPFVxDpsyIJK5yVkB3f/t1oIL8HUfXDI2TEKgfEbll/bfhGt7zf+8m7DEpVu6S18Nx7lyDyiDgEMSEhS4SeJg0cn4ckpHNO15OZuVt0SGpG1EjsXtGS1xnj+MaCh91xbAqdQrzY=";

        let data = std::fs::File::open("/Users/mark/hello.txt").unwrap();

        let pub_key = minisign::PublicKey::from_base64(&public_key.lines().skip(1).next().unwrap())
            .expect("PK");
        let priv_key = minisign::SecretKeyBox::from_string(&private_key)
            .unwrap()
            .into_secret_key(Some("a".to_string()))
            .expect("priv key");

        let sig = minisign::sign(Some(&pub_key), &priv_key, &data, false, None, None)
            .expect("sign")
            .to_string();

        let sig_box = minisign::SignatureBox::from_string(&sig).expect("sig box");
        assert!(minisign::verify(&pub_key, &sig_box, &data, true, false).is_ok());
    }
}

This appears to get stuck in an infinite loop. If it's not an infinite loop, it's very slow. I've let it sit for about 10 minutes or so.

hello.txt is just a file with hello in it.

I briefly looked over the code and wasn't able to identify the location of the problem

Hi,

I signed some data using both https://github.com/jedisct1/rsign2 and https://github.com/jedisct1/minisign

I was confused at first that the signatures did not match although both were valid. Going from the Signature format explanation on https://jedisct1.github.io/minisign/ I did not expect the second line of the signature to vary, though that may also be a result of me not being a cryptographer.

I then signed the same data a few more times to find out that minisign signature stays the same while rsign signature changes with every signing operation. Looking into the source code I saw that rust-minisign generates a nonce for every signing operation while the C minisign does not. Are there any security implications of this that would result in having to prefer one implementation over the other?

I also think it would be good to state the fact that rust-minisign uses a nonce in this crate's documentation, in rsign's documentation or in both. This would make it clearer to an investigating user that the second line of signature can change although the Signature format explanation does not mention any variable elements there (again, that's for a programmer with only basic knowledge of cryptography, maybe it would have been obvious to someone having a firm knowledge of ed25519 implementation).