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

proposal: crypto: single-shot signing interface #63405

Open
ueno opened this issue Oct 6, 2023 · 2 comments
Open

proposal: crypto: single-shot signing interface #63405

ueno opened this issue Oct 6, 2023 · 2 comments
Labels
Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone

Comments

@ueno
Copy link
Contributor

ueno commented Oct 6, 2023

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
@ueno ueno added the Proposal label Oct 6, 2023
@gopherbot gopherbot added this to the Proposal milestone Oct 6, 2023
@rittneje
Copy link

rittneje commented Oct 6, 2023

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

@seankhliao seankhliao added the Proposal-Crypto Proposal related to crypto packages or other security issues label Oct 6, 2023
@seankhliao
Copy link
Member

cc @golang/security

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
Status: Incoming
Development

No branches or pull requests

4 participants