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

Add warnings & disable PKCS #7 by default #6709

Merged
merged 4 commits into from
Dec 5, 2022

Conversation

daverodgman
Copy link
Contributor

@daverodgman daverodgman commented Dec 1, 2022

Disable PKCS 7 by default; add warnings to mbedtls_config.h and pkcs7.h.

Also clarify documentation for mbedtls_pkcs7_signed_xxx_verify() - this fixes #6692 .

Gatekeeper checklist

  • changelog provided, or not required
  • backport done, or not required
  • tests provided, or not required

Signed-off-by: Dave Rodgman <[email protected]>
Signed-off-by: Dave Rodgman <[email protected]>
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.

I wouldn't mind making the warning stronger, especially in mbedtls_config.h which I fear suggests only a verify-doesn't-enforce-all-policies kind of not ready, whereas we have a parser-risks-memory-corruption level of not ready. But I'm ok with the current wording.

Copy link
Contributor

@tom-cosgrove-arm tom-cosgrove-arm left a comment

Choose a reason for hiding this comment

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

I would also prefer a stronger warning, but not going to block this

@tom-cosgrove-arm tom-cosgrove-arm added approved Design and code approved - may be waiting for CI or backports priority-high High priority - will be reviewed soon labels Dec 1, 2022
Co-authored-by: Tom Cosgrove <[email protected]>
Signed-off-by: Dave Rodgman <[email protected]>
Copy link
Contributor

@tom-cosgrove-arm tom-cosgrove-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

@daverodgman daverodgman added the needs-ci Needs to pass CI tests label Dec 5, 2022
@daverodgman
Copy link
Contributor Author

The only CI failures are ABI compatibility, because PKCS 7 is now disabled by default.

@daverodgman daverodgman merged commit 80f8c67 into Mbed-TLS:development Dec 5, 2022
@daverodgman daverodgman deleted the pkcs7-docs branch December 5, 2022 16:24
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 needs-ci Needs to pass CI tests priority-high High priority - will be reviewed soon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PKCS7 API does not verify certificates
3 participants