Allow external access to the SecretBox::new method

[PedroMBB]

Type of PR:

• Bugfix

Required reviews:

• TBD

What this does:

• Changes the visibility of the SecretBox::new method from pub(crate) to pub.

Why it's needed:

• Currently, there is no way to instantiate a SecretBox from outside the library.
• The absence of an alternative method makes it impossible to instantiate a SecretKey.
• Instantiating a SecretKey outside the library is necessary, for example, when an application stores the secret key in a file and needs to retrieve it.

Notes for reviewers:

• This issue is not present on the WASM and Python APIs because the facades providing those interfaces use the SecretBox::new method internally.
• This is a minor change; however, I am unaware of the original reasoning for making this method only visible within the crate.
• This is my first pull request, so any suggestions for future PRs are always welcome.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (47867d2) 58.08% compared to head (8e3e4f1) 58.08%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #132   +/-   ##
=======================================
  Coverage   58.08%   58.08%           
=======================================
  Files          17       17           
  Lines        3123     3123           
=======================================
  Hits         1814     1814           
  Misses       1309     1309           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[fjarri]

The absence of an alternative method makes it impossible to instantiate a SecretKey.

Are you referring specifically to the SecretKey::try_from_be_bytes() method? I think a better (albeit an API-breaking) solution would be to for it to just take a &[u8], and leave the zeroization of the source buffer to the caller.

[PedroMBB]

The absence of an alternative method makes it impossible to instantiate a SecretKey.

Are you referring specifically to the SecretKey::try_from_be_bytes() method? I think a better (albeit an API-breaking) solution would be to for it to just take a &[u8], and leave the zeroization of the source buffer to the caller.

Yes, it's due to that method that the need to instantiate a SecretBox appears. My current implementation for Serialization and Deserialization is as follows:

// Serialization
let v: umbral_pre::SecretBox<GenericArray<_, _>> = self.0.to_be_bytes();
v.as_secret().serialize(serializer);

// Deserialization (Workaround)
let secret_key = SecretKey::random();
let mut secret_box = secret_key.to_be_bytes();
let key_array = secret_box.as_mut_secret();
*key_array = GenericArray::deserialize(deserializer)?;
SecretKey::try_from_be_bytes(&secret_box);

// Deserialization (with pub new)
let v = GenericArray::deserialize(deserializer)?;
let v = SecretBox::new(v);
SecretKey::try_from_be_bytes(&v);

Modifying the SecretKey::try_from_be_bytes() function signature would address the issue and also eliminate the need to use the older version of the generic_array crate (0.14.6 versus the current 1.0.0) for deserialization. However, this change would result in the SecretKey::to_be_bytes() and SecretKey::try_from_be_bytes() methods operating with different types, which could be misleading given their names. An alternative solution could be introducing a new function, for example: SecretKey::try_from_be_bytes_slice(). This would differentiate it from the existing methods while avoiding breaking changes.

[fjarri]

Should be made available by #134