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

digest v0.10 release #478

Closed
4 of 6 tasks
newpavlov opened this issue Jan 18, 2021 · 11 comments
Closed
4 of 6 tasks

digest v0.10 release #478

newpavlov opened this issue Jan 18, 2021 · 11 comments

Comments

@newpavlov
Copy link
Member

newpavlov commented Jan 18, 2021

Features to be implemented before release:

  • Add CoreWrapper type aliases for UpdateCore and other traits to prevent block size duplication.
  • Implement Debug on CoreWrapper (e.g. via an additional AlgName trait?).
  • Add block-padding feature and forward it to block-buffer.
  • Re-introduce a BlockInput-like trait (i.e. a trait with only one BlockSize associated type/constant) to unify UpdateCoreWrapper and XofReaderCoreWrapper? Think about potential implementation of the stream cipher trait for XOF readers.
  • Should we remove the core_api feature?
  • DynDigest improvements DynDigest requires alloc when it does not have to #545
@tarcieri
Copy link
Member

Some additional items I'd like to at least discuss:

@newpavlov
Copy link
Member Author

As I noted here, it's probably worth to release digest v0.10 without the blanket impl of StreamCipher and add it later in a patch release.

@tarcieri
Copy link
Member

I definitely agree with delaying adding a digest -> cipher dependency, as we already have "a lot in the pipe" for the next release(s).

That said, I think adding a blanket impl is a semver breaking change. It can potentially introduce new conflicts with impls that previously worked.

My suggestion would be to postpone it until digest v0.11.

@newpavlov
Copy link
Member Author

newpavlov commented Jan 22, 2021

That said, I think adding a blanket impl is a semver breaking change. It can potentially introduce new conflicts with impls that previously worked.

Shouldn't it be handled by the orphan rules? The stream cipher trait will be implemented on XofReaderCoreWrapper defined in digest, so conflicts should not be possible. Roughly, we will have the following code:

impl<R: XofReaderCore> XofReader for XofReaderCoreWrapper<R> { .. }
impl<R: StreamCipherCore> StreamCipher for XofReaderCoreWrapper<R> { .. }

It would mean that core XOF reader types would have to implement both XofReaderCore and StreamCipherCore and there will be a bit of code duplication with a wrapper defined in cipher, but I don't think it's a big problem. Though I am not sure what to do with the NewStreamCipher trait.

@tarcieri
Copy link
Member

Aah okay, yeah that might work out

@newpavlov
Copy link
Member Author

Going to close this issue since most entries are completed. I opened #510 for discussion on the cipher integration.

@newpavlov
Copy link
Member Author

I have added the question regarding potential removal of the core_api feature. Initially it was added for crates which will implement the mid-level traits directly. For such use case the dependency on block-buffer is redundant, plus it prevents compilation of a significant unused portion of digest code. But currently all our implementation crates will enable this feature, so I wonder if it's worth to keep this feature.

@newpavlov newpavlov reopened this Feb 9, 2021
@rvolgers
Copy link
Contributor

From discussions about the DSA implementation, I noticed this shortcoming of the DynDigest trait: #545

Should this perhaps be picked up for this release as well?

@tarcieri
Copy link
Member

tarcieri commented Mar 2, 2021

Checking in on this... it'd be really good to get another release out.

I can take a look at addressing #545 unless you have plans to @newpavlov

@newpavlov
Copy link
Member Author

Updating the DynDigest trait should be relatively simple. Currently I stuck with migrating blake2. Unfortunately because it supports alternative modes of initialization and it's key initialization requires presence of a buffer, it does not map well to the abstractions around block-level API. I've tried to work around it, but in the end I gave up... So I plan to write newtype wrappers on top of digest wrappers.

Right now I want to finish cipher v0.3 first, since it affects crypto-mac.

@newpavlov
Copy link
Member Author

Resolved in #819.

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

No branches or pull requests

3 participants