Skip to content

Commit d8bc890

Browse files
facutuescafacutuesca
committed
Fix the RSA OID used for signing PKCS#7/SMIME
The current implementation computes the algorithm identifier used in the `digest_encryption_algorithm` PKCS#7 field (or `SignatureAlgorithmIdentifier` in S/MIME) based on both the algorithm used to sign (e.g. RSA) and the digest algorithm (e.g. SHA512). This is correct for ECDSA signatures, where the OIDs used include the digest algorithm (e.g: ecdsa-with-SHA512). However, due to historical reasons, when signing with RSA the OID specified should be the one corresponding to just RSA ("1.2.840.113549.1.1.1" rsaEncryption), rather than OIDs which also include the digest algorithm (such as "1.2.840.113549.1.1.13", sha512WithRSAEncryption). This means that the logic to compute the algorithm identifier is the same except when signing with RSA, in which case the OID will always be `rsaEncryption`. This is consistent with the OpenSSL implementation, and the RFCs that define PKCS#7 and S/MIME. See RFC 3851 (section 2.2), and RFC 3370 (section 3.2) for more details.
1 parent 6ef8e9f commit d8bc890

File tree

2 files changed

+83
-32
lines changed

2 files changed

+83
-32
lines changed

‎src/rust/src/pkcs7.rs

+37-2
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,6 @@ fn sign_and_serialize<'p>(
196196
digest_algs.push(digest_alg.clone());
197197
}
198198
certs.push(cert.raw.borrow_dependent());
199-
200199
signer_infos.push(pkcs7::SignerInfo {
201200
version: 1,
202201
issuer_and_serial_number: pkcs7::IssuerAndSerialNumber {
@@ -205,7 +204,7 @@ fn sign_and_serialize<'p>(
205204
},
206205
digest_algorithm: digest_alg,
207206
authenticated_attributes: authenticated_attrs,
208-
digest_encryption_algorithm: x509::sign::compute_signature_algorithm(
207+
digest_encryption_algorithm: compute_pkcs7_signature_algorithm(
209208
py,
210209
py_private_key,
211210
py_hash_alg,
@@ -262,6 +261,42 @@ fn sign_and_serialize<'p>(
262261
}
263262
}
264263

264+
fn compute_pkcs7_signature_algorithm<'p>(
265+
py: pyo3::Python<'p>,
266+
private_key: &'p pyo3::PyAny,
267+
hash_algorithm: &'p pyo3::PyAny,
268+
rsa_padding: &'p pyo3::PyAny,
269+
) -> pyo3::PyResult<common::AlgorithmIdentifier<'static>> {
270+
let key_type = x509::sign::identify_key_type(py, private_key)?;
271+
272+
// If this is RSA-PSS we need to compute the signature algorithm from the
273+
// parameters provided in rsa_padding.
274+
if !rsa_padding.is_none() && rsa_padding.is_instance(types::PSS.get(py)?)? {
275+
return x509::sign::compute_rsa_pss_signature_algorithm(
276+
py,
277+
private_key,
278+
hash_algorithm,
279+
rsa_padding,
280+
);
281+
}
282+
// It's not an RSA PSS signature, so we compute the signature algorithm from
283+
// the key type. For ECDSA signatures, the OID is determined by the signature algorithm
284+
// and the digest algorithm. For RSA signatures, the OID is always the same no matter the
285+
// digest algorithm.
286+
match key_type {
287+
x509::sign::KeyType::Ec => {
288+
x509::sign::compute_signature_algorithm(py, private_key, hash_algorithm, rsa_padding)
289+
}
290+
x509::sign::KeyType::Rsa => Ok(common::AlgorithmIdentifier {
291+
oid: asn1::DefinedByMarker::marker(),
292+
params: common::AlgorithmParameters::Rsa(None),
293+
}),
294+
_ => Err(pyo3::exceptions::PyTypeError::new_err(
295+
"Key type must be either EC or RSA.",
296+
)),
297+
}
298+
}
299+
265300
fn smime_canonicalize(data: &[u8], text_mode: bool) -> (Cow<'_, [u8]>, Cow<'_, [u8]>) {
266301
let mut new_data_with_header = vec![];
267302
let mut new_data_without_header = vec![];

‎src/rust/src/x509/sign.rs

+46-30
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,10 @@ enum HashType {
4848
Sha3_512,
4949
}
5050

51-
fn identify_key_type(py: pyo3::Python<'_>, private_key: &pyo3::PyAny) -> pyo3::PyResult<KeyType> {
51+
pub(crate) fn identify_key_type(
52+
py: pyo3::Python<'_>,
53+
private_key: &pyo3::PyAny,
54+
) -> pyo3::PyResult<KeyType> {
5255
if private_key.is_instance(types::RSA_PRIVATE_KEY.get(py)?)? {
5356
Ok(KeyType::Rsa)
5457
} else if private_key.is_instance(types::DSA_PRIVATE_KEY.get(py)?)? {
@@ -123,6 +126,47 @@ fn compute_pss_salt_length<'p>(
123126
}
124127
}
125128

129+
pub(crate) fn compute_rsa_pss_signature_algorithm<'p>(
130+
py: pyo3::Python<'p>,
131+
private_key: &'p pyo3::PyAny,
132+
hash_algorithm: &'p pyo3::PyAny,
133+
rsa_padding: &'p pyo3::PyAny,
134+
) -> pyo3::PyResult<common::AlgorithmIdentifier<'static>> {
135+
assert!(!rsa_padding.is_none() && rsa_padding.is_instance(types::PSS.get(py)?)?);
136+
let hash_type = identify_hash_type(py, hash_algorithm)?;
137+
138+
// Compute the signature algorithm from the
139+
// parameters provided in rsa_padding.
140+
let hash_alg_params = identify_alg_params_for_hash_type(hash_type)?;
141+
let hash_algorithm_id = common::AlgorithmIdentifier {
142+
oid: asn1::DefinedByMarker::marker(),
143+
params: hash_alg_params,
144+
};
145+
let salt_length = compute_pss_salt_length(py, private_key, hash_algorithm, rsa_padding)?;
146+
let py_mgf_alg = rsa_padding
147+
.getattr(pyo3::intern!(py, "_mgf"))?
148+
.getattr(pyo3::intern!(py, "_algorithm"))?;
149+
let mgf_hash_type = identify_hash_type(py, py_mgf_alg)?;
150+
let mgf_alg = common::AlgorithmIdentifier {
151+
oid: asn1::DefinedByMarker::marker(),
152+
params: identify_alg_params_for_hash_type(mgf_hash_type)?,
153+
};
154+
let params = common::AlgorithmParameters::RsaPss(Some(Box::new(common::RsaPssParameters {
155+
hash_algorithm: hash_algorithm_id,
156+
mask_gen_algorithm: common::MaskGenAlgorithm {
157+
oid: oid::MGF1_OID,
158+
params: mgf_alg,
159+
},
160+
salt_length,
161+
_trailer_field: 1,
162+
})));
163+
164+
Ok(common::AlgorithmIdentifier {
165+
oid: asn1::DefinedByMarker::marker(),
166+
params,
167+
})
168+
}
169+
126170
pub(crate) fn compute_signature_algorithm<'p>(
127171
py: pyo3::Python<'p>,
128172
private_key: &'p pyo3::PyAny,
@@ -135,35 +179,7 @@ pub(crate) fn compute_signature_algorithm<'p>(
135179
// If this is RSA-PSS we need to compute the signature algorithm from the
136180
// parameters provided in rsa_padding.
137181
if !rsa_padding.is_none() && rsa_padding.is_instance(types::PSS.get(py)?)? {
138-
let hash_alg_params = identify_alg_params_for_hash_type(hash_type)?;
139-
let hash_algorithm_id = common::AlgorithmIdentifier {
140-
oid: asn1::DefinedByMarker::marker(),
141-
params: hash_alg_params,
142-
};
143-
let salt_length = compute_pss_salt_length(py, private_key, hash_algorithm, rsa_padding)?;
144-
let py_mgf_alg = rsa_padding
145-
.getattr(pyo3::intern!(py, "_mgf"))?
146-
.getattr(pyo3::intern!(py, "_algorithm"))?;
147-
let mgf_hash_type = identify_hash_type(py, py_mgf_alg)?;
148-
let mgf_alg = common::AlgorithmIdentifier {
149-
oid: asn1::DefinedByMarker::marker(),
150-
params: identify_alg_params_for_hash_type(mgf_hash_type)?,
151-
};
152-
let params =
153-
common::AlgorithmParameters::RsaPss(Some(Box::new(common::RsaPssParameters {
154-
hash_algorithm: hash_algorithm_id,
155-
mask_gen_algorithm: common::MaskGenAlgorithm {
156-
oid: oid::MGF1_OID,
157-
params: mgf_alg,
158-
},
159-
salt_length,
160-
_trailer_field: 1,
161-
})));
162-
163-
return Ok(common::AlgorithmIdentifier {
164-
oid: asn1::DefinedByMarker::marker(),
165-
params,
166-
});
182+
return compute_rsa_pss_signature_algorithm(py, private_key, hash_algorithm, rsa_padding);
167183
}
168184
// It's not an RSA PSS signature, so we compute the signature algorithm from
169185
// the union of key type and hash type.

0 commit comments

Comments
 (0)