-
Notifications
You must be signed in to change notification settings - Fork 315
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
Refactor core crypto module. #502
Merged
Merged
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
This change performs a refactoring of the encryption and cryptographic functions in the `core` component. In the last 1-2 months much of the underlying implementation has been swapped out (a GPG-based system to libsodium) and much more functionality has been added. As we seem to be approaching a steady state for at least the near-term it was worth revisiting what we've built up, and we noticed 3 issues: 1. The size of the single module file was racing upwards with new functionality 2. The number of tests were not keeping pace and worse, to add them led to an even larger single file that was getting harder to navigate 3. The call sites for the crypto code often lacked the context to determine the key cache path and some crypto functions required this even though it didn't use it at all This new code organization splits the main module into several smaller ones: * `crypto::artifact` which contains behavior related to Habitat artifact files * `crypto::hash` which contains behavior relating to hashing (sometimes called checksumming) files and artifacts * `crypto::keys` which contains several sub-modules relating to the 3 primary key types: Sym keys (used to encrypt communication), Box keys (used to encrypt/decrypt files and payloads destined for service groups), and Sig keys (used to sign/verify artifacts) The updated key-related APIs have been reorganized in an attempt to be consisten with one another and to not leak their internal implementation details. In particular, a consumer of `core::crypto` should **never** have to explictly import any libsodium modules, structs, or functions. Keys are type aliases of a generic type which is maintained internally and all specific public and secret keys are internal to that "key pair" struct--even if only one of the two keys are present or relevant. When working with keys, only the key pair struct are passed around. In addition, a number of unit tests have been added so that the public interfaces of each module has good and reasonable coverage that exercises the happy path and exceptional cases. The `crypto::hash` test suite has a first attempt at an explicit functional test which is activated by enabling a feature called "functional" when running tests: `cargo test --features functional`. This test in particular downloads a large file from a website in order to verify its hash/checksum without commiting a multi-megabyte file into the codebase. Bonus points for trying out hyper's http_proxy support which (eventually) worked like a charm ;) Finally, the command-related code in the `hab` component was re-organized so that every subcommand's "business logic" function lived in a consistent module space that tightly tracks its own dependencies and any related internal functions. Signed-off-by: Fletcher Nichol <[email protected]>
📌 Commit e5401dc has been approved by |
thesentinels
pushed a commit
that referenced
this pull request
May 11, 2016
This change performs a refactoring of the encryption and cryptographic functions in the `core` component. In the last 1-2 months much of the underlying implementation has been swapped out (a GPG-based system to libsodium) and much more functionality has been added. As we seem to be approaching a steady state for at least the near-term it was worth revisiting what we've built up, and we noticed 3 issues: 1. The size of the single module file was racing upwards with new functionality 2. The number of tests were not keeping pace and worse, to add them led to an even larger single file that was getting harder to navigate 3. The call sites for the crypto code often lacked the context to determine the key cache path and some crypto functions required this even though it didn't use it at all This new code organization splits the main module into several smaller ones: * `crypto::artifact` which contains behavior related to Habitat artifact files * `crypto::hash` which contains behavior relating to hashing (sometimes called checksumming) files and artifacts * `crypto::keys` which contains several sub-modules relating to the 3 primary key types: Sym keys (used to encrypt communication), Box keys (used to encrypt/decrypt files and payloads destined for service groups), and Sig keys (used to sign/verify artifacts) The updated key-related APIs have been reorganized in an attempt to be consisten with one another and to not leak their internal implementation details. In particular, a consumer of `core::crypto` should **never** have to explictly import any libsodium modules, structs, or functions. Keys are type aliases of a generic type which is maintained internally and all specific public and secret keys are internal to that "key pair" struct--even if only one of the two keys are present or relevant. When working with keys, only the key pair struct are passed around. In addition, a number of unit tests have been added so that the public interfaces of each module has good and reasonable coverage that exercises the happy path and exceptional cases. The `crypto::hash` test suite has a first attempt at an explicit functional test which is activated by enabling a feature called "functional" when running tests: `cargo test --features functional`. This test in particular downloads a large file from a website in order to verify its hash/checksum without commiting a multi-megabyte file into the codebase. Bonus points for trying out hyper's http_proxy support which (eventually) worked like a charm ;) Finally, the command-related code in the `hab` component was re-organized so that every subcommand's "business logic" function lived in a consistent module space that tightly tracks its own dependencies and any related internal functions. Signed-off-by: Fletcher Nichol <[email protected]> Pull request: #502 Approved by: metadave
☀️ Test successful - travis |
btw, @fnichol and I zoomed about lots of the content on this, which is why it got a quick +1 without lots of comments. This PR was awesome. |
jtimberman
pushed a commit
that referenced
this pull request
Jun 12, 2016
This change performs a refactoring of the encryption and cryptographic functions in the `core` component. In the last 1-2 months much of the underlying implementation has been swapped out (a GPG-based system to libsodium) and much more functionality has been added. As we seem to be approaching a steady state for at least the near-term it was worth revisiting what we've built up, and we noticed 3 issues: 1. The size of the single module file was racing upwards with new functionality 2. The number of tests were not keeping pace and worse, to add them led to an even larger single file that was getting harder to navigate 3. The call sites for the crypto code often lacked the context to determine the key cache path and some crypto functions required this even though it didn't use it at all This new code organization splits the main module into several smaller ones: * `crypto::artifact` which contains behavior related to Habitat artifact files * `crypto::hash` which contains behavior relating to hashing (sometimes called checksumming) files and artifacts * `crypto::keys` which contains several sub-modules relating to the 3 primary key types: Sym keys (used to encrypt communication), Box keys (used to encrypt/decrypt files and payloads destined for service groups), and Sig keys (used to sign/verify artifacts) The updated key-related APIs have been reorganized in an attempt to be consisten with one another and to not leak their internal implementation details. In particular, a consumer of `core::crypto` should **never** have to explictly import any libsodium modules, structs, or functions. Keys are type aliases of a generic type which is maintained internally and all specific public and secret keys are internal to that "key pair" struct--even if only one of the two keys are present or relevant. When working with keys, only the key pair struct are passed around. In addition, a number of unit tests have been added so that the public interfaces of each module has good and reasonable coverage that exercises the happy path and exceptional cases. The `crypto::hash` test suite has a first attempt at an explicit functional test which is activated by enabling a feature called "functional" when running tests: `cargo test --features functional`. This test in particular downloads a large file from a website in order to verify its hash/checksum without commiting a multi-megabyte file into the codebase. Bonus points for trying out hyper's http_proxy support which (eventually) worked like a charm ;) Finally, the command-related code in the `hab` component was re-organized so that every subcommand's "business logic" function lived in a consistent module space that tightly tracks its own dependencies and any related internal functions. Signed-off-by: Fletcher Nichol <[email protected]> Pull request: #502 Approved by: metadave
raskchanky
pushed a commit
that referenced
this pull request
Apr 16, 2019
This change performs a refactoring of the encryption and cryptographic functions in the `core` component. In the last 1-2 months much of the underlying implementation has been swapped out (a GPG-based system to libsodium) and much more functionality has been added. As we seem to be approaching a steady state for at least the near-term it was worth revisiting what we've built up, and we noticed 3 issues: 1. The size of the single module file was racing upwards with new functionality 2. The number of tests were not keeping pace and worse, to add them led to an even larger single file that was getting harder to navigate 3. The call sites for the crypto code often lacked the context to determine the key cache path and some crypto functions required this even though it didn't use it at all This new code organization splits the main module into several smaller ones: * `crypto::artifact` which contains behavior related to Habitat artifact files * `crypto::hash` which contains behavior relating to hashing (sometimes called checksumming) files and artifacts * `crypto::keys` which contains several sub-modules relating to the 3 primary key types: Sym keys (used to encrypt communication), Box keys (used to encrypt/decrypt files and payloads destined for service groups), and Sig keys (used to sign/verify artifacts) The updated key-related APIs have been reorganized in an attempt to be consisten with one another and to not leak their internal implementation details. In particular, a consumer of `core::crypto` should **never** have to explictly import any libsodium modules, structs, or functions. Keys are type aliases of a generic type which is maintained internally and all specific public and secret keys are internal to that "key pair" struct--even if only one of the two keys are present or relevant. When working with keys, only the key pair struct are passed around. In addition, a number of unit tests have been added so that the public interfaces of each module has good and reasonable coverage that exercises the happy path and exceptional cases. The `crypto::hash` test suite has a first attempt at an explicit functional test which is activated by enabling a feature called "functional" when running tests: `cargo test --features functional`. This test in particular downloads a large file from a website in order to verify its hash/checksum without commiting a multi-megabyte file into the codebase. Bonus points for trying out hyper's http_proxy support which (eventually) worked like a charm ;) Finally, the command-related code in the `hab` component was re-organized so that every subcommand's "business logic" function lived in a consistent module space that tightly tracks its own dependencies and any related internal functions. Signed-off-by: Fletcher Nichol <[email protected]> Pull request: #502 Approved by: metadave
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.
This change performs a refactoring of the encryption and cryptographic
functions in the
core
component. In the last 1-2 months much of theunderlying implementation has been swapped out (a GPG-based system to
libsodium) and much more functionality has been added. As we seem to be
approaching a steady state for at least the near-term it was worth
revisiting what we've built up, and we noticed 3 issues:
functionality
to an even larger single file that was getting harder to navigate
determine the key cache path and some crypto functions required this
even though it didn't use it at all
This new code organization splits the main module into several smaller
ones:
crypto::artifact
which contains behavior related to Habitat artifactfiles
crypto::hash
which contains behavior relating to hashing (sometimescalled checksumming) files and artifacts
crypto::keys
which contains several sub-modules relating to the 3primary key types: Sym keys (used to encrypt communication), Box keys
(used to encrypt/decrypt files and payloads destined for service
groups), and Sig keys (used to sign/verify artifacts)
The updated key-related APIs have been reorganized in an attempt to be
consisten with one another and to not leak their internal implementation
details. In particular, a consumer of
core::crypto
should neverhave to explictly import any libsodium modules, structs, or functions.
Keys are type aliases of a generic type which is maintained internally
and all specific public and secret keys are internal to that "key pair"
struct--even if only one of the two keys are present or relevant. When
working with keys, only the key pair struct are passed around.
In addition, a number of unit tests have been added so that the public
interfaces of each module has good and reasonable coverage that
exercises the happy path and exceptional cases.
The
crypto::hash
test suite has a first attempt at an explicitfunctional test which is activated by enabling a feature called
"functional" when running tests:
cargo test --features functional
.This test in particular downloads a large file from a website in order
to verify its hash/checksum without commiting a multi-megabyte file into
the codebase. Bonus points for trying out hyper's http_proxy support
which (eventually) worked like a charm ;)
Finally, the command-related code in the
hab
component wasre-organized so that every subcommand's "business logic" function lived
in a consistent module space that tightly tracks its own dependencies
and any related internal functions.
Signed-off-by: Fletcher Nichol [email protected]