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

fix: Allow empty passwords for encrypted pem files #381

Merged
merged 3 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 59 additions & 15 deletions src/crypto/signing_key/ecdsa/ec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,12 @@ where
let ec_seckey = SecretKey::<C>::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),
gmpinder marked this conversation as resolved.
Show resolved Hide resolved
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}"
))),
Expand Down Expand Up @@ -248,13 +254,10 @@ where
/// Return the encrypted private key in PEM-encoded format.
fn private_key_to_encrypted_pem(&self, password: &[u8]) -> Result<Zeroizing<String>> {
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))
}
Expand Down Expand Up @@ -369,6 +372,8 @@ where
mod tests {
use std::fs;

use rstest::rstest;

use crate::crypto::{
signing_key::{tests::MESSAGE, KeyPair, Signer},
verification_key::CosignVerificationKey,
Expand All @@ -378,6 +383,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`.
Expand All @@ -394,11 +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::<p256::NistP256>::from_encrypted_pem(&content, 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::<p256::NistP256>::from_encrypted_pem(&content, password);
assert!(
key.is_ok(),
"can not create EcdsaKeys from encrypted PEM file"
Expand All @@ -407,17 +418,34 @@ 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::<p256::NistP256>::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 ensure that an unencrypted
/// keypair will fail to read if a non-empty
/// password is given.
#[test]
fn ecdsa_error_unencrypted_pem_password() {
let content = fs::read("tests/data/keys/ecdsa_private.key").expect("read key failed.");
let key = EcdsaKeys::<p256::NistP256>::from_encrypted_pem(&content, PASSWORD);
assert!(
key.is_err_and(|e| e
.to_string()
.contains("Unencrypted private key but password provided")),
"read unencrypted key with password"
);
}

/// 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.
Expand All @@ -432,6 +460,22 @@ 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.
#[rstest]
#[case(PASSWORD)]
#[case::empty_password(EMPTY_PASSWORD)]
fn ecdsa_to_and_from_encrypted_pem(#[case] password: &[u8]) {
let key =
EcdsaKeys::<p256::NistP256>::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::<p256::NistP256>::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 der, and decode a new key from the generated der-encoded
/// private key.
Expand Down
69 changes: 54 additions & 15 deletions src/crypto/signing_key/ed25519.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
gmpinder marked this conversation as resolved.
Show resolved Hide resolved
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}"
))),
Expand Down Expand Up @@ -207,13 +213,10 @@ impl KeyPair for Ed25519Keys {
/// Return the encrypted asn.1 pkcs8 private key.
fn private_key_to_encrypted_pem(&self, password: &[u8]) -> Result<zeroize::Zeroizing<String>> {
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))
}
Expand Down Expand Up @@ -282,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,
Expand All @@ -291,6 +296,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`.
Expand All @@ -307,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"
Expand All @@ -320,16 +327,33 @@ 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 ensure that an unencrypted
/// keypair will fail to read if a non-empty
/// password is given.
#[test]
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_err_and(|e| e
.to_string()
.contains("Unencrypted private key but password provided")),
"read unencrypted key with password"
);
}

/// This test will generate a Ed25519Keys, encode the private key
/// into pem, and decode a new key from the generated pem-encoded
/// private key.
Expand All @@ -343,6 +367,21 @@ 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.
#[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(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
/// it into der, and decode a new key from the generated der-encoded
/// private key.
Expand Down
11 changes: 8 additions & 3 deletions src/crypto/signing_key/kdf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
},
Expand All @@ -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(),
Expand Down
Loading
Loading