Skip to content

Add support for zigbee algorithms#94

Merged
athoelke merged 7 commits into
ARM-software:mainfrom
athoelke:crypto-zigbee
Jan 17, 2024
Merged

Add support for zigbee algorithms#94
athoelke merged 7 commits into
ARM-software:mainfrom
athoelke:crypto-zigbee

Conversation

@athoelke
Copy link
Copy Markdown
Contributor

Add support for algorithms required by zigbee:

  • MMO hash construction over AES-128
  • CCM* optionally-authenticated variant of CCM

This PR adds a new hash function, and an unauthenticated cipher for the zero-length tag variant of CCM*. For non-zero-length tags, the existing support for CCM is adequate.

Fixes #14
Fixes #15

@athoelke athoelke added enhancement New feature or request Crypto API Issue or PR related to the Cryptography API labels Aug 10, 2023
@athoelke athoelke added this to the Crypto API 1.2 milestone Aug 10, 2023
Copy link
Copy Markdown
Contributor

@MarcusJGStreets MarcusJGStreets left a comment

Choose a reason for hiding this comment

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

Looks OK

@athoelke
Copy link
Copy Markdown
Contributor Author

Rebased this PR.

Last call for feedback, before this is merged to prepare for v1.2 release candidate.

Copy link
Copy Markdown
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

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

Correctness: looks good to me. I've checked against the Zigbee specification and the Mbed TLS draft implementation of CCM_STAR_NO_TAG, but not against the IEEE CCM* specification.

Writing: I left a couple of requests and suggestions.

Comment thread doc/crypto/api/ops/aead.rst Outdated

The CCM block cipher mode is defined in :RFC-title:`3610`.

.. subsection:: Usage in zigbee
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Editorial: “Zigbee” should be capitalized. (It used to be “ZigBee”, but that stopped years ago; permanently-uncapitalized “zigbee” seems to be the organization logo, not the name of the protocol suite).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've corrected all references except the document citation: the spec itself uses the uncapitalized form for its title and publisher.

Comment thread doc/crypto/api/ops/ciphers.rst Outdated

* For unauthenticated messages — when *M* = 0 --- the `PSA_ALG_CCM_STAR_NO_TAG` algorithm is used with an AES-128 key in a multi-part cipher operation. The 13-byte IV must be constructed as specified in `[ZIGBEE]`, and provided to the operation using `psa_cipher_set_iv()`.

An implementation of zigbee cannot use the single-part cipher functions, as these generate a random IV, which is not valid for the zigbee protocol.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Single-part encryption is out due to the non-random IV, but what's wrong with single-part decryption?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Although, technically, the single-part decryption function can be used for an implementation of Zigbee, in practice requires that the caller concatenate the 13-byte nonce and the cipher-text into a single memory buffer before calling the API.

But maybe there is no harm in making the explanatory text more precise.

Comment thread doc/crypto/api/ops/aead.rst Outdated

.. subsection:: Usage in zigbee

`PSA_ALG_CCM`, and its truncated variants, can be used to implement CCM* for non-zero tag lengths. CCM* is required by the :cite-title:`ZIGBEE`. For unauthenticated CCM*, the `PSA_ALG_CCM_STAR_NO_TAG` cipher algorithm can be used.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please add a hyperlink to the discussion of Zigbee usage under PSA_ALG_CCM_STAR_NO_TAG, since that discussion covers both CCM and CCM*-no-tag.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done


An implementation of zigbee cannot use the single-part cipher functions, as these generate a random IV, which is not valid for the zigbee protocol.

* For authenticated messages — when *M* ∈ {4, 8, 16} --- the :code:`PSA_ALG_AEAD_WITH_SHORTENED_TAG(PSA_ALG_CCM, tag_length)` algorithm is used with an AES-128 key, where ``tag_length`` is the required value of *M*. The 13-byte nonce must be constructed as specified in `[ZIGBEE]`.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please also list PSA_ALG_CCM as usable when the tag length is 16. It's logically redundant, but not didactically redundant.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread doc/crypto/api/ops/hashes.rst Outdated
RIPEMD-160 is defined in :cite-title:`RIPEMD`, and also in :cite-title:`ISO10118`.

.. macro:: PSA_ALG_AES_MMO_ZIGBEE
:definition: ((psa_algorithm_t)0x02000006)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor: it might conceivably make sense to define a SHA1 variant that forbids inputs which might be exploiting known collision weaknesses, due to SHA1's persisting use in protocols such as git. (Currently we aren't in a hurry to do that because that algorithm is not used in our typical domain space, and can't be implemented on constrained devices due to the high code size.) If we ever do that then it would be slightly more convenient to use hash 6 (since SHA-1 is 5). So MMO_ZIGBEE could be 7 instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok - we can recode this as 0x02000007.

This was not already defined in Mbed TLS, unlike PSA_ALG_CCM_START_NO_TAG.

Comment thread doc/crypto/api/ops/hashes.rst Outdated
:definition: ((psa_algorithm_t)0x02000006)

.. summary::
The *zigbee* 1.0 hash function based on a Matyas-Meyer-Oseas (MMO) construction of AES-128.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Editorial: “construction on AES-128”? over AES-128?

Copy link
Copy Markdown
Contributor Author

@athoelke athoelke Jan 16, 2024

Choose a reason for hiding this comment

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

Decided on 'using', seems to make sense with the rewording of the full description.

Comment thread doc/crypto/api/ops/hashes.rst Outdated
.. summary::
The *zigbee* 1.0 hash function based on a Matyas-Meyer-Oseas (MMO) construction of AES-128.

zigbee specifies a cryptographic hash function based on the MMO hash construction using the AES-128 block cipher. This is defined in :cite-title:`ZIGBEE` §B.6.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

“MMO construction using AES-128” is ambiguous since this is a theory-level definition which leaves some parameters open (padding, IV). The formulation here doesn't make it very clear that we mean the specific parametrization in the Zigbee specification. I suggest something like

This is the cryptographic hash function based on the Merkle–Damgård construction over a Matyas–Meyer–Oseas one-way compression function and the AES-128 block cipher with the parametrization defined in :cite-title:`ZIGBEE` §B.6.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you - I have adopted the revised wording.

Copy link
Copy Markdown
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

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

LGTM

@athoelke
Copy link
Copy Markdown
Contributor Author

Rebased for merge

@athoelke athoelke merged commit 83cec43 into ARM-software:main Jan 17, 2024
@athoelke athoelke deleted the crypto-zigbee branch January 18, 2024 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Crypto API Issue or PR related to the Cryptography API enhancement New feature or request

Projects

Development

Successfully merging this pull request may close these issues.

New algorithm: Zigbee's encryption and authentication block-cipher mode (CCM*) New algorithm: Zigbee's block-cipher based hash (AES-MMO)

3 participants