Stabilization of internal implementations of the cryptographic primitives

[brycx]

In an effort towards moving to a more stabilized crate, in terms of the internal cryptographic primitives and their implementation (not so much the high-level API in general), @vlmutolo and I have decided to do an evaluation of the current open issues. Also of course, checking if any other things had been left out (personal notes, experimental non-public things, etc.)

The end-result should be identification of issues potentially impacting implementation of currently supported cryptographic primitives and evaluate if they can/will be done in the near future.

Summary:

• Security audit #21: N/A.
• Add generated XChaCha20 test vectors #37: Suggest to close as fixed.
• Using Pin with SecretKey and other types that hold secret data #59: Suggest to leave as is.
• Add tests for trait implementations on newtypes #120: Include in 0.17.2.
• Investigate using haybale-pitchfork to test constant-time execution #121: N/A.
• Cache with cargo-audit in CI #147: Suggest to close as won't fix.
• subtle or subtle-ng? #176: TODO.
• Tracking issue for supporting ECDH and EdDSA #199: Leave as is (future, undetermined).
• Re-investigate possible support for Argon2id #236: Leave as is (future, undetermined).
• integrate with standard Read/Write IO traits #227: Include in 0.17.2.
• Rust 2021 Edition #237: Leave as is (future, ecosystem migration to higher MSRV).
• Considering removing from_slice from newtype APIs #249: Leave as is (await Rust 2021 Edition #237).
• cryptofuzz module #248: N/A.
• Future use of core::simd and moving to SIMD-based vectorized implementations #264: Leave as is (future, undetermined).
• Investigate performance regression of ChaCha20 implementation #244: Include in 0.17.2.

Issue #21: Security audit

Link: #21

N/A.

Issue #37: Add generated XChaCha20 test vectors

Link: #37

XChaCha20 is already tested a lot through XChaCha20-Poly1305 (differential fuzzing, Wycheproof, RFC test vectors, some tests from WireGuard and some from BoringSSL). Edge-cases like large counters are tested through the ChaCha20 implementation.

It seems as if this issue can be closed.

Issue #59: Using Pin with SecretKey and other types that hold secret data

Link: #59

Based upon discussion in the issue tracker, there currently seems to be no way to guarantee the wiping of secret data in Rust. Even using Pin, it would still stack-allocated memory without guarantees, as this cannot be pinned.

As suggested by @cyberia-ng, a better effort could probably be made by Boxing data that is used in the high-level API, but this would also not provide full guarantees with compiler optimizations.

Using Box would also only be possible in high-level because Orion's hazardous module should work in #[no_std]. Investigation into using unsafe for mlock etc, has not been done because of the general goal of Orion being as unsafe-free as possible.

All in all, this is not a problem that can be solved in the near future and Orion will continue zeroing memory as done currently, as a best effort approach, without increasing complexity of the newtypes too much.

Issue #120: Add tests for trait implementations on newtypes

Link: #120

This is a low-priority issue. The intention is simply to easily verify that all the newtypes
implement the traits we expect them to implement. Simply serving as an easier verification point as checking the macro rules for the newtypes.

This should be a relatively easy change and in my opinion implemented in the next minor release (0.17.2). Won't change internals of Orion cryptographic primitives.

Issue #121: Investigate using haybale-pitchfork to test constant-time execution

Link: #121

Ongoing investigation, won't change internals of Orion cryptographic primitives.

Issue #147: Cache with cargo-audit in CI

Link: #147

The intent here was to speed up the CI process of the dependency audit, however, recent frameworks (e.g Google's SLAS) to harden packages against supply-chain attacks, suggest against this. In terms of SLAS, see SLAS Level 3 of Hermetic and Ephemeral build environment. Avoiding caching could avoid caching potentially vulnerable/malicious packages.

Looking at the current runtimes for audits (using cargo-audit and audit-check Action), they both take ~1.5 minute, which is in no way a hindrance to development or productivity.

Suggest to close this as won't fix, especially with the hermetic/ephemeral build environment in mind.

Issue #176 [TODO]: subtle or subtle-ng?

Link: #176

Issue #199: Tracking issue for supporting ECDH and EdDSA

Link: #199

The first half of this issue was completed with the inclusion of X25519. Ed25519 may still be included in the future, should there be enough bandwidth for it. This will not affect other cryptographic primitives in Orion. Even though Ed25519 would interact with SHA-512, all functionality required therefrom should already be available.

Issue #236: Re-investigate possible support for Argon2id

Link: #236

This issue would affect the current implementation of Argon2i in Orion. Possibly also the high-level API, considering, that it would then utilize some degree of parallelism as a default. There currently however is very little bandwidth for this, while two requests for it have been made, these didn't seem like a high-priority feature-requests.

Unless we receive high-priority requests for this, this will be left as an investigation for now. Especially since it's not known if this would introduce too much unsafe in Orion's dependency tree.

Issue #227: integrate with standard Read/Write IO traits

Link: #227

This will impact the internal implementations of Orion's primitives, in terms of the API that is provided. While the implementation introduced in 0.17.1 was straight-forward and easy to reason about, it is still missing for AEAD.

This should be included in the 0.17.2 release in order to stabilize the hazardous-level API as much as possible.

Issue #237: Rust 2021 Edition

Link: #237

This would change the MSRV to 1.55. It would most likely not impact the internal primitive's implementation. Investigation hasn't revealed major pro's on migrating to the 2021 edition yet, except for TryFrom/TryInto being in prelude. This however is only a considerable improvement if #249 is also implemented.

Seeing as it could break a lot of current crates not moving as fast, this can wait.

Issue #249: Considering removing from_slice from newtype APIs

Link: #249

This would impact the API provided by newtypes, but in reality just move the current code from the from_slice() method into the TryFrom impl. Thus, in reality not impact the actual logic at all. Won't change internals of Orion cryptographic primitives.

Issue #248: cryptofuzz module

Link: #248

On hold, won't change internals of Orion cryptographic primitives.

Issue #264: Future use of core::simd and moving to SIMD-based vectorized implementations

Link: #264

This will change internals of Orion cryptographic primitives. Specifically, ChaCha20 and BLAKE2b. Poly1305 can also be implemented with AVX/SSE, but we prioritize the formally verified implementation based on fiat-crypto, introduced in 0.17.1, over a more perhaps more performant implementation.

Further, seeing as:

• core::simd is not stabilized yet and most likely won't be for some time
• Much more investigation is required to find out if this approach aligns with Orion in terms of unsafe and target support

we expect this to not be a near-future priority. Possibly wait until SIMD is stabilized. One thing to note is, the current implementations of BLAKE2b and ChaCha20 are already based on the vectorized approach, so theoretically switching to SIMD should be a fairly easy change (ChaCha20 depends on outcome of #244).

Issue #244: Investigate performance regression of ChaCha20 implementation

Link: #244

Will most likely result in change to internals of Orion cryptographic primitives. Suggested plan of action (inclusion in 0.17.2):

• Re-tested on current stable Rust (1.58.0). If the performance regression persists, evaluate if it is acceptable regression (the first investigation was nearly a halving of throughput, which isn't acceptable in most cases).
• Investigate how this can be mitigated. Possibly revert to previous ChaCha20 implementation or redo to match previous performance.