Conversation
|
|
||
| | Feature | Description | | ||
| | -------------------- | --------------------------------------------- | | ||
| | `tls-ring` (default) | rustls with ring crypto | |
There was a problem hiding this comment.
While the default provider for crypto is a Pure-Rust implementation that excludes ring, tls defaults to ring. What about the idea of defaulting to a Pure-Rust implementation that supports the major cipher suites?
https://github.com/RustCrypto/rustls-rustcrypto
EDIT: Until the official support is released, we will create an experimental branch that supports the new rc crates.
https://github.com/nabetti1720/rustls-rustcrypto/tree/next
% make check
cargo clippy --all-targets --all-features -- -D warnings
Checking rustls-rustcrypto v0.0.2-alpha (/Users/shinya/Workspaces/rustls-rustcrypto)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.15s
% make build
cargo +nightly build -r --target aarch64-apple-darwin
Compiling rustls-rustcrypto v0.0.2-alpha (/Users/shinya/Workspaces/rustls-rustcrypto)
Finished `release` profile [optimized] target(s) in 3.88s
There was a problem hiding this comment.
@nabetti1720 I think the reason is that this is pretty much unmaintained:
https://github.com/RustCrypto/rustls-rustcrypto. Last commit was 2 years ago
There was a problem hiding this comment.
@richarddavison Yes, so I am submitting a PR to update to the latest rustcrypto.
RustCrypto/rustls-rustcrypto#102
There was a problem hiding this comment.
@richarddavison It hasn't been published yet, but the PR has been merged. This has been experimentally implemented in PR #1343, so please take a look.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| | Feature | Description | | ||
| | ----------------------- | ----------------------------------------------------------------- | | ||
| | `crypto-rust` (default) | Pure Rust crypto using RustCrypto crates | | ||
| | `crypto-ring` | Ring-only crypto (limited algorithm support) | |
There was a problem hiding this comment.
Is there any point in providing partially supported features like crypto-ring or crypto-graviola?
By doing the following, it will be easier for users to choose a backend, and implementers will be able to optimize their internal implementations depending on the supported cipher suites of ring and glaviora without affecting users.
| Feature | Description |
|---|---|
crypto-rust (default) |
Pure Rust crypto using RustCrypto crates |
crypto-ring |
Ring for hashing/HMAC, RustCrypto for everything else |
crypto-graviola |
Graviola for hashing/HMAC/AES-GCM, RustCrypto for everything else |
crypto-openssl |
OpenSSL-based crypto |
EDIT: If we want to follow the tls concept, we may want to support crypto-aws-lc.
Added support for rustls, graviola and OpenSSL TLS backends This makes it easier to choose different TLS implementations depending on deployment needs.
Consistent with generate_ec_key which also returns SEC1 format for public keys.
- Add OkpJwkImport/OkpJwkExport structs and import_okp_jwk/export_okp_jwk methods to CryptoProvider trait - Implement OKP JWK methods in RustCrypto, OpenSSL, ring, and graviola providers - Create unified export_key.rs that uses CRYPTO_PROVIDER methods instead of inline RustCrypto code - Remove duplicate export_key_openssl.rs, import_key_openssl.rs, wrapping_openssl.rs - Simplify mod.rs feature gates to use _subtle-full instead of complex conditions - Fix EC private key export to use SecretKey::from_pkcs8_der for correct d value extraction This reduces code duplication by having shared format handling code call provider-specific methods for key component extraction.
Public key handle is SEC1 format, not SPKI DER. Parse coordinates directly from SEC1 point.
| @@ -23,10 +26,7 @@ pub async fn subtle_digest<'js>( | |||
| } | |||
|
|
|||
| pub fn digest(sha_algorithm: &ShaAlgorithm, data: &[u8]) -> Vec<u8> { | |||
There was a problem hiding this comment.
Use the HashAlgorithm I proposed earlier
modules/llrt_fetch/Cargo.toml
Outdated
| llrt_encoding = { version = "0.7.0-beta", path = "../../libs/llrt_encoding" } | ||
| llrt_http = { version = "0.7.0-beta", default-features = false, path = "../llrt_http" } | ||
| llrt_json = { version = "0.7.0-beta", path = "../../libs/llrt_json" } | ||
| llrt_test_tls = { path = "../../libs/llrt_test_tls", default-features = false, optional = true } |
There was a problem hiding this comment.
Comment above is still relevant
| http1 = ["hyper/http1", "hyper-rustls/http1"] | ||
| http2 = ["hyper/http2", "hyper-rustls/http2"] | ||
| http1 = ["hyper/http1", "hyper-rustls?/http1", "hyper-util/http1"] | ||
| http2 = ["hyper/http2", "hyper-rustls?/http2", "hyper-util/http2"] |
There was a problem hiding this comment.
Comment above is still relevant
- Replace ShaAlgorithm with unified HashAlgorithm enum - Consolidate Hash and Hmac classes into hash.rs module - Move aes_variants.rs to provider/rust/ directory - Remove separate md5_hash.rs and sha_hash.rs files - Update all provider implementations to use HashAlgorithm - Update subtle/digest.rs to use HashAlgorithm
This reverts commit b674e0e.
- Consolidate Hash and Hmac classes into hash.rs module - Move aes_variants.rs to provider/rust/ directory - Remove separate md5_hash.rs and sha_hash.rs files - Update all provider implementations to use HashAlgorithm - Update subtle/digest.rs to use HashAlgorithm
3 participants
Issue # (if available)
Description of changes
Refactor crypto and HTTP modules to support multiple TLS backends
This makes it easier to choose different Crypto & TLS implementations depending on deployment needs.
Checklist
tests/unitand/or in Rust for my feature if neededmake fixto format JS and apply Clippy auto fixesmake checktypes/directoryBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.