Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

KBKDF: add CounterLocation.MiddleFixed #7489

Merged
merged 10 commits into from
Aug 15, 2022
Merged

KBKDF: add CounterLocation.MiddleFixed #7489

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4288ba4
KBKDF: add CounterLocation.MiddleFixed
jeanpaulgalea Aug 9, 2022
726a9ac
tests_kbkdf: add new tests
jeanpaulgalea Aug 10, 2022
c49a9da
test_kbkdf_vectors: make use of MIDDLE_FIXED
jeanpaulgalea Aug 10, 2022
8f0f8f9
docs: document KBKDF CounterLocation.MiddleFixed
jeanpaulgalea Aug 10, 2022
94241e4
s/blocation/break_location/g
jeanpaulgalea Aug 10, 2022
bef83c8
CHANGELOG: document KBKDF{HMAC,CMAC} CounterLocation.MiddleFixed #7489
jeanpaulgalea Aug 11, 2022
fb3ff34
kbkdf: fixed is static across n iterations, only the counter changes
jeanpaulgalea Aug 11, 2022
b9e1357
kbkdf: check break_location is 0..len(fixed)
jeanpaulgalea Aug 11, 2022
ba4ea8c
kbkdf: keyword-only break_location
jeanpaulgalea Aug 15, 2022
8b0737f
docs: document kw-only KBKDF{HMAC,CMAC}
jeanpaulgalea Aug 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ Changelog
* Added :attr:`~cryptography.x509.Certificate.tbs_precertificate_bytes`, allowing
users to access the to-be-signed pre-certificate data needed for signed
certificate timestamp verification.
* :class:`~cryptography.hazmat.primitives.kdf.kbkdf.KBKDFHMAC` and
:class:`~cryptography.hazmat.primitives.kdf.kbkdf.KBKDFCMAC` now support
:attr:`~cryptography.hazmat.primitives.kdf.kbkdf.CounterLocation.MiddleFixed`
counter location.

.. _v37-0-4:

Expand Down
31 changes: 27 additions & 4 deletions docs/hazmat/primitives/key-derivation-functions.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,12 +670,20 @@ KBKDF
may supply your own fixed data. If ``fixed`` is specified, ``label``
and ``context`` is ignored.

:param int break_location: A keyword-only argument. An integer that
indicates the bytes offset where counter bytes are to be located.
Required when ``location`` is
:attr:`~cryptography.hazmat.primitives.kdf.kbkdf.CounterLocation.MiddleFixed`.

:raises TypeError: This exception is raised if ``label`` or ``context``
is not ``bytes``. Also raised if ``rlen`` or ``llen`` is not ``int``.
is not ``bytes``. Also raised if ``rlen``, ``llen``, or
``break_location`` is not ``int``.

:raises ValueError: This exception is raised if ``rlen`` or ``llen``
is greater than 4 or less than 1. This exception is also raised if
you specify a ``label`` or ``context`` and ``fixed``.
you specify a ``label`` or ``context`` and ``fixed``. This exception
is also raised if you specify ``break_location`` and ``location`` is not
:attr:`~cryptography.hazmat.primitives.kdf.kbkdf.CounterLocation.MiddleFixed`.

.. method:: derive(key_material)

Expand Down Expand Up @@ -788,20 +796,28 @@ KBKDF
may supply your own fixed data. If ``fixed`` is specified, ``label``
and ``context`` is ignored.

:param int break_location: A keyword-only argument. An integer that
indicates the bytes offset where counter bytes are to be located.
Required when ``location`` is
:attr:`~cryptography.hazmat.primitives.kdf.kbkdf.CounterLocation.MiddleFixed`.

:raises cryptography.exceptions.UnsupportedAlgorithm: This is raised
if ``algorithm`` is not a subclass of
:class:`~cryptography.hazmat.primitives.ciphers.CipherAlgorithm` and
:class:`~cryptography.hazmat.primitives.ciphers.BlockCipherAlgorithm`.

:raises TypeError: This exception is raised if ``label`` or ``context``
is not ``bytes``, ``rlen`` or ``llen`` is not ``int``, ``mode`` is not
is not ``bytes``, ``rlen``, ``llen``, or ``break_location` is not
``int``, ``mode`` is not
:class:`~cryptography.hazmat.primitives.kdf.kbkdf.Mode` or ``location``
is not
:class:`~cryptography.hazmat.primitives.kdf.kbkdf.CounterLocation`.

:raises ValueError: This exception is raised if ``rlen`` or ``llen``
is greater than 4 or less than 1. This exception is also raised if
you specify a ``label`` or ``context`` and ``fixed``.
you specify a ``label`` or ``context`` and ``fixed``. This exception
is also raised if you specify ``break_location`` and ``location`` is not
:attr:`~cryptography.hazmat.primitives.kdf.kbkdf.CounterLocation.MiddleFixed`.

.. method:: derive(key_material)

Expand Down Expand Up @@ -861,6 +877,13 @@ KBKDF
The counter iteration variable will be concatenated after
the fixed input data.

.. attribute:: MiddleFixed
reaperhulk marked this conversation as resolved.
Show resolved Hide resolved

.. versionadded:: 38.0

The counter iteration variable will be concatenated in the middle
of the fixed input data.


X963KDF
-------
Expand Down
51 changes: 45 additions & 6 deletions src/cryptography/hazmat/primitives/kdf/kbkdf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class Mode(utils.Enum):
class CounterLocation(utils.Enum):
BeforeFixed = "before_fixed"
AfterFixed = "after_fixed"
MiddleFixed = "middle_fixed"


class _KBKDFDeriver:
Expand All @@ -39,6 +40,7 @@ def __init__(
rlen: int,
llen: typing.Optional[int],
location: CounterLocation,
break_location: typing.Optional[int],
label: typing.Optional[bytes],
context: typing.Optional[bytes],
fixed: typing.Optional[bytes],
Expand All @@ -51,6 +53,24 @@ def __init__(
if not isinstance(location, CounterLocation):
raise TypeError("location must be of type CounterLocation")

if break_location is None and location is CounterLocation.MiddleFixed:
raise ValueError("Please specify a break_location")

if (
break_location is not None
and location != CounterLocation.MiddleFixed
):
raise ValueError(
"break_location is ignored when location is not"
" CounterLocation.MiddleFixed"
)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to check that blocation is within 0..len(fixed or label+context) ?

In its present state, it's allowed to set blocation=-1 or =1000. This works fine with fixed[blocation:] and fixed[:blocation] further below, but not sure how pedantic we should be.

Also not sure if you prefer to require location=BeforeFixed instead of allowing location=MiddleFixed && blocation=0, and similarly location=AfterFixed instead of location=MiddleFixed && blocation=len(fixed or label+context).

My personal preference is to do none of these length checks and leave it up to the user, but obviously it's your call.

if break_location is not None and not isinstance(break_location, int):
raise TypeError("break_location must be an integer")

if break_location is not None and break_location < 0:
raise ValueError("break_location must be a positive integer")
reaperhulk marked this conversation as resolved.
Show resolved Hide resolved

if (label or context) and fixed:
raise ValueError(
"When supplying fixed data, " "label and context are ignored."
Expand Down Expand Up @@ -79,6 +99,7 @@ def __init__(
self._rlen = rlen
self._llen = llen
self._location = location
self._break_location = break_location
self._label = label
self._context = context
self._used = False
Expand Down Expand Up @@ -114,17 +135,29 @@ def derive(self, key_material: bytes, prf_output_size: int) -> bytes:
if rounds > pow(2, len(r_bin) * 8) - 1:
raise ValueError("There are too many iterations.")

fixed = self._generate_fixed_input()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be fine to move outside the loop, the fixed input doesn't change across iterations, except for the counter value.


if self._location == CounterLocation.BeforeFixed:
data_before_ctr = b""
data_after_ctr = fixed
elif self._location == CounterLocation.AfterFixed:
data_before_ctr = fixed
data_after_ctr = b""
else:
if isinstance(
self._break_location, int
) and self._break_location > len(fixed):
raise ValueError("break_location offset > len(fixed)")
reaperhulk marked this conversation as resolved.
Show resolved Hide resolved
data_before_ctr = fixed[: self._break_location]
data_after_ctr = fixed[self._break_location :]

for i in range(1, rounds + 1):
h = self._prf(key_material)

counter = utils.int_to_bytes(i, self._rlen)
if self._location == CounterLocation.BeforeFixed:
h.update(counter)

h.update(self._generate_fixed_input())
input_data = data_before_ctr + counter + data_after_ctr

if self._location == CounterLocation.AfterFixed:
h.update(counter)
h.update(input_data)

output.append(h.finalize())

Expand Down Expand Up @@ -152,6 +185,8 @@ def __init__(
context: typing.Optional[bytes],
fixed: typing.Optional[bytes],
backend: typing.Any = None,
*,
break_location: typing.Optional[int] = None,
):
if not isinstance(algorithm, hashes.HashAlgorithm):
raise UnsupportedAlgorithm(
Expand All @@ -178,6 +213,7 @@ def __init__(
rlen,
llen,
location,
break_location,
label,
context,
fixed,
Expand Down Expand Up @@ -207,6 +243,8 @@ def __init__(
context: typing.Optional[bytes],
fixed: typing.Optional[bytes],
backend: typing.Any = None,
*,
break_location: typing.Optional[int] = None,
):
if not issubclass(
algorithm, ciphers.BlockCipherAlgorithm
Expand All @@ -226,6 +264,7 @@ def __init__(
rlen,
llen,
location,
break_location,
label,
context,
fixed,
Expand Down