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.

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

Additional cipher_info getters #5149

Conversation

mfil
Copy link
Contributor

@mfil mfil commented Nov 10, 2021

Description

While trying to add mbedtls 3 support to OpenVPN, we found that we need some additional getter functions that are not yet in the dev branch. This pull request adds functions that get the IV and block size from a cipher_info struct and functions that check if the IV or key size of a cipher are variable.

We also need some more getter functions in other parts of the library, but I figured it's easier if I keep the pull requests small.

Status

READY

Requires Backporting

NO

Migrations

NO

Todos

  • Tests
  • Documentation
  • Changelog updated

mfil added 2 commits November 10, 2021 14:20
Signed-off-by: Max Fillinger <max@max-fillinger.net>
Signed-off-by: Max Fillinger <max@max-fillinger.net>
@mfil mfil force-pushed the feature/additional_cipher_info_getters branch from 0e9df7f to 01bd69b Compare November 10, 2021 13:51
mfil added 2 commits November 10, 2021 15:12
Signed-off-by: Max Fillinger <max@max-fillinger.net>
Signed-off-by: Max Fillinger <max@max-fillinger.net>
@mfil mfil force-pushed the feature/additional_cipher_info_getters branch from 01bd69b to 7568d1a Compare November 10, 2021 14:12
@gabor-mezei-arm gabor-mezei-arm added Community component-crypto Crypto primitives and low-level interfaces enhancement needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Nov 11, 2021
@gabor-mezei-arm gabor-mezei-arm linked an issue Nov 16, 2021 that may be closed by this pull request
Copy link
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.

Looks mostly good to me (and thanks for including tests!), except that I think some types are wrong and some documentation is missing.

include/mbedtls/cipher.h Outdated Show resolved Hide resolved
include/mbedtls/cipher.h Show resolved Hide resolved
@gilles-peskine-arm gilles-peskine-arm added needs-work and removed needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Nov 18, 2021
mfil added 2 commits November 21, 2021 16:33
Signed-off-by: Max Fillinger <max@max-fillinger.net>
- Document unit (bytes)
- Explain what happens for stream ciphers

Signed-off-by: Max Fillinger <max@max-fillinger.net>
@mfil mfil force-pushed the feature/additional_cipher_info_getters branch from 66acc93 to e85bb70 Compare November 21, 2021 15:33
@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review and removed needs-work labels Nov 23, 2021
Copy link
Contributor

@davidhorstmann-arm davidhorstmann-arm left a comment

Choose a reason for hiding this comment

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

A couple of nits, but looks good otherwise.

include/mbedtls/cipher.h Show resolved Hide resolved
tests/suites/test_suite_cipher.function Outdated Show resolved Hide resolved
@daverodgman daverodgman removed the needs-reviewer This PR needs someone to pick it up for review label Nov 24, 2021
@daverodgman daverodgman added needs-work and removed needs-review Every commit must be reviewed by at least two team members, labels Nov 24, 2021
mfil added 2 commits November 28, 2021 14:13
Signed-off-by: Max Fillinger <max@max-fillinger.net>
Signed-off-by: Max Fillinger <max@max-fillinger.net>
@mfil mfil force-pushed the feature/additional_cipher_info_getters branch from 92886bb to 72abd8a Compare November 28, 2021 13:14
Copy link
Contributor

@davidhorstmann-arm davidhorstmann-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 - thanks!

@gilles-peskine-arm gilles-peskine-arm added approved Design and code approved - may be waiting for CI or backports and removed needs-work labels Dec 3, 2021
@gilles-peskine-arm gilles-peskine-arm merged commit 1bbf6d6 into Mbed-TLS:development Dec 3, 2021
@mfil mfil deleted the feature/additional_cipher_info_getters branch December 6, 2021 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports component-crypto Crypto primitives and low-level interfaces enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mbedtls_cipher_info_t private fields getters
5 participants