Skip to content

Commit 38ca864

Browse files
dwhjamesdwhjames
committed
introduce new error cases
- this led to some refactoring as well
1 parent a391e5b commit 38ca864

File tree

5 files changed

+128
-73
lines changed

5 files changed

+128
-73
lines changed

‎pkcs5/src/error.rs‎

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
//! Error types
22
33
use core::fmt;
4+
use der::asn1::ObjectIdentifier;
45

56
/// Result type
67
pub type Result<T> = core::result::Result<T, Error>;
@@ -9,14 +10,47 @@ pub type Result<T> = core::result::Result<T, Error>;
910
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
1011
#[non_exhaustive]
1112
pub enum Error {
12-
/// Cryptographic errors.
13-
Crypto,
13+
/// Given parameters are invalid for this algorithm
14+
AlgorithmParametersInvalid {
15+
/// OID for algorithm for which the parameters were invalid
16+
oid: ObjectIdentifier,
17+
},
18+
19+
/// Decryption Failed
20+
DecryptFailed,
21+
22+
/// Encryption Failed
23+
EncryptFailed,
24+
25+
/// Pbes1 support is limited to parsing; encryption/decryption is not supported (won't fix)
26+
#[cfg(feature = "pbes2")]
27+
NoPbes1CryptSupport,
28+
29+
/// Algorithm is not supported
30+
///
31+
/// This may be due to a disabled crate feature
32+
/// Or the algorithm is not supported at all.
33+
UnsupportedAlgorithm {
34+
/// OID of unsupported algorithm
35+
oid: ObjectIdentifier,
36+
},
1437
}
1538

1639
impl fmt::Display for Error {
1740
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
18-
f.write_str(match self {
19-
Error::Crypto => "PKCS#5 cryptographic error",
20-
})
41+
match self {
42+
Error::AlgorithmParametersInvalid { oid } => {
43+
write!(f, "PKCS#5 parameters for algorithm {} are invalid", oid)
44+
}
45+
Error::DecryptFailed => f.write_str("PKCS#5 decryption failed"),
46+
Error::EncryptFailed => f.write_str("PKCS#5 encryption failed"),
47+
#[cfg(feature = "pbes2")]
48+
Error::NoPbes1CryptSupport => {
49+
f.write_str("PKCS#5 encryption/decryption unsupported for PBES1 (won't fix)")
50+
}
51+
Error::UnsupportedAlgorithm { oid } => {
52+
write!(f, "PKCS#5 algorithm {} is unsupported", oid)
53+
}
54+
}
2155
}
2256
}

‎pkcs5/src/lib.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ impl<'a> EncryptionScheme<'a> {
6868
pub fn decrypt(&self, password: impl AsRef<[u8]>, ciphertext: &[u8]) -> Result<Vec<u8>> {
6969
match self {
7070
Self::Pbes2(params) => params.decrypt(password, ciphertext),
71-
_ => Err(Error::Crypto),
71+
Self::Pbes1(_) => Err(Error::NoPbes1CryptSupport),
7272
}
7373
}
7474

@@ -87,7 +87,7 @@ impl<'a> EncryptionScheme<'a> {
8787
) -> Result<&'b [u8]> {
8888
match self {
8989
Self::Pbes2(params) => params.decrypt_in_place(password, buffer),
90-
_ => Err(Error::Crypto),
90+
Self::Pbes1(_) => Err(Error::NoPbes1CryptSupport),
9191
}
9292
}
9393

@@ -99,7 +99,7 @@ impl<'a> EncryptionScheme<'a> {
9999
pub fn encrypt(&self, password: impl AsRef<[u8]>, plaintext: &[u8]) -> Result<Vec<u8>> {
100100
match self {
101101
Self::Pbes2(params) => params.encrypt(password, plaintext),
102-
_ => Err(Error::Crypto),
102+
Self::Pbes1(_) => Err(Error::NoPbes1CryptSupport),
103103
}
104104
}
105105

@@ -115,7 +115,7 @@ impl<'a> EncryptionScheme<'a> {
115115
) -> Result<&'b [u8]> {
116116
match self {
117117
Self::Pbes2(params) => params.encrypt_in_place(password, buffer, pos),
118-
_ => Err(Error::Crypto),
118+
Self::Pbes1(_) => Err(Error::NoPbes1CryptSupport),
119119
}
120120
}
121121

‎pkcs5/src/pbes2.rs‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ pub use self::kdf::{
1212
PBKDF2_OID, SCRYPT_OID,
1313
};
1414

15-
use crate::{AlgorithmIdentifier, Result};
15+
use crate::AlgorithmIdentifier;
16+
#[cfg(feature = "scrypt")]
17+
use crate::Result;
1618
use core::convert::{TryFrom, TryInto};
1719
use der::{
1820
asn1::{Any, ObjectIdentifier, OctetString},
@@ -81,10 +83,10 @@ impl<'a> Parameters<'a> {
8183
pbkdf2_iterations: u16,
8284
pbkdf2_salt: &'a [u8],
8385
aes_iv: &'a [u8; AES_BLOCK_SIZE],
84-
) -> Result<Self> {
85-
let kdf = Pbkdf2Params::hmac_with_sha256(pbkdf2_iterations, pbkdf2_salt)?.into();
86+
) -> Self {
87+
let kdf = Pbkdf2Params::hmac_with_sha256(pbkdf2_iterations, pbkdf2_salt).into();
8688
let encryption = EncryptionScheme::Aes128Cbc { iv: aes_iv };
87-
Ok(Self { kdf, encryption })
89+
Self { kdf, encryption }
8890
}
8991

9092
/// Initialize PBES2 parameters using PBKDF2-SHA256 as the password-based
@@ -93,10 +95,10 @@ impl<'a> Parameters<'a> {
9395
pbkdf2_iterations: u16,
9496
pbkdf2_salt: &'a [u8],
9597
aes_iv: &'a [u8; AES_BLOCK_SIZE],
96-
) -> Result<Self> {
97-
let kdf = Pbkdf2Params::hmac_with_sha256(pbkdf2_iterations, pbkdf2_salt)?.into();
98+
) -> Self {
99+
let kdf = Pbkdf2Params::hmac_with_sha256(pbkdf2_iterations, pbkdf2_salt).into();
98100
let encryption = EncryptionScheme::Aes256Cbc { iv: aes_iv };
99-
Ok(Self { kdf, encryption })
101+
Self { kdf, encryption }
100102
}
101103

102104
/// Initialize PBES2 parameters using scrypt as the password-based

‎pkcs5/src/pbes2/encryption.rs‎

Lines changed: 56 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -29,36 +29,50 @@ pub fn encrypt_in_place<'b>(
2929
buffer: &'b mut [u8],
3030
pos: usize,
3131
) -> Result<&'b [u8]> {
32-
let encryption_key = EncryptionKey::derive_from_password(
33-
password.as_ref(),
34-
&params.kdf,
35-
params.encryption.key_size(),
36-
)?;
32+
let key_size = params.encryption.key_size();
33+
let algo_params_invalid_error = Error::AlgorithmParametersInvalid {
34+
oid: params.encryption.oid(),
35+
};
36+
if key_size > MAX_KEY_LEN {
37+
return Err(algo_params_invalid_error);
38+
}
39+
let encryption_key =
40+
EncryptionKey::derive_from_password(password.as_ref(), &params.kdf, key_size)?;
3741

3842
match params.encryption {
3943
EncryptionScheme::Aes128Cbc { iv } => {
4044
let cipher = Aes128Cbc::new_from_slices(encryption_key.as_slice(), iv)
41-
.map_err(|_| Error::Crypto)?;
42-
cipher.encrypt(buffer, pos).map_err(|_| Error::Crypto)
45+
.map_err(|_| algo_params_invalid_error)?;
46+
cipher
47+
.encrypt(buffer, pos)
48+
.map_err(|_| Error::EncryptFailed)
4349
}
4450
EncryptionScheme::Aes192Cbc { iv } => {
4551
let cipher = Aes192Cbc::new_from_slices(encryption_key.as_slice(), iv)
46-
.map_err(|_| Error::Crypto)?;
47-
cipher.encrypt(buffer, pos).map_err(|_| Error::Crypto)
52+
.map_err(|_| algo_params_invalid_error)?;
53+
cipher
54+
.encrypt(buffer, pos)
55+
.map_err(|_| Error::EncryptFailed)
4856
}
4957
EncryptionScheme::Aes256Cbc { iv } => {
5058
let cipher = Aes256Cbc::new_from_slices(encryption_key.as_slice(), iv)
51-
.map_err(|_| Error::Crypto)?;
52-
cipher.encrypt(buffer, pos).map_err(|_| Error::Crypto)
59+
.map_err(|_| algo_params_invalid_error)?;
60+
cipher
61+
.encrypt(buffer, pos)
62+
.map_err(|_| Error::EncryptFailed)
5363
}
5464
#[cfg(feature = "3des")]
5565
EncryptionScheme::DesEde3Cbc { iv } => {
5666
let cipher = DesEde3Cbc::new_from_slices(encryption_key.as_slice(), iv)
57-
.map_err(|_| Error::Crypto)?;
58-
cipher.encrypt(buffer, pos).map_err(|_| Error::Crypto)
67+
.map_err(|_| algo_params_invalid_error)?;
68+
cipher
69+
.encrypt(buffer, pos)
70+
.map_err(|_| Error::EncryptFailed)
5971
}
6072
#[cfg(feature = "des-insecure")]
61-
EncryptionScheme::DesCbc { .. } => Err(Error::Crypto),
73+
EncryptionScheme::DesCbc { .. } => Err(Error::UnsupportedAlgorithm {
74+
oid: super::DES_CBC_OID,
75+
}),
6276
}
6377
}
6478

@@ -74,33 +88,36 @@ pub fn decrypt_in_place<'a>(
7488
params.encryption.key_size(),
7589
)?;
7690

91+
let algo_params_invalid_error = Error::AlgorithmParametersInvalid {
92+
oid: params.encryption.oid(),
93+
};
7794
match params.encryption {
7895
EncryptionScheme::Aes128Cbc { iv } => {
7996
let cipher = Aes128Cbc::new_from_slices(encryption_key.as_slice(), iv)
80-
.map_err(|_| Error::Crypto)?;
81-
cipher.decrypt(buffer).map_err(|_| Error::Crypto)
97+
.map_err(|_| algo_params_invalid_error)?;
98+
cipher.decrypt(buffer).map_err(|_| Error::DecryptFailed)
8299
}
83100
EncryptionScheme::Aes192Cbc { iv } => {
84101
let cipher = Aes192Cbc::new_from_slices(encryption_key.as_slice(), iv)
85-
.map_err(|_| Error::Crypto)?;
86-
cipher.decrypt(buffer).map_err(|_| Error::Crypto)
102+
.map_err(|_| algo_params_invalid_error)?;
103+
cipher.decrypt(buffer).map_err(|_| Error::DecryptFailed)
87104
}
88105
EncryptionScheme::Aes256Cbc { iv } => {
89106
let cipher = Aes256Cbc::new_from_slices(encryption_key.as_slice(), iv)
90-
.map_err(|_| Error::Crypto)?;
91-
cipher.decrypt(buffer).map_err(|_| Error::Crypto)
107+
.map_err(|_| algo_params_invalid_error)?;
108+
cipher.decrypt(buffer).map_err(|_| Error::DecryptFailed)
92109
}
93110
#[cfg(feature = "3des")]
94111
EncryptionScheme::DesEde3Cbc { iv } => {
95112
let cipher = DesEde3Cbc::new_from_slices(encryption_key.as_slice(), iv)
96-
.map_err(|_| Error::Crypto)?;
97-
cipher.decrypt(buffer).map_err(|_| Error::Crypto)
113+
.map_err(|_| algo_params_invalid_error)?;
114+
cipher.decrypt(buffer).map_err(|_| Error::DecryptFailed)
98115
}
99116
#[cfg(feature = "des-insecure")]
100117
EncryptionScheme::DesCbc { iv } => {
101118
let cipher = DesCbc::new_from_slices(encryption_key.as_slice(), iv)
102-
.map_err(|_| Error::Crypto)?;
103-
cipher.decrypt(buffer).map_err(|_| Error::Crypto)
119+
.map_err(|_| algo_params_invalid_error)?;
120+
cipher.decrypt(buffer).map_err(|_| Error::DecryptFailed)
104121
}
105122
}
106123
}
@@ -115,10 +132,15 @@ struct EncryptionKey {
115132
impl EncryptionKey {
116133
/// Derive an encryption key using the supplied PBKDF parameters.
117134
pub fn derive_from_password(password: &[u8], kdf: &Kdf<'_>, key_size: usize) -> Result<Self> {
135+
// if the kdf params defined a key length, ensure it matches the required key size
136+
if let Some(len) = kdf.key_length() {
137+
if key_size != len.into() {
138+
return Err(Error::AlgorithmParametersInvalid { oid: kdf.oid() });
139+
}
140+
}
141+
118142
match kdf {
119143
Kdf::Pbkdf2(pbkdf2_params) => {
120-
validate_key_length(key_size, pbkdf2_params.key_length.map(Into::into))?;
121-
122144
let key = match pbkdf2_params.prf {
123145
#[cfg(feature = "sha1")]
124146
Pbkdf2Prf::HmacWithSha1 => EncryptionKey::derive_with_pbkdf2::<sha1::Sha1>(
@@ -127,7 +149,11 @@ impl EncryptionKey {
127149
key_size,
128150
),
129151
#[cfg(not(feature = "sha1"))]
130-
Pbkdf2Prf::HmacWithSha1 => return Err(Error::Crypto),
152+
Pbkdf2Prf::HmacWithSha1 => {
153+
return Err(Error::UnsupportedAlgorithm {
154+
oid: super::HMAC_WITH_SHA1_OID,
155+
})
156+
}
131157
Pbkdf2Prf::HmacWithSha224 => EncryptionKey::derive_with_pbkdf2::<sha2::Sha224>(
132158
password,
133159
pbkdf2_params,
@@ -181,17 +207,16 @@ impl EncryptionKey {
181207
params: &ScryptParams<'_>,
182208
length: usize,
183209
) -> Result<Self> {
184-
// TODO(tarcieri): move to `derive_from_password`?
185-
validate_key_length(length, params.key_length.map(Into::into))?;
186-
187210
let mut buffer = [0u8; MAX_KEY_LEN];
188211
scrypt(
189212
password,
190213
params.salt,
191214
&params.try_into()?,
192215
&mut buffer[..length],
193216
)
194-
.map_err(|_| Error::Crypto)?;
217+
.map_err(|_| Error::AlgorithmParametersInvalid {
218+
oid: super::SCRYPT_OID,
219+
})?;
195220

196221
Ok(Self { buffer, length })
197222
}
@@ -201,20 +226,3 @@ impl EncryptionKey {
201226
&self.buffer[..self.length]
202227
}
203228
}
204-
205-
/// Validate key length
206-
// TODO(tarcieri): move to `EncryptionKey::derive_from_password`?
207-
fn validate_key_length(requested_len: usize, params_len: Option<usize>) -> Result<()> {
208-
// Ensure key length matches what is expected for the given algorithm
209-
if let Some(len) = params_len {
210-
if requested_len != len {
211-
return Err(Error::Crypto);
212-
}
213-
}
214-
215-
if requested_len > MAX_KEY_LEN {
216-
return Err(Error::Crypto);
217-
}
218-
219-
Ok(())
220-
}

0 commit comments

Comments
 (0)