Skip to content
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 1 commit into from
May 11, 2016
Merged

Conversation

fnichol
Copy link
Collaborator

@fnichol fnichol commented 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]

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]>
@fnichol
Copy link
Collaborator Author

fnichol commented May 11, 2016

gif-keyboard-15111334424557767708

@bookshelfdave
Copy link
Contributor

gif-keyboard-7001938884042763911

I'd like to tweak check_filename() to use regexes asap, as it gets confused when keys with common prefixes are in the same directory.

@bookshelfdave
Copy link
Contributor

@thesentinels r+

@thesentinels
Copy link
Contributor

📌 Commit e5401dc has been approved by metadave

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
@thesentinels
Copy link
Contributor

⌛ Testing commit e5401dc with merge ed45489...

@thesentinels
Copy link
Contributor

☀️ Test successful - travis

@thesentinels thesentinels merged commit e5401dc into master May 11, 2016
@bookshelfdave
Copy link
Contributor

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.

@fnichol fnichol deleted the fnichol/core-crypto-refactor branch June 10, 2016 04:20
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants