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

Fixes to cryptobox API #712

Merged
merged 6 commits into from
Feb 10, 2021

Commits on Feb 9, 2021

  1. Fixes to cryptobox API

    Recent testing has shown that sometimes, errors are mishandled
    when using the cryptobox API. The problem is:
    
    (1) Mac check failure is returned using a `bool`, but this doesn't
        have the `#[must_use]` annotation that `core::Result` does.
    (2) Cryptobox itself was still using the `Error::MacFailed` which
        was supposed to go away.
    (3) Cryptobox itself was not using xoloki's CtAead trait, it was
        still using the variable time thing, and only the return codes
        were changing.
    
    The solution proposed is:
    (1) Introduce CtDecryptResult type which is a new-type wrapper around
        `subtle::Choice` which has the must-use annotation. Put this
        in `mc-crypto-ct-aead` crate and use it.
        Previously I was worried to use `subtle::Choice` as an API type,
        but now we have doen that elsewhere and it hasn't caused churn
        or problems.
    (2) Make cryptobox return CtDecryptResult instead of bool as it had
        been doing, and nuke the `MacFailed` error variant which should
        be gone now. Make cryptobox also use the same version of `aead`
        crate that `mc-crypto-ct-aead` crate is exporting.
    (3) Make cryptobox actually use the constant-time aes-gcm.
    
    Besides these issues, while updating the fog hint code that uses this,
    we noticed:
    (4) The fog hint decryption code which used cryptobox is needlessly
        complicated, uses an RNG for no real reason, and introduces
        unnecessary types with special implementations of subtle trait.
    
    Proposed change:
    (4)  We have now made it completely determinsitic, which is better
        for testability, and documented that it doesn't make changes
        if decryption fails, which is all that is needed. Then the
        caller can decide what value the output parameter should have
        in that case. We deleted the unnecessary types and functions.
    cbeck88 committed Feb 9, 2021
    Configuration menu
    Copy the full SHA
    285bcf4 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    2d99389 View commit details
    Browse the repository at this point in the history
  3. lock file

    cbeck88 committed Feb 9, 2021
    Configuration menu
    Copy the full SHA
    21abc9a View commit details
    Browse the repository at this point in the history

Commits on Feb 10, 2021

  1. Configuration menu
    Copy the full SHA
    759ae29 View commit details
    Browse the repository at this point in the history
  2. Update crypto/box/src/versioned.rs

    Co-authored-by: James Cape <jamescape777@gmail.com>
    cbeck88 and jcape committed Feb 10, 2021
    Configuration menu
    Copy the full SHA
    0af0bff View commit details
    Browse the repository at this point in the history
  3. Update crypto/box/src/versioned.rs

    Co-authored-by: James Cape <jamescape777@gmail.com>
    cbeck88 and jcape committed Feb 10, 2021
    Configuration menu
    Copy the full SHA
    894193f View commit details
    Browse the repository at this point in the history