Fixes #6602 -- restore the behavior of strict DER parsing for CSRs (#7159)

This commit is contained in:
Alex Gaynor 2022-04-30 13:59:24 -06:00 committed by GitHub
parent 0739003e51
commit 4d5a2c6e1d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 8 additions and 58 deletions

View file

@ -19,6 +19,10 @@ Changelog
newer ``rustc`` if required.
* :meth:`~cryptography.fernet.Fernet.decrypt` and related methods now accept
both ``str`` and ``bytes`` tokens.
* Parsing ``CertificateSigningRequest`` restores the behavior of enforcing
that the ``Extension`` ``critical`` field must be correctly encoded DER. See
`the issue <https://github.com/pyca/cryptography/issues/6368>`_ for complete
details.
.. _v37-0-1:

View file

@ -49,19 +49,8 @@ fn check_attribute_length<'a>(values: asn1::SetOf<'a, asn1::Tlv<'a>>) -> Result<
}
}
// CsrExtension has same layout as Extension, but doesn't use `#[default]` for
// `critical` so we can avoid erroring on explicitly-encoded defaults.
#[derive(asn1::Asn1Read, asn1::Asn1Write, PartialEq, Hash)]
pub(crate) struct CsrExtension<'a> {
pub(crate) extn_id: asn1::ObjectIdentifier,
pub(crate) critical: Option<bool>,
pub(crate) extn_value: &'a [u8],
}
impl CertificationRequestInfo<'_> {
fn get_extension_attribute(
&self,
) -> Result<Option<asn1::SequenceOf<'_, CsrExtension<'_>>>, PyAsn1Error> {
fn get_extension_attribute(&self) -> Result<Option<x509::Extensions<'_>>, PyAsn1Error> {
for attribute in self.attributes.unwrap_read().clone() {
if attribute.type_id == oid::EXTENSION_REQUEST
|| attribute.type_id == oid::MS_EXTENSION_REQUEST
@ -285,40 +274,7 @@ impl CertificateSigningRequest {
#[getter]
fn extensions(&mut self, py: pyo3::Python<'_>) -> pyo3::PyResult<pyo3::PyObject> {
let csr_exts = self.raw.borrow_value().csr_info.get_extension_attribute()?;
let data;
// This is all very inefficient, to temporarily allow accepting
// extensions with `critical` having an explicit default encoding.
let exts = if let Some(v) = csr_exts {
// Raise a warning if there's an explicitly encoded false
for e in v.clone() {
if e.critical == Some(false) {
let cryptography_warning =
py.import("cryptography.utils")?.getattr("DeprecatedIn36")?;
let warnings = py.import("warnings")?;
warnings.call_method1(
"warn",
(
"This CSR contains an improperly encoded default value. Support for this will be removed in an upcoming cryptography release.",
cryptography_warning,
),
)?;
}
}
let x509_exts: Vec<x509::common::Extension<'_>> = v
.map(|e| x509::common::Extension {
extn_id: e.extn_id,
critical: e.critical.unwrap_or_default(),
extn_value: e.extn_value,
})
.collect();
data = asn1::write_single(&asn1::SequenceOfWriter::new(x509_exts));
Some(x509::Asn1ReadableOrWritable::new_read(
asn1::parse_single(&data).unwrap(),
))
} else {
None
};
let exts = self.raw.borrow_value().csr_info.get_extension_attribute()?;
x509::parse_and_cache_extensions(py, &mut self.cached_extensions, &exts, |oid, ext_data| {
certificate::parse_cert_ext(py, oid.clone(), ext_data)

View file

@ -1532,18 +1532,8 @@ class TestRSACertificateRequest:
x509.load_pem_x509_csr,
backend,
)
with pytest.warns(utils.DeprecatedIn36):
subject_alternative_name = csr.extensions.get_extension_for_class(
x509.SubjectAlternativeName
)
assert subject_alternative_name.critical is False
assert len(subject_alternative_name.value) == 3
san1 = subject_alternative_name.value[1]
assert san1.type_id.dotted_string == "1.3.6.1.4.1.311.20.2.3"
san2 = subject_alternative_name.value[2]
assert san2.type_id.dotted_string == "1.3.6.1.5.2.2"
with pytest.raises(ValueError):
csr.extensions
def test_public_bytes_pem(self, backend):
# Load an existing CSR.