Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
facutuesca committed Jun 24, 2024
1 parent 3b88019 commit d941be0
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 31 deletions.
31 changes: 16 additions & 15 deletions src/cryptography/hazmat/primitives/serialization/pkcs7.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,9 @@ def sign(
class PKCS7EnvelopeBuilder:
def __init__(
self,
data: bytes | None = None,
recipients: list[x509.Certificate] | None = None,
*,
_data: bytes | None = None,
_recipients: list[x509.Certificate] | None = None,
):
from cryptography.hazmat.backends.openssl.backend import (
backend as ossl,
Expand All @@ -194,15 +195,15 @@ def __init__(
" of OpenSSL.",
_Reasons.UNSUPPORTED_PADDING,
)
self._data = data
self._recipients = recipients if recipients is not None else []
self._data = _data
self._recipients = _recipients if _recipients is not None else []

def set_data(self, data: bytes) -> PKCS7EnvelopeBuilder:
_check_byteslike("data", data)
if self._data is not None:
raise ValueError("data may only be set once")

return PKCS7EnvelopeBuilder(data, self._recipients)
return PKCS7EnvelopeBuilder(_data=data, _recipients=self._recipients)

def add_recipient(
self,
Expand All @@ -215,8 +216,8 @@ def add_recipient(
raise TypeError("Only RSA keys are supported at this time.")

return PKCS7EnvelopeBuilder(
self._data,
[
_data=self._data,
_recipients=[
*self._recipients,
certificate,
],
Expand Down Expand Up @@ -244,16 +245,16 @@ def encrypt(
"Must be PEM, DER, or SMIME from the Encoding enum"
)

# These options only make sense when signing, not encrypting
if (
PKCS7Options.NoAttributes in options
or PKCS7Options.NoCapabilities in options
or PKCS7Options.NoCerts in options
or PKCS7Options.DetachedSignature in options
# Only allow options that make sense when signing
if any(
[
opt not in [PKCS7Options.Text, PKCS7Options.Binary]
for opt in options
]
):
raise ValueError(
"The following options are not supported for encryption:"
"NoAttributes, NoCapabilities, NoCerts, DetachedSignature"
"Only the following options are supported for signing: "
"Text, Binary"
)

return rust_pkcs7.encrypt_and_serialize(self, encoding, options)
Expand Down
4 changes: 2 additions & 2 deletions src/rust/cryptography-x509/src/pkcs7.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,10 @@ pub struct SignerInfo<'a> {

#[derive(asn1::Asn1Write)]
pub struct EncryptedContentInfo<'a> {
pub _content_type: asn1::ObjectIdentifier,
pub content_type: asn1::ObjectIdentifier,
pub content_encryption_algorithm: common::AlgorithmIdentifier<'a>,
#[implicit(0)]
pub content: Option<&'a [u8]>,
pub encrypted_content: Option<&'a [u8]>,
}

#[derive(asn1::Asn1Write)]
Expand Down
9 changes: 6 additions & 3 deletions src/rust/src/padding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,17 @@ pub(crate) struct PKCS7PaddingContext {
#[pyo3::prelude::pymethods]
impl PKCS7PaddingContext {
#[new]
fn new(block_size: usize) -> PKCS7PaddingContext {
pub(crate) fn new(block_size: usize) -> PKCS7PaddingContext {
PKCS7PaddingContext {
block_size: block_size / 8,
length_seen: Some(0),
}
}

fn update<'a>(&mut self, buf: CffiBuf<'a>) -> CryptographyResult<pyo3::Bound<'a, pyo3::PyAny>> {
pub(crate) fn update<'a>(
&mut self,
buf: CffiBuf<'a>,
) -> CryptographyResult<pyo3::Bound<'a, pyo3::PyAny>> {
match self.length_seen.as_mut() {
Some(v) => {
*v += buf.as_bytes().len();
Expand All @@ -95,7 +98,7 @@ impl PKCS7PaddingContext {
}
}

fn finalize<'p>(
pub(crate) fn finalize<'p>(
&mut self,
py: pyo3::Python<'p>,
) -> CryptographyResult<pyo3::Bound<'p, pyo3::types::PyBytes>> {
Expand Down
17 changes: 8 additions & 9 deletions src/rust/src/pkcs7.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ use once_cell::sync::Lazy;
#[cfg(not(CRYPTOGRAPHY_IS_BORINGSSL))]
use openssl::pkcs7::Pkcs7;
use pyo3::prelude::{PyAnyMethods, PyBytesMethods, PyListMethods, PyModuleMethods};
use pyo3::types::PyBytes;
#[cfg(not(CRYPTOGRAPHY_IS_BORINGSSL))]
use pyo3::IntoPy;

use crate::asn1::encode_der_data;
use crate::buf::CffiBuf;
use crate::error::{CryptographyError, CryptographyResult};
use crate::padding::PKCS7PaddingContext;
#[cfg(not(CRYPTOGRAPHY_IS_BORINGSSL))]
use crate::x509::certificate::load_der_x509_certificate;
use crate::{exceptions, types, x509};
Expand Down Expand Up @@ -92,13 +94,10 @@ fn encrypt_and_serialize<'p>(
smime_canonicalize(raw_data.as_bytes(), text_mode).0
};

let padder = types::SYMMETRIC_PADDING_PKCS7
.get(py)?
.call1((128,))?
.call_method0(pyo3::intern!(py, "padder"))?;
let padded_content_start =
padder.call_method1(pyo3::intern!(py, "update"), (data_with_header,))?;
let padded_content_end = padder.call_method0(pyo3::intern!(py, "finalize"))?;
let data_with_header = PyBytes::new_bound(py, &data_with_header);
let mut padder = PKCS7PaddingContext::new(128);
let padded_content_start = padder.update(data_with_header.extract()?)?;
let padded_content_end = padder.finalize(py)?;
let padded_content = padded_content_start.add(padded_content_end)?;

// The message is encrypted with AES-128-CBC, which the S/MIME v3.2 RFC
Expand Down Expand Up @@ -148,12 +147,12 @@ fn encrypt_and_serialize<'p>(
recipient_infos: asn1::SetOfWriter::new(&recipient_infos),

encrypted_content_info: pkcs7::EncryptedContentInfo {
_content_type: PKCS7_DATA_OID,
content_type: PKCS7_DATA_OID,
content_encryption_algorithm: AlgorithmIdentifier {
oid: asn1::DefinedByMarker::marker(),
params: AlgorithmParameters::AesCbc(iv.extract()?),
},
content: Some(encrypted_content.extract()?),
encrypted_content: Some(encrypted_content.extract()?),
},
};

Expand Down
2 changes: 0 additions & 2 deletions src/rust/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,8 +357,6 @@ pub static PREHASHED: LazyPyImport = LazyPyImport::new(
"cryptography.hazmat.primitives.asymmetric.utils",
&["Prehashed"],
);
pub static SYMMETRIC_PADDING_PKCS7: LazyPyImport =
LazyPyImport::new("cryptography.hazmat.primitives.padding", &["PKCS7"]);
pub static ASYMMETRIC_PADDING: LazyPyImport = LazyPyImport::new(
"cryptography.hazmat.primitives.asymmetric.padding",
&["AsymmetricPadding"],
Expand Down

0 comments on commit d941be0

Please sign in to comment.