Skip to content
This repository was archived by the owner on Mar 5, 2025. It is now read-only.

Conversation

@szszszsz
Copy link
Member

@szszszsz szszszsz commented Sep 6, 2022

Add basic software encryption-at-rest

@szszszsz
Copy link
Member Author

szszszsz commented Sep 6, 2022

Current notes after a brief review:

  • maybe add checksum for the "pin-key" encryption key? to check against corruption after rehashing
  • "pin-key" KDF should be proper, not a doubled sha2 / get_pin_key() - placeholder?
  • no dynamic closed/open state management AFAICS - TBD
  • design: why using AES-CBC, which does not have AEAD, while we have ChaCha20-Poly1305?

If AES is required then:

  • question: client side there is no pkcs padding or any kind of length encoding added to the plaintext before encryption
    • is this maintained by the underlying trussed crypto?
  • there is no checksum check for the ciphertext
    • full HMAC might be too slow for a quick, regular access? ChaCha20-Poly1305 does not have problem with that though
    • encrypt-than-HMAC
  • IV variable is called zero-IV
  • if AES-CBC, then ideally IV should be randomized per message, and stored in plaintext along with the ciphertext
  • if path-constructed SIV is required, hash it with the private key before using (see ESSIV)
  • if considering AES, the AES-GCM seems like a better choice here overall

Based on the HmacSha256 implementation - the only difference is, that
the result is stored as a P256 key, instead of a shared variant.

As disscussed, it would be best in the future to introduce shared
implementation across different combinations of hash and resulting key
algorithms.
@daringer daringer force-pushed the raw_store branch 5 times, most recently from fe578c5 to c221d8d Compare September 26, 2022 20:48
@daringer
Copy link

Added ChangePin and SetClientContextPin as a rudimentary mechanism for symmetric key wrapping using the pin:

  • initially the ClientContext::pin is 1234, the client has to ensure the correct pin is set before successful filesystem calls can be done
  • once the correct pin is set inside the client context, calling ChangePin will re-wrap the symmetric key with the new pin
  • any incorrect pin during read or write is propagating the Error: FilesystemEncryptionError which essentially translates to: could not unwrap the fs-encryption key due to a incorrect pin (make sure to use try_syscall!)
  • examples for api usage, integration and to play around: https://github.com/daringer/playground-app/tree/main/src

Jan Nordholz and others added 5 commits September 29, 2022 07:14
The 'service' module uses ClientId (via pipe::ServiceEndpoint::client_id) to
carry client-specific metadata. Previously, this was only the ID string, so
a type alias for PathBuf was sufficient. With the upcoming dynamic syscall
dispatch, the amount of client-specific state Trussed has to keep is going
to grow significantly. This commit creates the necessary scaffolding.

Minor changes:
  - in the filesystem-based software store implementations, use an explicit
    PathBuf - drop the ClientId type import / alias redeclaration
  - pass ClientId around as a mutable reference in the reply_to() machinery,
    as we do not want to clone this structure arbitrarily when it gets large
    (as a sentinel, also stop deriving Clone for it)
* add RawStore to wrap store::* calls
* add playgrond-app for testing
* extend ClientContext with 'pin'
szszszsz added a commit to Nitrokey/nitrokey-websmartcard-rust that referenced this pull request Oct 17, 2022
Depends on the non-merged Trussed modification

Nitrokey/trussed#2
@szszszsz
Copy link
Member Author

What's left to finish and merge this PR?

@daringer
Copy link

quite a lot:

overall I would suggest to revisit this topic once the PRs above are merged

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants