crypto: single-shot signing interface

[ueno]

Abstract

This proposal introduces a new interface for creating digital signatures in a single-shot manner, by combining message hashing and signing as an inseparable operation. The new interface can coexist alongside the existing crypto.Signer interface, while providing a clear indication that an entire message is expected as the input.

Background

The crypto.Signer interface is defined as follows:

type Signer interface {
	Public() PublicKey
	Sign(rand io.Reader, digest []byte, opts SignerOpts) (signature []byte, err error)
}

This interface is implemented for private keys in the RSA (crypto/rsa) and ECDSA (crypto/ecdsa) modules, as well as Ed25519 (crypto/ed25519), which expects the second parameter digest to be either an entire message (Ed25519) or the hash over it (Ed25519ph), depending on whether a special hash function (crypto.Hash(0)) is specified in opts. This design has the following drawbacks:

• Code readability: it is not obvious whether the Sign function operates on a digest or a message from the calling sites, as the interpretation of opts is up to the implementation of crypto.Signer
• Memory consumption: when the second parameter is a message, the entire content needs to be loaded before calling the Sign function, which can consume unreasonable amount of memory
• FIPS compliance: FIPS 140-3 requires both hashing and signing operations to be performed within the same cryptographic module boundary. With the crypto.Signer interface, it is not straightforward to enforce the requirement

Proposal

This proposal describes a new interface with a single SignMessage function which takes io.Reader as the input:

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

This interface can be used in a backward compatible manner, by checking the implementation at run-time, as suggested in https://go.dev/blog/module-compatibility#working-with-interfaces:

// in crypto/ed25519/ed25519.go:
func (priv PrivateKey) SignMessage(rand, message io.Reader, opts crypto.SignerOpts) (signature []byte, err error) {
	...
}

// in crypto/tls/handshake_client_tls13.go
func (hs *clientHandshakeStateTLS13) sendClientCertificate() error {
	…
	if ms, ok := cert.PrivateKey.(crypto.MessageSigner); ok {
    		// Use single-shot ms.SignMessage without hashing.
	} else if hs, ok := cert.PrivateKey.(crypto.Signer); ok {
    		// Use hs.Sign after hashing the message.
	} else {
		// Return an error.
	}
}

Rationale

This approach is non-invasive as the use of the new interface is on an opt-in basis. One shortcoming is the amount of code to be rewritten using the single-shot interface for FIPS compliance.

Compatibility

This change would maintain backward compatibility.

Implementation

There is a preliminary implementation of this proposal at [1], though it currently simply calls the Sign function in crypto.Signer underneath, which could be either optimized by reading a message in a piecemeal fashion or deferred to the single-shot signing API in BoringSSL.

1. master...ueno:go:wip/single-shot-signing

[rittneje]

If this proposal is accepted, I think SignMessage should also take context.Context, for the same reasons mentioned in #56508.

[seankhliao]

cc @golang/security

[rolandshoemaker]

Ideally, I think we'd want to try to take advantage of the SignerOpts to trigger a whole message signer, but there are a number of places in the standard library where we pass through a signer, but the SignerOpts are generated internally, so the caller has no control.

I think our only option here is to introduce a new interface, and do interface upgrades internally where we currently use Signer.

The new interface should probably be a superset of Signer though, so that in places where we currently specific the interface as a Signer (instead of any), it can still be passed. Implementations that don't want to use the Sign method at all can just implement a stub that returns an error.

E.g.

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

In terms of naming, WholeMessageSigner might be more explicit about it's purpose, but I'm not strongly opinionated.

[rsc]

It seems like message should be a []byte, not a io.Reader.
And it seems like maybe we should drop rand, although maybe not.
(I think in general we are moving toward these routines ignoring rand and hard-coding use of crypto/rand.Reader.)

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[rsc]

I believe the latest version of the proposal is:

type MessageSigner interface {
	Signer
	SignMessage(rand io.Reader, msg []byte, opts SignerOpts) (signature []byte, err error)
}

which expands to

type MessageSigner interface {
	Public() PublicKey
	Sign(rand io.Reader, digest []byte, opts SignerOpts) (signature []byte, err error)
	SignMessage(rand io.Reader, msg []byte, opts SignerOpts) (signature []byte, err error)
}

I'm a little confused though, since SignerOpts says:

type SignerOpts interface {
	// HashFunc returns an identifier for the hash function used to produce
	// the message passed to Signer.Sign, or else zero to indicate that no
	// hashing was done.
	HashFunc() Hash
}

The "or else zero..." suggests we've already contemplated Sign calls getting the full message by making HashFunc return nil.

[rsc]

Talked to Roland a bit more, and I see that since Signer is something implemented by PrivateKey, it's not otherwise easy to pass the hash function through, and most of the shoehorning one can imagine would end up too easily to break silently. The separate method is more explicit.

Looking at the top message in this issue, there is a claim that "when the second parameter is a message, the entire content needs to be loaded before calling the Sign function, which can consume unreasonable amount of memory". At least for x509.CreateCertificate this is not true. Are there any cases where the message really can't fit in memory? It's kind of annoying to pass an io.Reader since that implies more copies, although I suppose all implementations could be responsible for detecting a bytes.Reader and special-casing that type.

Finally, any time we add an optional interface like this, we should also add a function to help use it (like fs.GlobFS has fs.Glob). In this case the helper function would be

func SignMessage(signer Signer, rand io.Reader, msg []byte, opts SignerOpts) (signature []byte, err error)

That should be included in the proposal.

[rolandshoemaker]

Proposal as it currently stands:

// MessageSigner is an interface for an opaque private key that can be used for signing
// operations where the message should not be pre-hashed by the caller. It is a superset
// of the Signer interface so that it can be passed to APIs which accept Signer, which may
// try to do an interface upgrade.
//
// Implementations which must not use the pre-hashing Sign API should implement
// Signer.Sign to always return an error, or panic.
type MessageSigner interface {
	Signer
	SignMessage(rand io.Reader, msg []byte, opts SignerOpts) (signature []byte, err error)
}

// SignMessage takes a Signer and attempts an interface upgrade to MessageSigner, calling signer.SignMessage
// if successful, or signer.Sign if not, and passes the relevant arguments.
func SignMessage(signer Signer, rand io.Reader, msg []byte, opts SignerOpts) (signature []byte, err error)

I think for the use cases in the standard library (crypto/x509, crypto/tls) the messages are small enough that it should
be fine to pass them in directly, without the need for an io.Reader. It seems complicated enough (and divergent from our other APIs) to not worry about until someone has a specific use case for giant messages.