feat(kas): implement KAS nanotdf standard crypto #807
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
Refactored the Key Access Service provider to use a pointer receiver and updated the cryptography system for better private-public key management. Enhanced the elliptic curve digital signature algorithm (ECDSA) methods, and included functions to generate a symmetric key, ephemeral keys, and a session key. Also fixed minor issues and commented out code blocks.
pflynn-virtru
changed the title
pflynn-virtru
changed the title
Removed commented-out code and the errNotImplemented variable from the standard_crypto.go file. Also replaced a panic scenario with a standard error message to improve error handling.
Added unit tests to test the functionality of different methods present in the standard crypto file. Additionally, handled an edge case where the code could break when the length of ecKeys is less than or equal to privateKeyHandle.
The commit refactors assertions in "standard_crypto_test.go" to use Require instead of Assert, as Require stops test execution once a failure is encountered. Moreover, dead code primarily containing key validation functions has been removed for better readability and maintenance.
Removed unused variable in the function "expect" in the standard_crypto_test specifically within the "Success" test case. This clear unused variables and helps to improve the readability of the code.
sujankota
requested changes
May 17, 2024
| } | ||
|
|
||
| // We have to ensure we have an ECDSA public key | ||
| pubKey, ok := pubKeyInterface.(*ecdsa.PublicKey) |
There was a problem hiding this comment.
This should ECDH not ECDSA
There was a problem hiding this comment.
Session key = KDF(ECDH(sdk-session-public-key, kas-ephemeral-private-key), 'L1L')
L1L is the salt
There was a problem hiding this comment.
ocrypto module should have this functionality. We can just call into it.
sujankota
reviewed
May 17, 2024
| // ecPublicKey *ecdh.PublicKey | ||
| // ecPrivateKey *ecdh.PrivateKey | ||
| Identifier string | ||
| ecPrivateKey *ecdsa.PrivateKey |
This commit introduces a new ECCertificate function in the HSMSession struct along with a corresponding method in the ICryptoProvider interface. It also updates the "eccertid" in various service configuration yaml files. At the same time, several Go dependencies have been updated to their newer versions.
This commit refines the error logging in `rewrap.go` file for better tracking and modifies the key generation process in `access/rewrap.go` for more efficiency. It has also corrected an error message in `server.go`. Updated dependencies in `go.work.sum` for synchronicity with new alterations.
Updated the code to support ECDH keys in addition to ECDSA keys. This includes changes in key generation, marshalling, and unmarshalling, as well as handling these keys within struct types. Also improved error handling and logging across multiple files for better debugging and traceability.
Updated the `nanoTDFRewrap` function in `rewrap.go` to now take a context argument. Refactored key variables for clarity and simplicity. Simplified the conversion of public keys to bytes. Modified logic of `GenerateNanoTDFSessionKey` function for a cleaner key generation approach.
This commit cleans up and optimizes key management and generation in nanoTDFRewrap. The unnecessary check of mismatched key access URL is removed to simplify the flow. Moreover, the generation and use of symmetric keys (now referred to as DEK) and session keys are clarified. Naming in the standard_crypto module is also improved to better reflect the purpose of the keys.
Modified the init-temp-keys.sh script to generate an ECDSA private key and certificate, and handled loading the EC certificate in the standard_crypto.go file. Also, updated publicKey.go to fetch the public key based on the EC certificate ID instead of a hardcoded value. Additionally, some debug log messages were inserted for detailed tracing.
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.
Refactored the Key Access Service provider to use a pointer receiver and updated the cryptography system for better private-public key management. Enhanced the elliptic curve digital signature algorithm (ECDSA) methods, and included functions to generate a symmetric key, ephemeral keys, and a session key. Also fixed minor issues and commented out code blocks.