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

Add error support when RustCrypto doesn't #176

Closed
ia0 opened this issue Jun 15, 2023 · 5 comments · Fixed by #429
Closed

Add error support when RustCrypto doesn't #176

ia0 opened this issue Jun 15, 2023 · 5 comments · Fixed by #429
Assignees
Labels
crate:board Modifies the board API for:security Improves firmware or project security good first issue Good for newcomers needs:design Needs design to make progress

Comments

@ia0
Copy link
Member

ia0 commented Jun 15, 2023

After #174 the hash API doesn't support returning errors because the RustCrypto API for digest (e.g. Update, FixedOutput) don't return a Result. We should somehow add the possibility to report errors in those operations, for example using a side channel that may be checked. This could be a LastError trait that would be used after each operation to check for error.

trait LastError {
    type Error: Copy;
    fn last_error(&self) -> Option<Error>;
}
@ia0 ia0 added needs:design Needs design to make progress crate:board Modifies the board API for:security Improves firmware or project security labels Jun 15, 2023
@ia0 ia0 added the good first issue Good for newcomers label Jan 15, 2024
@zhouwfang
Copy link
Member

Hi, Could you assign this issue to me? Thanks.

@zhouwfang
Copy link
Member

Could you give an example on when a hash API should return an error? Thanks.

For example, when should this call of Update return an error?
https://github.com/zhouwfang/wasefire/blob/9888e8fd20018d0e5f55275a9b559bfdfa8d3db6/crates/scheduler/src/call/crypto/hash.rs#L86

@ia0
Copy link
Member Author

ia0 commented Apr 12, 2024

Actually, all those functions should return an error for both hash and hmac: initialization, update, finalization.

One solution I see is to make the board::crypto::Hmac trait FixedOutputReset like it is for board::crypto::Hash (not sure if it will break something), and add for both traits the new LastError trait I described above. Then in the scheduler as you pointed out, we will need to call context.last_error() to access the error after each operation that uses the RustCrypto API (which doesn't return errors). So in your case, each branch needs to call context.last_error() after calling context.update(data).

Actually if LastError returns Result<(), Error> this is easier, because that's going to be directly the result for update and finalize. For initialize, it's going to be Result::map first to get the handler.

use wasefire_error::Error;
trait LastError {
    fn last_error(&self) -> Result<(), Error>;
}

@zhouwfang
Copy link
Member

Thanks for the explanation! Still wanted to get more contexts:

  1. The original question was more about: under what condition, a operation that uses the RustCrypto API would produce an error. In Abstract Hkdf and HkdfExtract over hmac::Mac RustCrypto/KDFs#80, you mentioned the errors could result from hardware failure. How could we detect and get access to hardware failure? Basically, where does the error returned from last_error come from specifically?

  2. I could understand we should use LastError for update, and finalize (since it happens after update). Why do we need to use it for initialize? Initializing a HashContext does not seem to use RustCrypto API.

@ia0
Copy link
Member Author

ia0 commented Apr 16, 2024

Basically, where does the error returned from last_error come from specifically?

This will come from the board implementation (so it won't be implemented as part of this issue). Platforms where crypto may fail at any time would have to handle an error state on their side. When an error happens during some call (e.g. initialize, update, finalize), they would need to return a dummy result (for unit-returning function it's easy) and store the error somewhere. There is a guarantee that after each function, last_error is called, which is when the error is read.

Actually, this guarantee does not exactly hold when using a software HMAC on top of a hardware hash, because multiple operations may be called before last_error is called at the HMAC level (the software HMAC implementation calls last_error on the hash to implement its own last_error). So the guarantee is more that last_error is eventually called and so before the result is used by the end user.

Why do we need to use it for initialize? Initializing a HashContext does not seem to use RustCrypto API.

The Default trait is an assumption of RustCrypto for the default implementation of Digest. So it's true we could have our own initialization function that may fail, but then we would still need to implement Default which cannot fail to be able to compose the hash API with the HMAC API.

The reason we don't create our own API instead of using RustCrypto, is precisely to be able to have a software implementation of HMAC done by RustCrypto. The only crypto implementation at the moment is HKDF because we can't use RustCrypto, see RustCrypto/KDFs#80.

@ia0 ia0 closed this as completed in #429 Apr 22, 2024
ia0 added a commit that referenced this issue Apr 22, 2024
Fixes #176

Co-authored-by: Zhou Fang <[email protected]>
Co-authored-by: Julien Cretin <[email protected]>
chris-dietz pushed a commit to chris-dietz/wasefire that referenced this issue Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate:board Modifies the board API for:security Improves firmware or project security good first issue Good for newcomers needs:design Needs design to make progress
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants