From c80a715e2cf5413d8db5d049336242cf1683c27c Mon Sep 17 00:00:00 2001 From: Gerald Pinder Date: Tue, 6 Aug 2024 15:08:29 -0400 Subject: [PATCH 1/3] fix: Allow empty passwords for encrypted pem files Signed-off-by: Gerald Pinder --- src/crypto/signing_key/ecdsa/ec.rs | 53 +++++++++++++++++++++++---- src/crypto/signing_key/ed25519.rs | 50 +++++++++++++++++++++---- src/crypto/signing_key/rsa/keypair.rs | 51 ++++++++++++++++++++++---- 3 files changed, 133 insertions(+), 21 deletions(-) diff --git a/src/crypto/signing_key/ecdsa/ec.rs b/src/crypto/signing_key/ecdsa/ec.rs index a9cd15b5d3..8e68f892d1 100644 --- a/src/crypto/signing_key/ecdsa/ec.rs +++ b/src/crypto/signing_key/ecdsa/ec.rs @@ -248,13 +248,10 @@ where /// Return the encrypted private key in PEM-encoded format. fn private_key_to_encrypted_pem(&self, password: &[u8]) -> Result> { let der = self.private_key_to_der()?; - let pem = match password.len() { - 0 => pem::Pem::new(PRIVATE_KEY_PEM_LABEL, der.to_vec()), - _ => pem::Pem::new( - SIGSTORE_PRIVATE_KEY_PEM_LABEL, - kdf::encrypt(&der, password)?, - ), - }; + let pem = pem::Pem::new( + SIGSTORE_PRIVATE_KEY_PEM_LABEL, + kdf::encrypt(&der, password)?, + ); let pem = pem::encode(&pem); Ok(zeroize::Zeroizing::new(pem)) } @@ -378,6 +375,7 @@ mod tests { use super::{EcdsaKeys, EcdsaSigner}; const PASSWORD: &[u8] = b"123"; + const EMPTY_PASSWORD: &[u8] = b""; /// This test will try to read an unencrypted ecdsa /// private key file, which is generated by `sigstore`. @@ -418,6 +416,19 @@ mod tests { ); } + /// This test will try to encrypt a ecdsa keypair and + /// return the pem-encoded contents. + #[test] + fn ecdsa_to_encrypted_pem_empty_password() { + let key = + EcdsaKeys::::new().expect("create ecdsa keys with P256 curve failed."); + let key = key.private_key_to_encrypted_pem(EMPTY_PASSWORD); + assert!( + key.is_ok(), + "can not export private key in encrypted PEM format." + ); + } + /// This test will generate a EcdsaKeys, encode the private key /// it into pem, and decode a new key from the generated pem-encoded /// private key. @@ -432,6 +443,34 @@ mod tests { assert!(key.is_ok(), "can not create EcdsaKeys from PEM string."); } + /// This test will generate a EcdsaKeys, encode the private key + /// it into pem, and decode a new key from the generated pem-encoded + /// private key. + #[test] + fn ecdsa_to_and_from_encrypted_pem() { + let key = + EcdsaKeys::::new().expect("create ecdsa keys with P256 curve failed."); + let key = key + .private_key_to_encrypted_pem(PASSWORD) + .expect("export private key to PEM format failed."); + let key = EcdsaKeys::::from_encrypted_pem(key.as_bytes(), PASSWORD); + assert!(key.is_ok(), "can not create EcdsaKeys from PEM string."); + } + + /// This test will generate a EcdsaKeys, encode the private key + /// it into pem, and decode a new key from the generated pem-encoded + /// private key. + #[test] + fn ecdsa_to_and_from_encrypted_pem_empty_password() { + let key = + EcdsaKeys::::new().expect("create ecdsa keys with P256 curve failed."); + let key = key + .private_key_to_encrypted_pem(EMPTY_PASSWORD) + .expect("export private key to PEM format failed."); + let key = EcdsaKeys::::from_encrypted_pem(key.as_bytes(), EMPTY_PASSWORD); + assert!(key.is_ok(), "can not create EcdsaKeys from PEM string."); + } + /// This test will generate a EcdsaKeys, encode the private key /// it into der, and decode a new key from the generated der-encoded /// private key. diff --git a/src/crypto/signing_key/ed25519.rs b/src/crypto/signing_key/ed25519.rs index f1292c12a7..24d01ba261 100644 --- a/src/crypto/signing_key/ed25519.rs +++ b/src/crypto/signing_key/ed25519.rs @@ -207,13 +207,10 @@ impl KeyPair for Ed25519Keys { /// Return the encrypted asn.1 pkcs8 private key. fn private_key_to_encrypted_pem(&self, password: &[u8]) -> Result> { let der = self.private_key_to_der()?; - let pem = match password.len() { - 0 => pem::Pem::new(PRIVATE_KEY_PEM_LABEL, der.to_vec()), - _ => pem::Pem::new( - SIGSTORE_PRIVATE_KEY_PEM_LABEL, - kdf::encrypt(&der, password)?, - ), - }; + let pem = pem::Pem::new( + SIGSTORE_PRIVATE_KEY_PEM_LABEL, + kdf::encrypt(&der, password)?, + ); let pem = pem::encode(&pem); Ok(zeroize::Zeroizing::new(pem)) } @@ -291,6 +288,7 @@ mod tests { use super::{Ed25519Keys, Ed25519Signer}; const PASSWORD: &[u8] = b"123"; + const EMPTY_PASSWORD: &[u8] = b""; /// This test will try to read an unencrypted ed25519 /// private key file, which is generated by `sigstore`. @@ -330,6 +328,18 @@ mod tests { ); } + /// This test will try to encrypt a ed25519 keypair and + /// return the pem-encoded contents. + #[test] + fn ed25519_to_encrypted_pem_empty_password() { + let key = Ed25519Keys::new().expect("create Ed25519 keys failed."); + let key = key.private_key_to_encrypted_pem(EMPTY_PASSWORD); + assert!( + key.is_ok(), + "can not export private key in encrypted PEM format." + ); + } + /// This test will generate a Ed25519Keys, encode the private key /// into pem, and decode a new key from the generated pem-encoded /// private key. @@ -343,6 +353,32 @@ mod tests { assert!(key.is_ok(), "can not create Ed25519Keys from PEM string."); } + /// This test will generate a Ed25519Keys, encode the private key + /// into pem, and decode a new key from the generated pem-encoded + /// private key. + #[test] + fn ed25519_to_and_from_encrypted_pem() { + let key = Ed25519Keys::new().expect("create ed25519 keys failed."); + let key = key + .private_key_to_encrypted_pem(PASSWORD) + .expect("export private key to PEM format failed."); + let key = Ed25519Keys::from_encrypted_pem(key.as_bytes(), PASSWORD); + assert!(key.is_ok(), "can not create Ed25519Keys from PEM string."); + } + + /// This test will generate a Ed25519Keys, encode the private key + /// into pem, and decode a new key from the generated pem-encoded + /// private key. + #[test] + fn ed25519_to_and_from_encrypted_pem_empty_password() { + let key = Ed25519Keys::new().expect("create ed25519 keys failed."); + let key = key + .private_key_to_encrypted_pem(EMPTY_PASSWORD) + .expect("export private key to PEM format failed."); + let key = Ed25519Keys::from_encrypted_pem(key.as_bytes(), EMPTY_PASSWORD); + assert!(key.is_ok(), "can not create Ed25519Keys from PEM string."); + } + /// This test will generate a Ed25519Keys, encode the private key /// it into der, and decode a new key from the generated der-encoded /// private key. diff --git a/src/crypto/signing_key/rsa/keypair.rs b/src/crypto/signing_key/rsa/keypair.rs index f54294f598..9f02468adc 100644 --- a/src/crypto/signing_key/rsa/keypair.rs +++ b/src/crypto/signing_key/rsa/keypair.rs @@ -231,13 +231,10 @@ impl KeyPair for RSAKeys { /// Return the encrypted asn.1 pkcs8 private key. fn private_key_to_encrypted_pem(&self, password: &[u8]) -> Result> { let der = self.private_key_to_der()?; - let pem = match password.len() { - 0 => pem::Pem::new(PRIVATE_KEY_PEM_LABEL, der.to_vec()), - _ => pem::Pem::new( - SIGSTORE_PRIVATE_KEY_PEM_LABEL, - kdf::encrypt(&der, password)?, - ), - }; + let pem = pem::Pem::new( + SIGSTORE_PRIVATE_KEY_PEM_LABEL, + kdf::encrypt(&der, password)?, + ); let pem = pem::encode(&pem); Ok(zeroize::Zeroizing::new(pem)) } @@ -283,6 +280,7 @@ mod tests { use super::RSAKeys; const PASSWORD: &[u8] = b"123"; + const EMPTY_PASSWORD: &[u8] = b""; const KEY_SIZE: usize = 2048; /// This test will try to read an unencrypted rsa @@ -324,6 +322,19 @@ mod tests { ); } + /// This test will try to encrypt a rsa keypair and + /// return the pem-encoded contents. The bit size + /// of the rsa key is [`KEY_SIZE`]. + #[test] + fn rsa_to_encrypted_pem_empty_password() { + let key = RSAKeys::new(KEY_SIZE).expect("create rsa keys failed."); + let key = key.private_key_to_encrypted_pem(EMPTY_PASSWORD); + assert!( + key.is_ok(), + "can not export private key in encrypted PEM format." + ); + } + /// This test will generate a RSAKeys, encode the private key /// it into pem, and decode a new key from the generated pem-encoded /// private key. @@ -337,6 +348,32 @@ mod tests { assert!(key.is_ok(), "can not create RSAKeys from PEM string."); } + /// This test will generate a RSAKeys, encode the private key + /// it into pem, and decode a new key from the generated pem-encoded + /// private key. + #[test] + fn rsa_to_and_from_encrypted_pem() { + let key = RSAKeys::new(KEY_SIZE).expect("create rsa keys failed."); + let key = key + .private_key_to_encrypted_pem(PASSWORD) + .expect("export private key to PEM format failed."); + let key = RSAKeys::from_encrypted_pem(key.as_bytes(), PASSWORD); + assert!(key.is_ok(), "can not create RSAKeys from PEM string."); + } + + /// This test will generate a RSAKeys, encode the private key + /// it into pem, and decode a new key from the generated pem-encoded + /// private key. + #[test] + fn rsa_to_and_from_encrypted_pem_empty_password() { + let key = RSAKeys::new(KEY_SIZE).expect("create rsa keys failed."); + let key = key + .private_key_to_encrypted_pem(EMPTY_PASSWORD) + .expect("export private key to PEM format failed."); + let key = RSAKeys::from_encrypted_pem(key.as_bytes(), EMPTY_PASSWORD); + assert!(key.is_ok(), "can not create RSAKeys from PEM string."); + } + /// This test will generate a RSAKeys, encode the private key /// it into der, and decode a new key from the generated der-encoded /// private key. From 48857ffaf1a8aa3bdb03d3d6bf6b93e7a9802472 Mon Sep 17 00:00:00 2001 From: Gerald Pinder Date: Tue, 6 Aug 2024 15:11:18 -0400 Subject: [PATCH 2/3] fix: Allow reading cosign generated encrypted PEM files Signed-off-by: Gerald Pinder --- src/crypto/signing_key/ecdsa/ec.rs | 13 +++++++++++++ src/crypto/signing_key/kdf.rs | 11 ++++++++--- .../cosign_generated_encrypted_empty_private.key | 11 +++++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 tests/data/keys/cosign_generated_encrypted_empty_private.key diff --git a/src/crypto/signing_key/ecdsa/ec.rs b/src/crypto/signing_key/ecdsa/ec.rs index 8e68f892d1..353379b92c 100644 --- a/src/crypto/signing_key/ecdsa/ec.rs +++ b/src/crypto/signing_key/ecdsa/ec.rs @@ -403,6 +403,19 @@ mod tests { ); } + /// This test will try to read an encrypted ecdsa + /// private key file, which is generated by `sigstore`. + #[test] + fn ecdsa_from_encrypted_pem_cosign_empty_password() { + let content = fs::read("tests/data/keys/cosign_generated_encrypted_empty_private.key") + .expect("read tests/data/keys/cosign_generated_encrypted_empty_private.key failed."); + let key = EcdsaKeys::::from_encrypted_pem(&content, EMPTY_PASSWORD); + assert!( + key.is_ok(), + "can not create EcdsaKeys from encrypted PEM file" + ); + } + /// This test will try to encrypt a ecdsa keypair and /// return the pem-encoded contents. #[test] diff --git a/src/crypto/signing_key/kdf.rs b/src/crypto/signing_key/kdf.rs index 4fb5a7f9db..b991b4eae6 100644 --- a/src/crypto/signing_key/kdf.rs +++ b/src/crypto/signing_key/kdf.rs @@ -33,7 +33,9 @@ pub const SALT_SIZE: u32 = 32; pub const NAME_SCRYPT: &str = "scrypt"; /// Scrypt algorithm parameter log2(n) -pub const SCRYPT_N: u32 = 32768; +pub const SCRYPT_N_LOW: u32 = 32768; + +pub const SCRYPT_N_HIGH: u32 = 65536; /// Scrypt algorithm parameter r pub const SCRYPT_R: u32 = 8; @@ -95,7 +97,7 @@ impl Default for ScryptKDF { Self { name: NAME_SCRYPT.into(), params: ScryptParams { - n: SCRYPT_N, + n: SCRYPT_N_LOW, r: SCRYPT_R, p: SCRYPT_P, }, @@ -122,7 +124,10 @@ impl ScryptKDF { /// Check whether the given params is as the default, /// to avoid a DoS attack. fn check_params(&self) -> Result<()> { - match self.params.n == SCRYPT_N && self.params.r == SCRYPT_R && self.params.p == SCRYPT_P { + match (self.params.n == SCRYPT_N_LOW || self.params.n == SCRYPT_N_HIGH) + && self.params.r == SCRYPT_R + && self.params.p == SCRYPT_P + { true => Ok(()), false => Err(SigstoreError::PrivateKeyDecryptError( "Unexpected kdf parameters".into(), diff --git a/tests/data/keys/cosign_generated_encrypted_empty_private.key b/tests/data/keys/cosign_generated_encrypted_empty_private.key new file mode 100644 index 0000000000..44246c12ff --- /dev/null +++ b/tests/data/keys/cosign_generated_encrypted_empty_private.key @@ -0,0 +1,11 @@ +-----BEGIN ENCRYPTED SIGSTORE PRIVATE KEY----- +eyJrZGYiOnsibmFtZSI6InNjcnlwdCIsInBhcmFtcyI6eyJOIjo2NTUzNiwiciI6 +OCwicCI6MX0sInNhbHQiOiJFY3pJR3Z0bnpFcCsxaEpudERnbGI1UnoxN3gwQVMy +YklvWGRNcDQrU1NJPSJ9LCJjaXBoZXIiOnsibmFtZSI6Im5hY2wvc2VjcmV0Ym94 +Iiwibm9uY2UiOiJtSVVKbG1OMzNINExvZ0dhaG5MamdmK3R4SmJwci93MCJ9LCJj +aXBoZXJ0ZXh0IjoiM3FtS2FidXRuYm1WT1IvTkZGM2NDaUErTXp1WWQ5L0VERDVI +MVlUSGhQVzZsSzAvdjVqdDZCQzlsME12NGV5am9qZkl0d3B6a2JJOXQrZGx5VXZ3 +VUgvMWZjbkFZT2dEdXRLQzkvSkNiOE02SVY2VHpVaDF5c0ZFWDFzeG9xb1FzeUpL +bjM0UVlldFNlaVdDaGtmUHUyZXByQjFEV0pwekdzTGZIakxNVTkzOEdmNk1xM1A0 +N0ZSd2syZzY1cFZnSGMwVWV1L1N1OUs0Zmc9PSJ9 +-----END ENCRYPTED SIGSTORE PRIVATE KEY----- From cd0d10429fc09ddb08a8d1c7244a1623772113af Mon Sep 17 00:00:00 2001 From: Gerald Pinder Date: Sat, 10 Aug 2024 13:04:48 -0400 Subject: [PATCH 3/3] fix: Allow using from_encrypted_pem for unencrypted pems if an empty password is given Signed-off-by: Gerald Pinder --- src/crypto/signing_key/ecdsa/ec.rs | 86 ++++++++++++--------------- src/crypto/signing_key/ed25519.rs | 67 +++++++++++---------- src/crypto/signing_key/rsa/keypair.rs | 72 +++++++++++----------- 3 files changed, 112 insertions(+), 113 deletions(-) diff --git a/src/crypto/signing_key/ecdsa/ec.rs b/src/crypto/signing_key/ecdsa/ec.rs index 353379b92c..94577afba5 100644 --- a/src/crypto/signing_key/ecdsa/ec.rs +++ b/src/crypto/signing_key/ecdsa/ec.rs @@ -153,6 +153,12 @@ where let ec_seckey = SecretKey::::from_sec1_der(pkcs8.private_key)?; Self::from_private_key(ec_seckey) } + PRIVATE_KEY_PEM_LABEL if password.is_empty() => Self::from_pem(private_key), + PRIVATE_KEY_PEM_LABEL if !password.is_empty() => { + Err(SigstoreError::PrivateKeyDecryptError( + "Unencrypted private key but password provided".into(), + )) + } tag => Err(SigstoreError::PrivateKeyDecryptError(format!( "Unsupported pem tag {tag}" ))), @@ -366,6 +372,8 @@ where mod tests { use std::fs; + use rstest::rstest; + use crate::crypto::{ signing_key::{tests::MESSAGE, KeyPair, Signer}, verification_key::CosignVerificationKey, @@ -392,24 +400,16 @@ mod tests { /// This test will try to read an encrypted ecdsa /// private key file, which is generated by `sigstore`. - #[test] - fn ecdsa_from_encrypted_pem() { - let content = fs::read("tests/data/keys/ecdsa_encrypted_private.key") - .expect("read tests/data/keys/ecdsa_encrypted_private.key failed."); - let key = EcdsaKeys::::from_encrypted_pem(&content, PASSWORD); - assert!( - key.is_ok(), - "can not create EcdsaKeys from encrypted PEM file" - ); - } - - /// This test will try to read an encrypted ecdsa - /// private key file, which is generated by `sigstore`. - #[test] - fn ecdsa_from_encrypted_pem_cosign_empty_password() { - let content = fs::read("tests/data/keys/cosign_generated_encrypted_empty_private.key") - .expect("read tests/data/keys/cosign_generated_encrypted_empty_private.key failed."); - let key = EcdsaKeys::::from_encrypted_pem(&content, EMPTY_PASSWORD); + #[rstest] + #[case("tests/data/keys/ecdsa_encrypted_private.key", PASSWORD)] + #[case::empty_password( + "tests/data/keys/cosign_generated_encrypted_empty_private.key", + EMPTY_PASSWORD + )] + #[case::empty_password_unencrypted("tests/data/keys/ecdsa_private.key", EMPTY_PASSWORD)] + fn ecdsa_from_encrypted_pem(#[case] keypath: &str, #[case] password: &[u8]) { + let content = fs::read(keypath).expect("read key failed."); + let key = EcdsaKeys::::from_encrypted_pem(&content, password); assert!( key.is_ok(), "can not create EcdsaKeys from encrypted PEM file" @@ -418,27 +418,31 @@ mod tests { /// This test will try to encrypt a ecdsa keypair and /// return the pem-encoded contents. - #[test] - fn ecdsa_to_encrypted_pem() { + #[rstest] + #[case(PASSWORD)] + #[case::empty_password(EMPTY_PASSWORD)] + fn ecdsa_to_encrypted_pem(#[case] password: &[u8]) { let key = EcdsaKeys::::new().expect("create ecdsa keys with P256 curve failed."); - let key = key.private_key_to_encrypted_pem(PASSWORD); + let key = key.private_key_to_encrypted_pem(password); assert!( key.is_ok(), "can not export private key in encrypted PEM format." ); } - /// This test will try to encrypt a ecdsa keypair and - /// return the pem-encoded contents. + /// This test will ensure that an unencrypted + /// keypair will fail to read if a non-empty + /// password is given. #[test] - fn ecdsa_to_encrypted_pem_empty_password() { - let key = - EcdsaKeys::::new().expect("create ecdsa keys with P256 curve failed."); - let key = key.private_key_to_encrypted_pem(EMPTY_PASSWORD); + fn ecdsa_error_unencrypted_pem_password() { + let content = fs::read("tests/data/keys/ecdsa_private.key").expect("read key failed."); + let key = EcdsaKeys::::from_encrypted_pem(&content, PASSWORD); assert!( - key.is_ok(), - "can not export private key in encrypted PEM format." + key.is_err_and(|e| e + .to_string() + .contains("Unencrypted private key but password provided")), + "read unencrypted key with password" ); } @@ -459,28 +463,16 @@ mod tests { /// This test will generate a EcdsaKeys, encode the private key /// it into pem, and decode a new key from the generated pem-encoded /// private key. - #[test] - fn ecdsa_to_and_from_encrypted_pem() { - let key = - EcdsaKeys::::new().expect("create ecdsa keys with P256 curve failed."); - let key = key - .private_key_to_encrypted_pem(PASSWORD) - .expect("export private key to PEM format failed."); - let key = EcdsaKeys::::from_encrypted_pem(key.as_bytes(), PASSWORD); - assert!(key.is_ok(), "can not create EcdsaKeys from PEM string."); - } - - /// This test will generate a EcdsaKeys, encode the private key - /// it into pem, and decode a new key from the generated pem-encoded - /// private key. - #[test] - fn ecdsa_to_and_from_encrypted_pem_empty_password() { + #[rstest] + #[case(PASSWORD)] + #[case::empty_password(EMPTY_PASSWORD)] + fn ecdsa_to_and_from_encrypted_pem(#[case] password: &[u8]) { let key = EcdsaKeys::::new().expect("create ecdsa keys with P256 curve failed."); let key = key - .private_key_to_encrypted_pem(EMPTY_PASSWORD) + .private_key_to_encrypted_pem(password) .expect("export private key to PEM format failed."); - let key = EcdsaKeys::::from_encrypted_pem(key.as_bytes(), EMPTY_PASSWORD); + let key = EcdsaKeys::::from_encrypted_pem(key.as_bytes(), password); assert!(key.is_ok(), "can not create EcdsaKeys from PEM string."); } diff --git a/src/crypto/signing_key/ed25519.rs b/src/crypto/signing_key/ed25519.rs index 24d01ba261..0946e0fbe9 100644 --- a/src/crypto/signing_key/ed25519.rs +++ b/src/crypto/signing_key/ed25519.rs @@ -120,6 +120,12 @@ impl Ed25519Keys { })?; Self::from_key_pair_bytes(key_pair_bytes) } + PRIVATE_KEY_PEM_LABEL if password.is_empty() => Self::from_pem(encrypted_pem), + PRIVATE_KEY_PEM_LABEL if !password.is_empty() => { + Err(SigstoreError::PrivateKeyDecryptError( + "Unencrypted private key but password provided".into(), + )) + } tag => Err(SigstoreError::PrivateKeyDecryptError(format!( "Unsupported pem tag {tag}" ))), @@ -279,6 +285,8 @@ impl Signer for Ed25519Signer { mod tests { use std::fs; + use rstest::rstest; + use crate::crypto::{ signing_key::{tests::MESSAGE, KeyPair, Signer}, verification_key::CosignVerificationKey, @@ -305,11 +313,12 @@ mod tests { /// This test will try to read an encrypted ed25519 /// private key file, which is generated by `sigstore`. - #[test] - fn ed25519_from_encrypted_pem() { - let content = fs::read("tests/data/keys/ed25519_encrypted_private.key") - .expect("read tests/data/keys/ed25519_encrypted_private.key failed."); - let key = Ed25519Keys::from_encrypted_pem(&content, PASSWORD); + #[rstest] + #[case("tests/data/keys/ed25519_encrypted_private.key", PASSWORD)] + #[case::empty_password("tests/data/keys/ed25519_private.key", EMPTY_PASSWORD)] + fn ed25519_from_encrypted_pem(#[case] keypath: &str, #[case] password: &[u8]) { + let content = fs::read(keypath).expect("read key failed."); + let key = Ed25519Keys::from_encrypted_pem(&content, password); assert!( key.is_ok(), "can not create Ed25519Keys from encrypted PEM file" @@ -318,25 +327,30 @@ mod tests { /// This test will try to encrypt a ed25519 keypair and /// return the pem-encoded contents. - #[test] - fn ed25519_to_encrypted_pem() { + #[rstest] + #[case(PASSWORD)] + #[case::empty_password(EMPTY_PASSWORD)] + fn ed25519_to_encrypted_pem(#[case] password: &[u8]) { let key = Ed25519Keys::new().expect("create Ed25519 keys failed."); - let key = key.private_key_to_encrypted_pem(PASSWORD); + let key = key.private_key_to_encrypted_pem(password); assert!( key.is_ok(), "can not export private key in encrypted PEM format." ); } - /// This test will try to encrypt a ed25519 keypair and - /// return the pem-encoded contents. + /// This test will ensure that an unencrypted + /// keypair will fail to read if a non-empty + /// password is given. #[test] - fn ed25519_to_encrypted_pem_empty_password() { - let key = Ed25519Keys::new().expect("create Ed25519 keys failed."); - let key = key.private_key_to_encrypted_pem(EMPTY_PASSWORD); + fn ed25519_error_unencrypted_pem_password() { + let content = fs::read("tests/data/keys/ed25519_private.key").expect("read key failed."); + let key = Ed25519Keys::from_encrypted_pem(&content, PASSWORD); assert!( - key.is_ok(), - "can not export private key in encrypted PEM format." + key.is_err_and(|e| e + .to_string() + .contains("Unencrypted private key but password provided")), + "read unencrypted key with password" ); } @@ -356,26 +370,15 @@ mod tests { /// This test will generate a Ed25519Keys, encode the private key /// into pem, and decode a new key from the generated pem-encoded /// private key. - #[test] - fn ed25519_to_and_from_encrypted_pem() { - let key = Ed25519Keys::new().expect("create ed25519 keys failed."); - let key = key - .private_key_to_encrypted_pem(PASSWORD) - .expect("export private key to PEM format failed."); - let key = Ed25519Keys::from_encrypted_pem(key.as_bytes(), PASSWORD); - assert!(key.is_ok(), "can not create Ed25519Keys from PEM string."); - } - - /// This test will generate a Ed25519Keys, encode the private key - /// into pem, and decode a new key from the generated pem-encoded - /// private key. - #[test] - fn ed25519_to_and_from_encrypted_pem_empty_password() { + #[rstest] + #[case(PASSWORD)] + #[case::empty_password(EMPTY_PASSWORD)] + fn ed25519_to_and_from_encrypted_pem(#[case] password: &[u8]) { let key = Ed25519Keys::new().expect("create ed25519 keys failed."); let key = key - .private_key_to_encrypted_pem(EMPTY_PASSWORD) + .private_key_to_encrypted_pem(password) .expect("export private key to PEM format failed."); - let key = Ed25519Keys::from_encrypted_pem(key.as_bytes(), EMPTY_PASSWORD); + let key = Ed25519Keys::from_encrypted_pem(key.as_bytes(), password); assert!(key.is_ok(), "can not create Ed25519Keys from PEM string."); } diff --git a/src/crypto/signing_key/rsa/keypair.rs b/src/crypto/signing_key/rsa/keypair.rs index 9f02468adc..868b2a3b8c 100644 --- a/src/crypto/signing_key/rsa/keypair.rs +++ b/src/crypto/signing_key/rsa/keypair.rs @@ -101,7 +101,14 @@ impl RSAKeys { })?; Ok(Self::from(private_key)) } - + RSA_PRIVATE_KEY_PEM_LABEL | PRIVATE_KEY_PEM_LABEL if password.is_empty() => { + Self::from_pem(encrypted_pem) + } + RSA_PRIVATE_KEY_PEM_LABEL | PRIVATE_KEY_PEM_LABEL if !password.is_empty() => { + Err(SigstoreError::PrivateKeyDecryptError( + "Unencrypted private key but password provided".into(), + )) + } tag => Err(SigstoreError::PrivateKeyDecryptError(format!( "Unsupported pem tag {tag}" ))), @@ -267,6 +274,8 @@ impl KeyPair for RSAKeys { mod tests { use std::fs; + use rstest::rstest; + use crate::crypto::{ signing_key::{ rsa::{DigestAlgorithm, PaddingScheme, RSASigner}, @@ -298,11 +307,13 @@ mod tests { /// This test will try to read an encrypted rsa /// private key file, which is generated by `sigstore`. - #[test] - fn rsa_from_encrypted_pem() { - let content = fs::read("tests/data/keys/rsa_encrypted_private.key") - .expect("read tests/data/keys/rsa_encrypted_private.key failed."); - let key = RSAKeys::from_encrypted_pem(&content, PASSWORD); + #[rstest] + #[case("tests/data/keys/rsa_encrypted_private.key", PASSWORD)] + #[case("tests/data/keys/rsa_private.key", EMPTY_PASSWORD)] + fn rsa_from_encrypted_pem(#[case] keypath: &str, #[case] password: &[u8]) { + let content = + fs::read(keypath).expect("read tests/data/keys/rsa_encrypted_private.key failed."); + let key = RSAKeys::from_encrypted_pem(&content, password); assert!( key.is_ok(), "can not create RSAKeys from encrypted PEM file" @@ -312,26 +323,30 @@ mod tests { /// This test will try to encrypt a rsa keypair and /// return the pem-encoded contents. The bit size /// of the rsa key is [`KEY_SIZE`]. - #[test] - fn rsa_to_encrypted_pem() { + #[rstest] + #[case(PASSWORD)] + #[case::empty_password(PASSWORD)] + fn rsa_to_encrypted_pem(#[case] password: &[u8]) { let key = RSAKeys::new(KEY_SIZE).expect("create rsa keys failed."); - let key = key.private_key_to_encrypted_pem(PASSWORD); + let key = key.private_key_to_encrypted_pem(password); assert!( key.is_ok(), "can not export private key in encrypted PEM format." ); } - /// This test will try to encrypt a rsa keypair and - /// return the pem-encoded contents. The bit size - /// of the rsa key is [`KEY_SIZE`]. + /// This test will ensure that an unencrypted + /// keypair will fail to read if a non-empty + /// password is given. #[test] - fn rsa_to_encrypted_pem_empty_password() { - let key = RSAKeys::new(KEY_SIZE).expect("create rsa keys failed."); - let key = key.private_key_to_encrypted_pem(EMPTY_PASSWORD); + fn rsa_error_unencrypted_pem_password() { + let content = fs::read("tests/data/keys/rsa_private.key").expect("read key failed."); + let key = RSAKeys::from_encrypted_pem(&content, PASSWORD); assert!( - key.is_ok(), - "can not export private key in encrypted PEM format." + key.is_err_and(|e| e + .to_string() + .contains("Unencrypted private key but password provided")), + "read unencrypted key with password" ); } @@ -351,26 +366,15 @@ mod tests { /// This test will generate a RSAKeys, encode the private key /// it into pem, and decode a new key from the generated pem-encoded /// private key. - #[test] - fn rsa_to_and_from_encrypted_pem() { - let key = RSAKeys::new(KEY_SIZE).expect("create rsa keys failed."); - let key = key - .private_key_to_encrypted_pem(PASSWORD) - .expect("export private key to PEM format failed."); - let key = RSAKeys::from_encrypted_pem(key.as_bytes(), PASSWORD); - assert!(key.is_ok(), "can not create RSAKeys from PEM string."); - } - - /// This test will generate a RSAKeys, encode the private key - /// it into pem, and decode a new key from the generated pem-encoded - /// private key. - #[test] - fn rsa_to_and_from_encrypted_pem_empty_password() { + #[rstest] + #[case(PASSWORD)] + #[case::empty_password(EMPTY_PASSWORD)] + fn rsa_to_and_from_encrypted_pem(#[case] password: &[u8]) { let key = RSAKeys::new(KEY_SIZE).expect("create rsa keys failed."); let key = key - .private_key_to_encrypted_pem(EMPTY_PASSWORD) + .private_key_to_encrypted_pem(password) .expect("export private key to PEM format failed."); - let key = RSAKeys::from_encrypted_pem(key.as_bytes(), EMPTY_PASSWORD); + let key = RSAKeys::from_encrypted_pem(key.as_bytes(), password); assert!(key.is_ok(), "can not create RSAKeys from PEM string."); }