diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 580a777bfdd0..96a60c2106da 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -55,6 +55,7 @@ Changelog :class:`~cryptography.hazmat.primitives.kdf.concatkdf.ConcatKDFHMAC`, :class:`~cryptography.hazmat.primitives.kdf.argon2.Argon2id`, :class:`~cryptography.hazmat.primitives.kdf.pbkdf2.PBKDF2HMAC`, + :class:`~cryptography.hazmat.primitives.kdf.kbkdf.KBKDFHMAC`, :class:`~cryptography.hazmat.primitives.kdf.scrypt.Scrypt`, and :class:`~cryptography.hazmat.primitives.kdf.x963kdf.X963KDF` to allow deriving keys directly into pre-allocated buffers. diff --git a/docs/hazmat/primitives/key-derivation-functions.rst b/docs/hazmat/primitives/key-derivation-functions.rst index f8228c6d9586..4115624d6022 100644 --- a/docs/hazmat/primitives/key-derivation-functions.rst +++ b/docs/hazmat/primitives/key-derivation-functions.rst @@ -1090,13 +1090,40 @@ KBKDF :raises TypeError: This exception is raised if ``key_material`` is not ``bytes``. :raises cryptography.exceptions.AlreadyFinalized: This is raised when - :meth:`derive` or + :meth:`derive`, + :meth:`derive_into`, or :meth:`verify` is called more than once. Derives a new key from the input key material. + .. method:: derive_into(key_material, buffer) + + .. versionadded:: 47.0.0 + + :param key_material: The input key material. + :type key_material: :term:`bytes-like` + :param buffer: A writable buffer to write the derived key into. The + buffer must be equal to the length supplied in the + constructor. + :type buffer: :term:`bytes-like` + :return int: the number of bytes written to the buffer. + :raises ValueError: This exception is raised if the buffer length does + not match the specified ``length``. + :raises TypeError: This exception is raised if ``key_material`` or + ``buffer`` is not ``bytes``. + :raises cryptography.exceptions.AlreadyFinalized: This is raised when + :meth:`derive`, + :meth:`derive_into`, or + :meth:`verify` is + called more than + once. + + Derives a new key from the input key material and writes it into + the provided buffer. This is useful when you want to avoid allocating + new memory for the derived key. + .. method:: verify(key_material, expected_key) :param bytes key_material: The input key material. This is the same as @@ -1108,7 +1135,8 @@ KBKDF derived key does not match the expected key. :raises cryptography.exceptions.AlreadyFinalized: This is raised when - :meth:`derive` or + :meth:`derive`, + :meth:`derive_into`, or :meth:`verify` is called more than once. diff --git a/src/cryptography/hazmat/bindings/_rust/openssl/kdf.pyi b/src/cryptography/hazmat/bindings/_rust/openssl/kdf.pyi index 29d380ab214f..d807755559ab 100644 --- a/src/cryptography/hazmat/bindings/_rust/openssl/kdf.pyi +++ b/src/cryptography/hazmat/bindings/_rust/openssl/kdf.pyi @@ -5,6 +5,7 @@ import typing from cryptography.hazmat.primitives.hashes import HashAlgorithm +from cryptography.hazmat.primitives.kdf.kbkdf import CounterLocation, Mode from cryptography.utils import Buffer class PBKDF2HMAC: @@ -162,3 +163,23 @@ class ConcatKDFHMAC: def derive(self, key_material: Buffer) -> bytes: ... def derive_into(self, key_material: Buffer, buffer: Buffer) -> int: ... def verify(self, key_material: bytes, expected_key: bytes) -> None: ... + +class KBKDFHMAC: + def __init__( + self, + algorithm: HashAlgorithm, + mode: Mode, + length: int, + rlen: int, + llen: int | None, + location: CounterLocation, + label: bytes | None, + context: bytes | None, + fixed: bytes | None, + backend: typing.Any = None, + *, + break_location: int | None = None, + ) -> None: ... + def derive(self, key_material: Buffer) -> bytes: ... + def derive_into(self, key_material: Buffer, buffer: Buffer) -> int: ... + def verify(self, key_material: bytes, expected_key: bytes) -> None: ... diff --git a/src/cryptography/hazmat/primitives/kdf/kbkdf.py b/src/cryptography/hazmat/primitives/kdf/kbkdf.py index 5b4713761679..a00b00008dd6 100644 --- a/src/cryptography/hazmat/primitives/kdf/kbkdf.py +++ b/src/cryptography/hazmat/primitives/kdf/kbkdf.py @@ -14,13 +14,8 @@ UnsupportedAlgorithm, _Reasons, ) -from cryptography.hazmat.primitives import ( - ciphers, - cmac, - constant_time, - hashes, - hmac, -) +from cryptography.hazmat.bindings._rust import openssl as rust_openssl +from cryptography.hazmat.primitives import ciphers, cmac, constant_time from cryptography.hazmat.primitives.kdf import KeyDerivationFunction @@ -178,62 +173,8 @@ def _generate_fixed_input(self) -> bytes: return b"".join([self._label, b"\x00", self._context, l_val]) -class KBKDFHMAC(KeyDerivationFunction): - def __init__( - self, - algorithm: hashes.HashAlgorithm, - mode: Mode, - length: int, - rlen: int, - llen: int | None, - location: CounterLocation, - label: bytes | None, - context: bytes | None, - fixed: bytes | None, - backend: typing.Any = None, - *, - break_location: int | None = None, - ): - if not isinstance(algorithm, hashes.HashAlgorithm): - raise UnsupportedAlgorithm( - "Algorithm supplied is not a supported hash algorithm.", - _Reasons.UNSUPPORTED_HASH, - ) - - from cryptography.hazmat.backends.openssl.backend import ( - backend as ossl, - ) - - if not ossl.hmac_supported(algorithm): - raise UnsupportedAlgorithm( - "Algorithm supplied is not a supported hmac algorithm.", - _Reasons.UNSUPPORTED_HASH, - ) - - self._algorithm = algorithm - - self._deriver = _KBKDFDeriver( - self._prf, - mode, - length, - rlen, - llen, - location, - break_location, - label, - context, - fixed, - ) - - def _prf(self, key_material: bytes) -> hmac.HMAC: - return hmac.HMAC(key_material, self._algorithm) - - def derive(self, key_material: utils.Buffer) -> bytes: - return self._deriver.derive(key_material, self._algorithm.digest_size) - - def verify(self, key_material: bytes, expected_key: bytes) -> None: - if not constant_time.bytes_eq(self.derive(key_material), expected_key): - raise InvalidKey +KBKDFHMAC = rust_openssl.kdf.KBKDFHMAC +KeyDerivationFunction.register(KBKDFHMAC) class KBKDFCMAC(KeyDerivationFunction): diff --git a/src/rust/src/asn1.rs b/src/rust/src/asn1.rs index cb71dccbe62a..f99dec1864d1 100644 --- a/src/rust/src/asn1.rs +++ b/src/rust/src/asn1.rs @@ -71,22 +71,31 @@ pub(crate) fn py_uint_to_big_endian_bytes<'p>( py: pyo3::Python<'p>, v: pyo3::Bound<'p, pyo3::types::PyInt>, ) -> pyo3::PyResult { - if v.lt(0)? { - return Err(pyo3::exceptions::PyValueError::new_err( - "Negative integers are not supported", - )); - } - // Round the length up so that we prefix an extra \x00. This ensures that // integers that'd have the high bit set in their first octet are not // encoded as negative in DER. - let n = v + let length = v .call_method0(pyo3::intern!(py, "bit_length"))? .extract::()? / 8 + 1; - Ok(v.call_method1(pyo3::intern!(py, "to_bytes"), (n, "big"))? - .extract()?) + py_uint_to_be_bytes_with_length(py, v, length) +} + +pub(crate) fn py_uint_to_be_bytes_with_length<'p>( + py: pyo3::Python<'p>, + v: pyo3::Bound<'p, pyo3::types::PyInt>, + length: usize, +) -> pyo3::PyResult { + if v.lt(0)? { + return Err(pyo3::exceptions::PyValueError::new_err( + "Negative integers are not supported", + )); + } + Ok( + v.call_method1(pyo3::intern!(py, "to_bytes"), (length, "big"))? + .extract()?, + ) } pub(crate) fn encode_der_data<'p>( diff --git a/src/rust/src/backend/kdf.rs b/src/rust/src/backend/kdf.rs index 68e4af677a70..3101543cc430 100644 --- a/src/rust/src/backend/kdf.rs +++ b/src/rust/src/backend/kdf.rs @@ -9,6 +9,7 @@ use base64::engine::Engine; use cryptography_crypto::constant_time; use pyo3::types::{PyAnyMethods, PyBytesMethods}; +use crate::asn1::py_uint_to_be_bytes_with_length; use crate::backend::hashes; use crate::backend::hmac::Hmac; use crate::buf::{CffiBuf, CffiMutBuf}; @@ -1696,11 +1697,318 @@ impl ConcatKdfHmac { } } +// NO-COVERAGE-START +#[pyo3::pyclass( + module = "cryptography.hazmat.primitives.kdf.kbkdf", + name = "KBKDFHMAC" +)] +// NO-COVERAGE-END +struct KbkdfHmac { + algorithm: pyo3::Py, + length: usize, + params: KbkdfParams, + used: bool, +} + +#[allow(clippy::enum_variant_names)] +#[derive(PartialEq)] +enum CounterLocation { + BeforeFixed, + AfterFixed, + MiddleFixed(usize), +} + +struct KbkdfParams { + rlen: usize, + llen: Option, + location: CounterLocation, + label: Option>, + context: Option>, + fixed: Option>, +} + +#[allow(clippy::too_many_arguments)] +fn validate_kbkdf_parameters( + py: pyo3::Python<'_>, + mode: pyo3::Py, + rlen: usize, + llen: Option, + location: pyo3::Py, + label: Option>, + context: Option>, + fixed: Option>, + break_location: Option, +) -> CryptographyResult { + let mode_bound = mode.bind(py); + let mode_type = crate::types::KBKDF_MODE.get(py)?; + if !mode_bound.is_instance(&mode_type)? { + return Err(CryptographyError::from( + pyo3::exceptions::PyTypeError::new_err("mode must be of type Mode"), + )); + } + + let location_bound = location.bind(py); + let counter_location = crate::types::KBKDF_COUNTER_LOCATION.get(py)?; + if !location_bound.is_instance(&counter_location)? { + return Err(CryptographyError::from( + pyo3::exceptions::PyTypeError::new_err("location must be of type CounterLocation"), + )); + } + + let counter_location_before_fixed = + counter_location.getattr(pyo3::intern!(py, "BeforeFixed"))?; + let counter_location_after_fixed = counter_location.getattr(pyo3::intern!(py, "AfterFixed"))?; + let rust_location = if location_bound.eq(&counter_location_before_fixed)? { + CounterLocation::BeforeFixed + } else if location_bound.eq(&counter_location_after_fixed)? { + CounterLocation::AfterFixed + } else { + // There are only 3 options so this is MiddleFixed + match break_location { + Some(break_location) => CounterLocation::MiddleFixed(break_location), + None => { + return Err(CryptographyError::from( + pyo3::exceptions::PyValueError::new_err("Please specify a break_location"), + )) + } + } + }; + + if break_location.is_some() && !matches!(rust_location, CounterLocation::MiddleFixed(_)) { + return Err(CryptographyError::from( + pyo3::exceptions::PyValueError::new_err( + "break_location is ignored when location is not CounterLocation.MiddleFixed", + ), + )); + } + + if (label.is_some() || context.is_some()) && fixed.is_some() { + return Err(CryptographyError::from( + pyo3::exceptions::PyValueError::new_err( + "When supplying fixed data, label and context are ignored.", + ), + )); + } + + if !(1..=4).contains(&rlen) { + return Err(CryptographyError::from( + pyo3::exceptions::PyValueError::new_err("rlen must be between 1 and 4"), + )); + } + + if fixed.is_none() && llen.is_none() { + return Err(CryptographyError::from( + pyo3::exceptions::PyValueError::new_err("Please specify an llen"), + )); + } + + if llen == Some(0) { + return Err(CryptographyError::from( + pyo3::exceptions::PyValueError::new_err("llen must be non-zero"), + )); + } + + Ok(KbkdfParams { + rlen, + llen, + location: rust_location, + label, + context, + fixed, + }) +} + +impl KbkdfHmac { + fn derive_into_buffer( + &mut self, + py: pyo3::Python<'_>, + key_material: &[u8], + output: &mut [u8], + ) -> CryptographyResult { + if self.used { + return Err(exceptions::already_finalized_error()); + } + self.used = true; + + if output.len() != self.length { + return Err(CryptographyError::from( + pyo3::exceptions::PyValueError::new_err(format!( + "buffer must be {} bytes", + self.length + )), + )); + } + + let algorithm_bound = self.algorithm.bind(py); + let digest_size = algorithm_bound + .getattr(pyo3::intern!(py, "digest_size"))? + .extract::()?; + + let fixed = self.generate_fixed_input(py)?; + + let (data_before_ctr, data_after_ctr) = match self.params.location { + CounterLocation::BeforeFixed => (&b""[..], &fixed[..]), + CounterLocation::AfterFixed => (&fixed[..], &b""[..]), + CounterLocation::MiddleFixed(break_location) => { + if break_location > fixed.len() { + return Err(CryptographyError::from( + pyo3::exceptions::PyValueError::new_err( + "break_location offset > len(fixed)", + ), + )); + } + (&fixed[..break_location], &fixed[break_location..]) + } + }; + + let mut pos = 0usize; + let rounds = self.length.div_ceil(digest_size); + for i in 1..=rounds { + let mut hmac = Hmac::new_bytes(py, key_material, algorithm_bound)?; + + let py_i = pyo3::types::PyInt::new(py, i); + let counter = py_uint_to_be_bytes_with_length(py, py_i, self.params.rlen)?; + hmac.update_bytes(data_before_ctr)?; + hmac.update_bytes(counter.as_ref())?; + hmac.update_bytes(data_after_ctr)?; + + let result = hmac.finalize_bytes()?; + + let copy_len = (self.length - pos).min(digest_size); + output[pos..pos + copy_len].copy_from_slice(&result[..copy_len]); + pos += copy_len; + } + + Ok(self.length) + } + + fn generate_fixed_input(&self, py: pyo3::Python<'_>) -> CryptographyResult> { + if let Some(ref fixed_data) = self.params.fixed { + return Ok(fixed_data.as_bytes(py).to_vec()); + } + + // llen will exist if fixed data is not provided + let py_bitlength = pyo3::types::PyInt::new( + py, + self.length + .checked_mul(8) + .ok_or(pyo3::exceptions::PyOverflowError::new_err( + "Length too large, would cause overflow in bit length calculation", + ))?, + ); + let l_val = py_uint_to_be_bytes_with_length(py, py_bitlength, self.params.llen.unwrap())?; + + let mut result = Vec::new(); + let label: &[u8] = self.params.label.as_ref().map_or(b"", |l| l.as_bytes(py)); + result.extend_from_slice(label); + result.push(0x00); + let context: &[u8] = self.params.context.as_ref().map_or(b"", |l| l.as_bytes(py)); + result.extend_from_slice(context); + result.extend_from_slice(l_val.as_ref()); + + Ok(result) + } +} + +#[pyo3::pymethods] +impl KbkdfHmac { + #[new] + #[pyo3(signature = (algorithm, mode, length, rlen, llen, location, label, context, fixed, backend=None, *, break_location=None))] + #[allow(clippy::too_many_arguments)] + fn new( + py: pyo3::Python<'_>, + algorithm: pyo3::Py, + mode: pyo3::Py, + length: usize, + rlen: usize, + llen: Option, + location: pyo3::Py, + label: Option>, + context: Option>, + fixed: Option>, + backend: Option>, + break_location: Option, + ) -> CryptographyResult { + _ = backend; + + // Validate common KBKDF parameters + let params = validate_kbkdf_parameters( + py, + mode, + rlen, + llen, + location, + label, + context, + fixed, + break_location, + )?; + + let algorithm_bound = algorithm.bind(py); + let _md = hashes::message_digest_from_algorithm(py, algorithm_bound)?; + let digest_size = algorithm_bound + .getattr(pyo3::intern!(py, "digest_size"))? + .extract::()?; + let rounds = length.div_ceil(digest_size); + if rounds as u64 > (1u64 << (params.rlen * 8)) - 1 { + return Err(CryptographyError::from( + pyo3::exceptions::PyValueError::new_err("There are too many iterations."), + )); + } + + Ok(KbkdfHmac { + algorithm, + length, + params, + used: false, + }) + } + + fn derive<'p>( + &mut self, + py: pyo3::Python<'p>, + key_material: CffiBuf<'_>, + ) -> CryptographyResult> { + Ok(pyo3::types::PyBytes::new_with(py, self.length, |output| { + self.derive_into_buffer(py, key_material.as_bytes(), output)?; + Ok(()) + })?) + } + + fn derive_into( + &mut self, + py: pyo3::Python<'_>, + key_material: CffiBuf<'_>, + mut buf: CffiMutBuf<'_>, + ) -> CryptographyResult { + self.derive_into_buffer(py, key_material.as_bytes(), buf.as_mut_bytes()) + } + + fn verify( + &mut self, + py: pyo3::Python<'_>, + key_material: CffiBuf<'_>, + expected_key: CffiBuf<'_>, + ) -> CryptographyResult<()> { + let actual = self.derive(py, key_material)?; + let actual_bytes = actual.as_bytes(); + let expected_bytes = expected_key.as_bytes(); + + if !constant_time::bytes_eq(actual_bytes, expected_bytes) { + return Err(CryptographyError::from(exceptions::InvalidKey::new_err( + "Keys do not match.", + ))); + } + + Ok(()) + } +} + #[pyo3::pymodule(gil_used = false)] pub(crate) mod kdf { #[pymodule_export] use super::{ - Argon2d, Argon2i, Argon2id, ConcatKdfHash, ConcatKdfHmac, Hkdf, HkdfExpand, Pbkdf2Hmac, - Scrypt, X963Kdf, + Argon2d, Argon2i, Argon2id, ConcatKdfHash, ConcatKdfHmac, Hkdf, HkdfExpand, KbkdfHmac, + Pbkdf2Hmac, Scrypt, X963Kdf, }; } diff --git a/src/rust/src/types.rs b/src/rust/src/types.rs index a1330da6baa2..30c5376f0f2e 100644 --- a/src/rust/src/types.rs +++ b/src/rust/src/types.rs @@ -602,6 +602,13 @@ pub static LEGACY_PROVIDER_LOADED: LazyPyImport = LazyPyImport::new( &["openssl", "_legacy_provider_loaded"], ); +pub static KBKDF_MODE: LazyPyImport = + LazyPyImport::new("cryptography.hazmat.primitives.kdf.kbkdf", &["Mode"]); +pub static KBKDF_COUNTER_LOCATION: LazyPyImport = LazyPyImport::new( + "cryptography.hazmat.primitives.kdf.kbkdf", + &["CounterLocation"], +); + #[cfg(test)] mod tests { use super::LazyPyImport; diff --git a/tests/hazmat/primitives/test_kbkdf.py b/tests/hazmat/primitives/test_kbkdf.py index 900c7664bb4b..f3af568da065 100644 --- a/tests/hazmat/primitives/test_kbkdf.py +++ b/tests/hazmat/primitives/test_kbkdf.py @@ -4,6 +4,7 @@ import re +import sys import pytest @@ -112,11 +113,27 @@ def test_already_finalized(self, backend): with pytest.raises(AlreadyFinalized): kdf.verify(b"material", key) - def test_key_length(self, backend): + def test_derive_into(self, backend): kdf = KBKDFHMAC( - hashes.SHA1(), + hashes.SHA256(), Mode.CounterMode, - 85899345920, + 32, + 4, + 4, + CounterLocation.BeforeFixed, + b"label", + b"context", + None, + backend=backend, + ) + buf = bytearray(32) + n = kdf.derive_into(b"material", buf) + assert n == 32 + # Verify the output matches what derive would produce + kdf2 = KBKDFHMAC( + hashes.SHA256(), + Mode.CounterMode, + 32, 4, 4, CounterLocation.BeforeFixed, @@ -125,9 +142,60 @@ def test_key_length(self, backend): None, backend=backend, ) + expected = kdf2.derive(b"material") + assert buf == expected - with pytest.raises(ValueError): - kdf.derive(b"material") + @pytest.mark.parametrize(("buflen", "outlen"), [(31, 32), (33, 32)]) + def test_derive_into_buffer_incorrect_size(self, buflen, outlen, backend): + kdf = KBKDFHMAC( + hashes.SHA256(), + Mode.CounterMode, + outlen, + 4, + 4, + CounterLocation.BeforeFixed, + b"label", + b"context", + None, + backend=backend, + ) + buf = bytearray(buflen) + with pytest.raises(ValueError, match="buffer must be"): + kdf.derive_into(b"material", buf) + + def test_derive_into_already_finalized(self, backend): + kdf = KBKDFHMAC( + hashes.SHA256(), + Mode.CounterMode, + 32, + 4, + 4, + CounterLocation.BeforeFixed, + b"label", + b"context", + None, + backend=backend, + ) + buf = bytearray(32) + kdf.derive_into(b"material", buf) + with pytest.raises(AlreadyFinalized): + kdf.derive_into(b"material2", buf) + + def test_key_length(self, backend): + error = OverflowError if sys.maxsize <= 2**31 else ValueError + with pytest.raises(error): + KBKDFHMAC( + hashes.SHA1(), + Mode.CounterMode, + 85899345920, + 4, + 4, + CounterLocation.BeforeFixed, + b"label", + b"context", + None, + backend=backend, + ) def test_rlen(self, backend): with pytest.raises(ValueError): @@ -302,27 +370,7 @@ def test_keyword_only_break_location(self, backend): ) def test_invalid_break_location(self, backend): - with pytest.raises( - TypeError, match=re.escape("break_location must be an integer") - ): - KBKDFHMAC( - hashes.SHA256(), - Mode.CounterMode, - 32, - 4, - 4, - CounterLocation.MiddleFixed, - b"label", - b"context", - None, - backend=backend, - break_location="0", # type: ignore[arg-type] - ) - - with pytest.raises( - ValueError, - match=re.escape("break_location must be a positive integer"), - ): + with pytest.raises(OverflowError): KBKDFHMAC( hashes.SHA256(), Mode.CounterMode, @@ -402,7 +450,7 @@ def test_ignored_break_location_after(self, backend): def test_unsupported_hash(self, backend): with raises_unsupported_algorithm(_Reasons.UNSUPPORTED_HASH): KBKDFHMAC( - object(), # type: ignore[arg-type] + DummyHashAlgorithm(), Mode.CounterMode, 32, 4,