Skip to content

Conversation

@ueno
Copy link
Collaborator

@ueno ueno commented Sep 20, 2023

When signed message is large, passing it through a byte array would cost memory; this switches to using io.Reader to avoid that.

Using io.Reader in the function signatures could also help diversify those HashSign/HashVerify from Sign/Verify, through a separate interface, alongside the crypto.Signer interface.

When signed message is large, passing it through a byte array would
cost memory; this switches to using io.Reader to avoid that.

Using `io.Reader` in the function signatures could also help diversify
those HashSign/HashVerify from Sign/Verify, through a separate
interface, alongside the `crypto.Signer` interface.

Signed-off-by: Daiki Ueno <[email protected]>
@ueno
Copy link
Collaborator Author

ueno commented Sep 20, 2023

The idea behind this is to allow one-shot signature interface to be defined alongside crypto.Signer, something like:

type HashSigner interface {
  HashSign(rand, message io.Reader, opts SignerOpts) (signature []byte, err error)
}

and can be used like:

var sig []byte
var err error

if hs, ok := priv.(crypto.HashSigner); ok {
  sig, err = hs.HashSign(config.rand(), bytes.NewReader(msg), signOpts)
  ...
}

signed := ... // calculate hash of msg
sig, err = priv.Sign(config.rand(), signed, signOpts)
...

as suggested in https://go.dev/blog/module-compatibility#working-with-interfaces

Copy link
Collaborator

@qmuntal qmuntal left a comment

Choose a reason for hiding this comment

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

LGTM!

@ueno ueno requested a review from derekparker September 22, 2023 08:10
@ueno
Copy link
Collaborator Author

ueno commented Oct 10, 2023

The idea behind this is to allow one-shot signature interface to be defined alongside crypto.Signer, something like:

FYI, here is a proposal for this; I'll try to see how well the HashSign/HashVerify could align with it.

Copy link
Contributor

@derekparker derekparker left a comment

Choose a reason for hiding this comment

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

LGTM

@karianna
Copy link
Collaborator

@ueno - Assuming this is still valid, are you able to rebase?

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.

4 participants