Remove PKCS12 support #269
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Taowyoo
requested changes
May 22, 2023
There was a problem hiding this comment.
Tests need to be cleaned in ct.sh & mbedtls/tests/pbkdf.rs
I include changes in this this PR in #213 , and you could cherry-pick these two commits to cleanup tests:
bors bot
added a commit
that referenced
this pull request
Jun 26, 2023
278: Upgrade `mbedtls` to 3.4.0 r=Taowyoo a=Taowyoo ## Overview [PR #213](#213) introduces some breaking changes: - Update vendor `mbedtls` code from version `2.28.3` to a `3.4.0` - Please checkout C `mbedtls` migration guide for 3.X here [3.0-migration-guide.md](https://github.com/fortanix/rust-mbedtls/blob/yx/upgrade-mbedtls/mbedtls-sys/vendor/docs/3.0-migration-guide.md) to - Update the build code for `mbedtls-sys-auto` crate to sync up with vendor code change: - Changes in C DEFINE's for configuration - Changes in header files - Add binding code generation for `static inline` code in C side - Removing deprecated cargo features or dependencies - Add a cargo feature for TLS 1.3 - Update rust wrapper code in `./mbedtls` to sync up the changes in `mbedtls-sys-auto` - Removing deprecated cargo features or dependencies - Update rust code to sync up API changes - Add types and functions for TLS 1.3 - Add a cargo feature for TLS 1.3 - Update integration tests for new API and TLS 1.3 - Update dependencies ## Breaking Changes: **Note**: entries with 💥 are ensured that they will break the downstream. ### Changes in `mbedtls-sys-auto` #### Vendor code changes **Upstream changes:** - 💥 Upgrade vendor `mbeldtls` code to `3.4.0` **Changes on our side:** - Cherry picked previous changes in old versions: - commit: [vendor change: Adding mpi_force_c_code feature](c8cd406) - New changes - [vendor change: fix time call in tls13 client&server](bafc52d) : This has been merged into upstream, see: Mbed-TLS/mbedtls#7639 . #### rust code changes **Features:** - 💥 `zlib` is removed: support for TLS record-level compression is removed in `mbedtls` 3.X - Related C DEFINE `MBEDTLS_ZLIB_SUPPORT` is also removed - 💥 `legacy_protocols` is removed: all protocols early than TLS 1.2 is removed in `mbedtls` 3.X - Related C DEFINE's are also removed: `MBEDTLS_SSL_PROTO_SSL3`, `MBEDTLS_SSL_PROTO_TLS1`, `MBEDTLS_SSL_PROTO_TLS1_1`, `MBEDTLS_SSL_CBC_RECORD_SPLITTING` - 💥 `pkcs11` is removed: wrapper for `libpkcs11-helper` is removed in `mbedtls` 3.X, see [3.0-migration-guide.md](https://github.com/fortanix/rust-mbedtls/blob/yx/upgrade-mbedtls/mbedtls-sys/vendor/docs/3.0-migration-guide.md#remove-wrapper-for-libpkcs11-helper) - 💥 ~~`pkcs12` is removed: because #269 `pkcs12` still defined since it is needed for reading some pkcs8 format keys - Put TLS 1.3 behind a feature `tls13` : because the dependency of TLS 1.3 in mbedtls 3.X are using a global state RNG which breaks the requirements for FIPS, so this feature enables use to avoid these code from compilation. - 💥 Deprecated features `custom_threading` , `custom_time` , `custom_gmtime_r` , `pthread` are removed **Dependencies:** - 💥 Bump `mbedtls-sys` version to `3.4.0` - 💥 Deprecated dependencies are removed - `libz-sys` : support for TLS record-level compression is removed in `mbedtls` 3.X - `libc`: `libc` is not needed in `sgx` **Build code changes:** Following changes are made according to [3.0-migration-guide.md](https://github.com/fortanix/rust-mbedtls/blob/yx/upgrade-mbedtls/mbedtls-sys/vendor/docs/3.0-migration-guide.md). - Remove `MBEDTLS_CONFIG_H` in `mbedtls_config.h` - Remove `#include <mbedtls/check_config.h>` - `mbedtls-sys/build/bindgen.rs` - Allow `bindgen` to generate bindings for functions, types and variables start with `psa_`, and put them in a sub `mod psa` because they are needed by TLS 1.3 - Use `bindgen` experiment feature to generate C function wrapper for C `static inline` functions - `mbedtls-sys/build/headers.rs` : Update header files - `mbedtls-sys/build/config.rs` : Remove/add C defines - 💥 Added `MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG` : this is added because TLS 1.3 need to use PSA library which need this when feature `std` is off, user need to provide their implementation. In `rus-mbedtls`, one implementation is proveded, see: `mbedtls/src/rng/mod.rs`. ### Changes in `mbedtls-platform-support` - Added `once_cell` for initializing PSA only one times: see `fn psa_crypto_init()` in `mbedtls-platform-support/src/lib.rs`, this is needed because: - > MBEDTLS_USE_PSA_CRYPTO means that X.509 and TLS will use PSA Crypto as much as possible (that is, everywhere except for features that are not supported by PSA Crypto, see "Internal Changes" below for a complete list of exceptions). When it is enabled, you need to call psa_crypto_init() before calling any function from PK, X.509 or TLS; - Ref: https://github.com/Mbed-TLS/mbedtls/blob/0b3de6fcec4aa4b23a9ee1e076714cbc796f3ac4/docs/use-psa-crypto.md#general-considerations - Add function pointer `mbedtls_psa_external_get_random` which is needed when C DEFINE option `MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG` is turned on. This will be used in case when system default RNG or entropy is no available (for example in SGX) - Using function pointer here ensure there is no link time conflict in future when mutiple `rust-mbedtls` is using this crate. - User need to call function `set_psa_external_rng_callback` before using any PSA functions or TLS 1.3 - Add rust implementation of `explicit_bzero`, which is needed in SGX. Because in SGX, our [rs-libc](https://github.com/fortanix/rust-sgx/tree/master/rs-libc) does not support this function. - Update self tests: - following are removed: - arc4_self_test - md2_self_test - md4_self_test - x509_self_test - xtea_self_test - following are added: - sha384_self_test - chacha20_self_test - chachapoly_self_test - poly1305_self_test - sha224_self_test ### Changes in `mbedtls` **Features:** - 💥 `zlib`, `legacy_protocols` removed to sync with changes in `mbedtls-sys` - 💥 `pkcs12` and `pkcs12_rc2` are removed: see #269 - Put TLS 1.3 logic behind a feature `tls13`: check reason above **Dependencies:** - 💥 Bump `rust-mbedtls` version to `0.11.0` - 💥 Bump dependency `mbedtls-sys-auto` version to `3.4.0` - Added `rstest` `rstest_reuse` `lazy_static` `env_logger`: used for improving code of tests ### Code changes - Function `EcPoint::mul` now need to pass in a RNG to ensure blinding. - Add prefix `private_` to some fields of `mbedtls` types under `mbedtls/src/cipher/raw` - 💥 Removed and added some options in `Error`, `CipherType`, `CipherMode` and `CipherSuite` to sync with changes in `mbedtls` 3.X - 💥 `mbedtls/src/pk/ec.rs` : User need to provide a RNG callback for function `EcPoint::mul`, this originally is not a hard requirement, but in C `mbedtls` 3.X this become a hard requirement for blinding to defend side channel attack. - 💥 `mbedtls/hash` : - `Md2` and `Md4` are removed since they are no longer supported in `mbedtls` 3.X - fn `pbkdf_pkcs12` is removed since `pkcs12` is removed - 💥 `mbedtls/pk/mod.rs` : - Remove `CustomPk` - User need to provide a RNG callback for `Pk::from_private_key`, this originally is not a hard requirement, but in C `mbedtls` 3.X this become a hard requirement for blinding to defend side channel attack. - 💥 `mbedtls/src/ssl/ciphersuites.rs`: Rename `TlsCipherSuite` to `Tls12CipherSuite`, and add enum: `Tls13CipherSuite`, `IanaTlsNamedGroup`, `TLS13SignatureAlgorithms`: these are introduced by TLS 1.3 - `mbedtls/src/ssl/ssl_states.rs`: Add `SslStates` to represent the state of SSL handshake - Update tests accordingly **Special code need to notice**: In `impl` of `std::io::Read` under `mbedtls/src/ssl/io.rs` and `tokio::io::AsyncRead` under `mbedtls/src/ssl/async_io.rs`, there are some code to handle the special case when using `mbedtls` as `client` to connect to a server whose `session ticket` extension is enabled. This case is found when connecting to `goolge.com`, where Google's server send the `session ticket` after the completion of handshake, which cause `C-mbedtls` throw errors when client is try to read msg data. ## CI changes - Use [cargo-nextest](https://nexte.st/#cargo-nextest) to run tests - Reduce time to run tests - Have ability to run some tests in serial - tests under `hyper.rs` need to access to `google.com` which has QPS limit - some tests function use some system resource, see https://github.com/fortanix/rust-mbedtls/blob/yx/upgrade-mbedtls/mbedtls/tests/support/net.rs Co-authored-by: Yuxiang Cao <[email protected]> Co-authored-by: Vardhan Thigle <[email protected]> Co-authored-by: Raoul Strackx <[email protected]>
bors bot
added a commit
that referenced
this pull request
Jun 26, 2023
278: Upgrade `mbedtls` to 3.4.0 r=Taowyoo a=Taowyoo ## Overview [PR #213](#213) introduces some breaking changes: - Update vendor `mbedtls` code from version `2.28.3` to a `3.4.0` - Please checkout C `mbedtls` migration guide for 3.X here [3.0-migration-guide.md](https://github.com/fortanix/rust-mbedtls/blob/yx/upgrade-mbedtls/mbedtls-sys/vendor/docs/3.0-migration-guide.md) to - Update the build code for `mbedtls-sys-auto` crate to sync up with vendor code change: - Changes in C DEFINE's for configuration - Changes in header files - Add binding code generation for `static inline` code in C side - Removing deprecated cargo features or dependencies - Add a cargo feature for TLS 1.3 - Update rust wrapper code in `./mbedtls` to sync up the changes in `mbedtls-sys-auto` - Removing deprecated cargo features or dependencies - Update rust code to sync up API changes - Add types and functions for TLS 1.3 - Add a cargo feature for TLS 1.3 - Update integration tests for new API and TLS 1.3 - Update dependencies ## Breaking Changes: **Note**: entries with 💥 are ensured that they will break the downstream. ### Changes in `mbedtls-sys-auto` #### Vendor code changes **Upstream changes:** - 💥 Upgrade vendor `mbeldtls` code to `3.4.0` **Changes on our side:** - Cherry picked previous changes in old versions: - commit: [vendor change: Adding mpi_force_c_code feature](c8cd406) - New changes - [vendor change: fix time call in tls13 client&server](bafc52d) : This has been merged into upstream, see: Mbed-TLS/mbedtls#7639 . #### rust code changes **Features:** - 💥 `zlib` is removed: support for TLS record-level compression is removed in `mbedtls` 3.X - Related C DEFINE `MBEDTLS_ZLIB_SUPPORT` is also removed - 💥 `legacy_protocols` is removed: all protocols early than TLS 1.2 is removed in `mbedtls` 3.X - Related C DEFINE's are also removed: `MBEDTLS_SSL_PROTO_SSL3`, `MBEDTLS_SSL_PROTO_TLS1`, `MBEDTLS_SSL_PROTO_TLS1_1`, `MBEDTLS_SSL_CBC_RECORD_SPLITTING` - 💥 `pkcs11` is removed: wrapper for `libpkcs11-helper` is removed in `mbedtls` 3.X, see [3.0-migration-guide.md](https://github.com/fortanix/rust-mbedtls/blob/yx/upgrade-mbedtls/mbedtls-sys/vendor/docs/3.0-migration-guide.md#remove-wrapper-for-libpkcs11-helper) - 💥 ~~`pkcs12` is removed: because #269 `pkcs12` still defined since it is needed for reading some pkcs8 format keys - Put TLS 1.3 behind a feature `tls13` : because the dependency of TLS 1.3 in mbedtls 3.X are using a global state RNG which breaks the requirements for FIPS, so this feature enables use to avoid these code from compilation. - 💥 Deprecated features `custom_threading` , `custom_time` , `custom_gmtime_r` , `pthread` are removed **Dependencies:** - 💥 Bump `mbedtls-sys` version to `3.4.0` - 💥 Deprecated dependencies are removed - `libz-sys` : support for TLS record-level compression is removed in `mbedtls` 3.X - `libc`: `libc` is not needed in `sgx` **Build code changes:** Following changes are made according to [3.0-migration-guide.md](https://github.com/fortanix/rust-mbedtls/blob/yx/upgrade-mbedtls/mbedtls-sys/vendor/docs/3.0-migration-guide.md). - Remove `MBEDTLS_CONFIG_H` in `mbedtls_config.h` - Remove `#include <mbedtls/check_config.h>` - `mbedtls-sys/build/bindgen.rs` - Allow `bindgen` to generate bindings for functions, types and variables start with `psa_`, and put them in a sub `mod psa` because they are needed by TLS 1.3 - Use `bindgen` experiment feature to generate C function wrapper for C `static inline` functions - `mbedtls-sys/build/headers.rs` : Update header files - `mbedtls-sys/build/config.rs` : Remove/add C defines - 💥 Added `MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG` : this is added because TLS 1.3 need to use PSA library which need this when feature `std` is off, user need to provide their implementation. In `rus-mbedtls`, one implementation is proveded, see: `mbedtls/src/rng/mod.rs`. ### Changes in `mbedtls-platform-support` - Added `once_cell` for initializing PSA only one times: see `fn psa_crypto_init()` in `mbedtls-platform-support/src/lib.rs`, this is needed because: - > MBEDTLS_USE_PSA_CRYPTO means that X.509 and TLS will use PSA Crypto as much as possible (that is, everywhere except for features that are not supported by PSA Crypto, see "Internal Changes" below for a complete list of exceptions). When it is enabled, you need to call psa_crypto_init() before calling any function from PK, X.509 or TLS; - Ref: https://github.com/Mbed-TLS/mbedtls/blob/0b3de6fcec4aa4b23a9ee1e076714cbc796f3ac4/docs/use-psa-crypto.md#general-considerations - Add function pointer `mbedtls_psa_external_get_random` which is needed when C DEFINE option `MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG` is turned on. This will be used in case when system default RNG or entropy is no available (for example in SGX) - Using function pointer here ensure there is no link time conflict in future when mutiple `rust-mbedtls` is using this crate. - User need to call function `set_psa_external_rng_callback` before using any PSA functions or TLS 1.3 - Add rust implementation of `explicit_bzero`, which is needed in SGX. Because in SGX, our [rs-libc](https://github.com/fortanix/rust-sgx/tree/master/rs-libc) does not support this function. - Update self tests: - following are removed: - arc4_self_test - md2_self_test - md4_self_test - x509_self_test - xtea_self_test - following are added: - sha384_self_test - chacha20_self_test - chachapoly_self_test - poly1305_self_test - sha224_self_test ### Changes in `mbedtls` **Features:** - 💥 `zlib`, `legacy_protocols` removed to sync with changes in `mbedtls-sys` - 💥 `pkcs12` and `pkcs12_rc2` are removed: see #269 - Put TLS 1.3 logic behind a feature `tls13`: check reason above **Dependencies:** - 💥 Bump `rust-mbedtls` version to `0.11.0` - 💥 Bump dependency `mbedtls-sys-auto` version to `3.4.0` - Added `rstest` `rstest_reuse` `lazy_static` `env_logger`: used for improving code of tests ### Code changes - Function `EcPoint::mul` now need to pass in a RNG to ensure blinding. - Add prefix `private_` to some fields of `mbedtls` types under `mbedtls/src/cipher/raw` - 💥 Removed and added some options in `Error`, `CipherType`, `CipherMode` and `CipherSuite` to sync with changes in `mbedtls` 3.X - 💥 `mbedtls/src/pk/ec.rs` : User need to provide a RNG callback for function `EcPoint::mul`, this originally is not a hard requirement, but in C `mbedtls` 3.X this become a hard requirement for blinding to defend side channel attack. - 💥 `mbedtls/hash` : - `Md2` and `Md4` are removed since they are no longer supported in `mbedtls` 3.X - fn `pbkdf_pkcs12` is removed since `pkcs12` is removed - 💥 `mbedtls/pk/mod.rs` : - Remove `CustomPk` - User need to provide a RNG callback for `Pk::from_private_key`, this originally is not a hard requirement, but in C `mbedtls` 3.X this become a hard requirement for blinding to defend side channel attack. - 💥 `mbedtls/src/ssl/ciphersuites.rs`: Rename `TlsCipherSuite` to `Tls12CipherSuite`, and add enum: `Tls13CipherSuite`, `IanaTlsNamedGroup`, `TLS13SignatureAlgorithms`: these are introduced by TLS 1.3 - `mbedtls/src/ssl/ssl_states.rs`: Add `SslStates` to represent the state of SSL handshake - Update tests accordingly **Special code need to notice**: In `impl` of `std::io::Read` under `mbedtls/src/ssl/io.rs` and `tokio::io::AsyncRead` under `mbedtls/src/ssl/async_io.rs`, there are some code to handle the special case when using `mbedtls` as `client` to connect to a server whose `session ticket` extension is enabled. This case is found when connecting to `goolge.com`, where Google's server send the `session ticket` after the completion of handshake, which cause `C-mbedtls` throw errors when client is try to read msg data. ## CI changes - Use [cargo-nextest](https://nexte.st/#cargo-nextest) to run tests - Reduce time to run tests - Have ability to run some tests in serial - tests under `hyper.rs` need to access to `google.com` which has QPS limit - some tests function use some system resource, see https://github.com/fortanix/rust-mbedtls/blob/yx/upgrade-mbedtls/mbedtls/tests/support/net.rs Co-authored-by: Yuxiang Cao <[email protected]> Co-authored-by: Vardhan Thigle <[email protected]> Co-authored-by: Raoul Strackx <[email protected]>
|
Done by #213 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PKCS#12 is awful. It's a very complicated spec that's not inline with modern cryptographic best practices. It's common for implementations to have unsafe cryptographic defaults, even to this day. RC2, DES, MD5, etc. remain in common use! In addition, common implementations don't follow the password string encoding method from the specification.
There's no reason for us to support this.
This requires a major version bump of the
mbedtlscrate.