Skip to content

Extend nss-rs with SHA3 support#17

Open
beurdouche wants to merge 1 commit into
mainfrom
beurdouche-sha3
Open

Extend nss-rs with SHA3 support#17
beurdouche wants to merge 1 commit into
mainfrom
beurdouche-sha3

Conversation

@beurdouche
Copy link
Copy Markdown
Member

WIP

Comment thread src/hash.rs
/// # Errors
///
/// Returns an error if NSS initialization fails or the hash operation fails.
pub fn sha256(data: &[u8]) -> Result<[u8; 32], Error> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether this could use generics instead? It's a shame, but const generics currently don't support enums for the value, but you can still define types for each, just like the rust crypto crates do:

struct Sha256;
impl OidTag for Sha256 {
  const OID: usize = SECOidTag::SEC_OID_SHA256;
}
impl Hash for Sha256 { // Hash can implement OidTag
  const HASHLEN: usize = oid_to_hash_len(Self::OID);
}
pub fn hash<A: Hash>(data: &[u8]) -> Result<[u8; A::HASHLEN], Error> {
   // ...
}
pub fn sha256(data:&[u8]) -> Result<[u8; 32], Error> {
  hash::<Sha256>(data)
}

Comment thread src/hash.rs

/// Returns the output length in bytes for the given hash algorithm.
#[must_use]
pub const fn hash_alg_to_hash_len(alg: &HashAlgorithm) -> usize {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not take a reference. HashAlgorithm is Copy (and likely smaller than the reference is).

Comment thread src/hash.rs
SHA3_512,
}

const fn hash_alg_to_oid(alg: &HashAlgorithm) -> SECOidTag::Type {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These functions should be a const functions on the type (and take self, see below).

Comment thread src/hash.rs
Comment on lines 83 to 86
let data_len: i32 = match i32::try_from(data.len()) {
Ok(data_len) => data_len,
_ => return Err(Error::Internal),
};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be able to use the ? operator, exposing integer sizing issues with a distinct type, rather than Error::Internal.

Comment thread src/hash.rs
_ => return Err(Error::Internal),
};
let expected_len = hash_alg_to_hash_len(alg);
let mut digest = vec![0u8; expected_len];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With a generic type, this could be an array allocated on the stack (faster).

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.

2 participants