Merge branch 'main' into updatereg-1

ca/ca.go

@@ -15,7 +15,6 @@ import (
 	"fmt"
 	"math/big"
 	mrand "math/rand/v2"
-	"strings"
 	"time"
 
 	ct "github.com/google/certificate-transparency-go"
@@ -51,6 +50,25 @@ const (
 	certType    = certificateType("certificate")
 )
 
+// issuanceEvent is logged before and after issuance of precertificates and certificates.
+// The `omitempty` fields are not always present.
+// CSR, Precertificate, and Certificate are hex-encoded DER bytes to make it easier to
+// ad-hoc search for sequences or OIDs in logs. Other data, like public key within CSR,
+// is logged as base64 because it doesn't have interesting DER structure.
+type issuanceEvent struct {
+	CSR             string `json:",omitempty"`
+	IssuanceRequest *issuance.IssuanceRequest
+	Issuer          string
+	OrderID         int64
+	Profile         string
+	ProfileHash     string
+	Requester       int64
+	Result          struct {
+		Precertificate string `json:",omitempty"`
+		Certificate    string `json:",omitempty"`
+	}
+}
+
 // Two maps of keys to Issuers. Lookup by PublicKeyAlgorithm is useful for
 // determining the set of issuers which can sign a given (pre)cert, based on its
 // PublicKeyAlgorithm. Lookup by NameID is useful for looking up a specific
@@ -428,17 +446,22 @@ func (ca *certificateAuthorityImpl) IssueCertificateForPrecertificate(ctx contex
 		return nil, err
 	}
 
-	names := strings.Join(issuanceReq.DNSNames, ", ")
-	ca.log.AuditInfof("Signing cert: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certProfileName=[%s] certProfileHash=[%x] precert=[%s]",
-		issuer.Name(), serialHex, req.RegistrationID, names, certProfile.name, certProfile.hash, hex.EncodeToString(precert.Raw))
-
 	lintCertBytes, issuanceToken, err := issuer.Prepare(certProfile.profile, issuanceReq)
 	if err != nil {
-		ca.log.AuditErrf("Preparing cert failed: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certProfileName=[%s] certProfileHash=[%x] err=[%v]",
-			issuer.Name(), serialHex, req.RegistrationID, names, certProfile.name, certProfile.hash, err)
+		ca.log.AuditErrf("Preparing cert failed: serial=[%s] err=[%v]", serialHex, err)
 		return nil, berrors.InternalServerError("failed to prepare certificate signing: %s", err)
 	}
 
+	logEvent := issuanceEvent{
+		IssuanceRequest: issuanceReq,
+		Issuer:          issuer.Name(),
+		OrderID:         req.OrderID,
+		Profile:         certProfile.name,
+		ProfileHash:     hex.EncodeToString(certProfile.hash[:]),
+		Requester:       req.RegistrationID,
+	}
+	ca.log.AuditObject("Signing cert", logEvent)
+
 	_, span := ca.tracer.Start(ctx, "signing cert", trace.WithAttributes(
 		attribute.String("serial", serialHex),
 		attribute.String("issuer", issuer.Name()),
@@ -448,8 +471,7 @@ func (ca *certificateAuthorityImpl) IssueCertificateForPrecertificate(ctx contex
 	certDER, err := issuer.Issue(issuanceToken)
 	if err != nil {
 		ca.metrics.noteSignError(err)
-		ca.log.AuditErrf("Signing cert failed: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certProfileName=[%s] certProfileHash=[%x] err=[%v]",
-			issuer.Name(), serialHex, req.RegistrationID, names, certProfile.name, certProfile.hash, err)
+		ca.log.AuditErrf("Signing cert failed: serial=[%s] err=[%v]", serialHex, err)
 		span.SetStatus(codes.Error, err.Error())
 		span.End()
 		return nil, berrors.InternalServerError("failed to sign certificate: %s", err)
@@ -462,17 +484,16 @@ func (ca *certificateAuthorityImpl) IssueCertificateForPrecertificate(ctx contex
 	}
 
 	ca.metrics.signatureCount.With(prometheus.Labels{"purpose": string(certType), "issuer": issuer.Name()}).Inc()
-	ca.log.AuditInfof("Signing cert success: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certificate=[%s] certProfileName=[%s] certProfileHash=[%x]",
-		issuer.Name(), serialHex, req.RegistrationID, names, hex.EncodeToString(certDER), certProfile.name, certProfile.hash)
+	logEvent.Result.Certificate = hex.EncodeToString(certDER)
+	ca.log.AuditObject("Signing cert success", logEvent)
 
 	_, err = ca.sa.AddCertificate(ctx, &sapb.AddCertificateRequest{
 		Der:    certDER,
 		RegID:  req.RegistrationID,
 		Issued: timestamppb.New(ca.clk.Now()),
 	})
 	if err != nil {
-		ca.log.AuditErrf("Failed RPC to store at SA: issuer=[%s] serial=[%s] cert=[%s] regID=[%d] orderID=[%d] certProfileName=[%s] certProfileHash=[%x] err=[%v]",
-			issuer.Name(), serialHex, hex.EncodeToString(certDER), req.RegistrationID, req.OrderID, certProfile.name, certProfile.hash, err)
+		ca.log.AuditErrf("Failed RPC to store at SA: serial=[%s] err=[%v]", serialHex, hex.EncodeToString(certDER))
 		return nil, err
 	}
 
@@ -568,7 +589,7 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
 
 	names := csrlib.NamesFromCSR(csr)
 	req := &issuance.IssuanceRequest{
-		PublicKey:         csr.PublicKey,
+		PublicKey:         issuance.MarshalablePublicKey{PublicKey: csr.PublicKey},
 		SubjectKeyId:      subjectKeyId,
 		Serial:            serialBigInt.Bytes(),
 		DNSNames:          names.SANs,
@@ -579,19 +600,20 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
 		NotAfter:          notAfter,
 	}
 
-	ca.log.AuditInfof("Signing precert: serial=[%s] regID=[%d] names=[%s] csr=[%s]",
-		serialHex, issueReq.RegistrationID, strings.Join(req.DNSNames, ", "), hex.EncodeToString(csr.Raw))
-
 	lintCertBytes, issuanceToken, err := issuer.Prepare(certProfile.profile, req)
 	if err != nil {
-		ca.log.AuditErrf("Preparing precert failed: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certProfileName=[%s] certProfileHash=[%x] err=[%v]",
-			issuer.Name(), serialHex, issueReq.RegistrationID, strings.Join(req.DNSNames, ", "), certProfile.name, certProfile.hash, err)
+		ca.log.AuditErrf("Preparing precert failed: serial=[%s] err=[%v]", serialHex, err)
 		if errors.Is(err, linter.ErrLinting) {
 			ca.metrics.lintErrorCount.Inc()
 		}
 		return nil, nil, berrors.InternalServerError("failed to prepare precertificate signing: %s", err)
 	}
 
+	// Note: we write the linting certificate bytes to this table, rather than the precertificate
+	// (which we audit log but do not put in the database). This is to ensure that even if there is
+	// an error immediately after signing the precertificate, we have a record in the DB of what we
+	// intended to sign, and can do revocations based on that. See #6807.
+	// The name of the SA method ("AddPrecertificate") is a historical artifact.
 	_, err = ca.sa.AddPrecertificate(context.Background(), &sapb.AddCertificateRequest{
 		Der:          lintCertBytes,
 		RegID:        issueReq.RegistrationID,
@@ -603,6 +625,17 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
 		return nil, nil, err
 	}
 
+	logEvent := issuanceEvent{
+		CSR:             hex.EncodeToString(csr.Raw),
+		IssuanceRequest: req,
+		Issuer:          issuer.Name(),
+		Profile:         certProfile.name,
+		ProfileHash:     hex.EncodeToString(certProfile.hash[:]),
+		Requester:       issueReq.RegistrationID,
+		OrderID:         issueReq.OrderID,
+	}
+	ca.log.AuditObject("Signing precert", logEvent)
+
 	_, span := ca.tracer.Start(ctx, "signing precert", trace.WithAttributes(
 		attribute.String("serial", serialHex),
 		attribute.String("issuer", issuer.Name()),
@@ -612,8 +645,7 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
 	certDER, err := issuer.Issue(issuanceToken)
 	if err != nil {
 		ca.metrics.noteSignError(err)
-		ca.log.AuditErrf("Signing precert failed: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certProfileName=[%s] certProfileHash=[%x] err=[%v]",
-			issuer.Name(), serialHex, issueReq.RegistrationID, strings.Join(req.DNSNames, ", "), certProfile.name, certProfile.hash, err)
+		ca.log.AuditErrf("Signing precert failed: serial=[%s] err=[%v]", serialHex, err)
 		span.SetStatus(codes.Error, err.Error())
 		span.End()
 		return nil, nil, berrors.InternalServerError("failed to sign precertificate: %s", err)
@@ -626,8 +658,11 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
 	}
 
 	ca.metrics.signatureCount.With(prometheus.Labels{"purpose": string(precertType), "issuer": issuer.Name()}).Inc()
-	ca.log.AuditInfof("Signing precert success: issuer=[%s] serial=[%s] regID=[%d] names=[%s] precert=[%s] certProfileName=[%s] certProfileHash=[%x]",
-		issuer.Name(), serialHex, issueReq.RegistrationID, strings.Join(req.DNSNames, ", "), hex.EncodeToString(certDER), certProfile.name, certProfile.hash)
+
+	logEvent.Result.Precertificate = hex.EncodeToString(certDER)
+	// The CSR is big and not that informative, so don't log it a second time.
+	logEvent.CSR = ""
+	ca.log.AuditObject("Signing precert success", logEvent)
 
 	return certDER, &certProfileWithID{certProfile.name, certProfile.hash, nil}, nil
 }

ca/ca_test.go

@@ -332,7 +332,6 @@ func TestIssuePrecertificate(t *testing.T) {
 
 				var certDER []byte
 				response, err := ca.IssuePrecertificate(ctx, issueReq)
-
 				test.AssertNotError(t, err, "Failed to issue precertificate")
 				certDER = response.DER
 

cmd/admin/key.go

@@ -3,7 +3,9 @@ package main
 import (
 	"bufio"
 	"context"
+	"crypto/x509"
 	"encoding/hex"
+	"encoding/pem"
 	"errors"
 	"flag"
 	"fmt"
@@ -26,9 +28,14 @@ import (
 type subcommandBlockKey struct {
 	parallelism uint
 	comment     string
-	privKey     string
-	spkiFile    string
-	certFile    string
+
+	privKey           string
+	spkiFile          string
+	certFile          string
+	csrFile           string
+	csrFileExpectedCN string
+
+	checkSignature bool
 }
 
 var _ subcommand = (*subcommandBlockKey)(nil)
@@ -46,6 +53,10 @@ func (s *subcommandBlockKey) Flags(flag *flag.FlagSet) {
 	flag.StringVar(&s.privKey, "private-key", "", "Block issuance for the pubkey corresponding to this private key")
 	flag.StringVar(&s.spkiFile, "spki-file", "", "Block issuance for all keys listed in this file as SHA256 hashes of SPKI, hex encoded, one per line")
 	flag.StringVar(&s.certFile, "cert-file", "", "Block issuance for the public key of the single PEM-formatted certificate in this file")
+	flag.StringVar(&s.csrFile, "csr-file", "", "Block issuance for the public key of the single PEM-formatted CSR in this file")
+	flag.StringVar(&s.csrFileExpectedCN, "csr-file-expected-cn", "The key that signed this CSR has been publicly disclosed. It should not be used for any purpose.", "The Subject CN of a CSR will be verified to match this before blocking")
+
+	flag.BoolVar(&s.checkSignature, "check-signature", true, "Check self-signature of CSR before revoking")
 }
 
 func (s *subcommandBlockKey) Run(ctx context.Context, a *admin) error {
@@ -56,6 +67,7 @@ func (s *subcommandBlockKey) Run(ctx context.Context, a *admin) error {
 		"-private-key": s.privKey != "",
 		"-spki-file":   s.spkiFile != "",
 		"-cert-file":   s.certFile != "",
+		"-csr-file":    s.csrFile != "",
 	}
 	maps.DeleteFunc(setInputs, func(_ string, v bool) bool { return !v })
 	if len(setInputs) == 0 {
@@ -75,6 +87,8 @@ func (s *subcommandBlockKey) Run(ctx context.Context, a *admin) error {
 		spkiHashes, err = a.spkiHashesFromFile(s.spkiFile)
 	case "-cert-file":
 		spkiHashes, err = a.spkiHashesFromCertPEM(s.certFile)
+	case "-csr-file":
+		spkiHashes, err = a.spkiHashFromCSRPEM(s.csrFile, s.checkSignature, s.csrFileExpectedCN)
 	default:
 		return errors.New("no recognized input method flag set (this shouldn't happen)")
 	}
@@ -146,6 +160,43 @@ func (a *admin) spkiHashesFromCertPEM(filename string) ([][]byte, error) {
 	return [][]byte{spkiHash[:]}, nil
 }
 
+func (a *admin) spkiHashFromCSRPEM(filename string, checkSignature bool, expectedCN string) ([][]byte, error) {
+	csrFile, err := os.ReadFile(filename)
+	if err != nil {
+		return nil, fmt.Errorf("reading CSR file %q: %w", filename, err)
+	}
+
+	data, _ := pem.Decode(csrFile)
+	if data == nil {
+		return nil, fmt.Errorf("no PEM data found in %q", filename)
+	}
+
+	a.log.AuditInfof("Parsing key to block from CSR PEM: %x", data)
+
+	csr, err := x509.ParseCertificateRequest(data.Bytes)
+	if err != nil {
+		return nil, fmt.Errorf("parsing CSR %q: %w", filename, err)
+	}
+
+	if checkSignature {
+		err = csr.CheckSignature()
+		if err != nil {
+			return nil, fmt.Errorf("checking CSR signature: %w", err)
+		}
+	}
+
+	if csr.Subject.CommonName != expectedCN {
+		return nil, fmt.Errorf("Got CSR CommonName %q, expected %q", csr.Subject.CommonName, expectedCN)
+	}
+
+	spkiHash, err := core.KeyDigest(csr.PublicKey)
+	if err != nil {
+		return nil, fmt.Errorf("computing SPKI hash: %w", err)
+	}
+
+	return [][]byte{spkiHash[:]}, nil
+}
+
 func (a *admin) blockSPKIHashes(ctx context.Context, spkiHashes [][]byte, comment string, parallelism uint) error {
 	u, err := user.Current()
 	if err != nil {

cmd/admin/key_test.go

@@ -68,6 +68,53 @@ func TestSPKIHashesFromFile(t *testing.T) {
 	}
 }
 
+// The key is the p256 test key from RFC9500
+const goodCSR = `
+-----BEGIN CERTIFICATE REQUEST-----
+MIG6MGICAQAwADBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABEIlSPiPt4L/teyj
+dERSxyoeVY+9b3O+XkjpMjLMRcWxbEzRDEy41bihcTnpSILImSVymTQl9BQZq36Q
+pCpJQnKgADAKBggqhkjOPQQDAgNIADBFAiBadw3gvL9IjUfASUTa7MvmkbC4ZCvl
+21m1KMwkIx/+CQIhAKvuyfCcdZ0cWJYOXCOb1OavolWHIUzgEpNGUWul6O0s
+-----END CERTIFICATE REQUEST-----
+`
+
+// TestCSR checks that we get the correct SPKI from a CSR, even if its signature is invalid
+func TestCSR(t *testing.T) {
+	expectedSPKIHash := "b2b04340cfaee616ec9c2c62d261b208e54bb197498df52e8cadede23ac0ba5e"
+
+	goodCSRFile := path.Join(t.TempDir(), "good.csr")
+	err := os.WriteFile(goodCSRFile, []byte(goodCSR), 0600)
+	test.AssertNotError(t, err, "writing good csr")
+
+	a := admin{log: blog.NewMock()}
+
+	goodHash, err := a.spkiHashFromCSRPEM(goodCSRFile, true, "")
+	test.AssertNotError(t, err, "expected to read CSR")
+
+	if len(goodHash) != 1 {
+		t.Fatalf("expected to read 1 SPKI from CSR, read %d", len(goodHash))
+	}
+	test.AssertEquals(t, hex.EncodeToString(goodHash[0]), expectedSPKIHash)
+
+	// Flip a bit, in the signature, to make a bad CSR:
+	badCSR := strings.Replace(goodCSR, "Wul6", "Wul7", 1)
+
+	csrFile := path.Join(t.TempDir(), "bad.csr")
+	err = os.WriteFile(csrFile, []byte(badCSR), 0600)
+	test.AssertNotError(t, err, "writing bad csr")
+
+	_, err = a.spkiHashFromCSRPEM(csrFile, true, "")
+	test.AssertError(t, err, "expected invalid signature")
+
+	badHash, err := a.spkiHashFromCSRPEM(csrFile, false, "")
+	test.AssertNotError(t, err, "expected to read CSR with bad signature")
+
+	if len(badHash) != 1 {
+		t.Fatalf("expected to read 1 SPKI from CSR, read %d", len(badHash))
+	}
+	test.AssertEquals(t, hex.EncodeToString(badHash[0]), expectedSPKIHash)
+}
+
 // mockSARecordingBlocks is a mock which only implements the AddBlockedKey gRPC
 // method.
 type mockSARecordingBlocks struct {

core/objects.go

@@ -297,13 +297,13 @@ func (ch Challenge) StringID() string {
 type Authorization struct {
 	// An identifier for this authorization, unique across
 	// authorizations and certificates within this instance.
-	ID string `json:"id,omitempty" db:"id"`
+	ID string `json:"-" db:"id"`
 
 	// The identifier for which authorization is being given
 	Identifier identifier.ACMEIdentifier `json:"identifier,omitempty" db:"identifier"`
 
 	// The registration ID associated with the authorization
-	RegistrationID int64 `json:"regId,omitempty" db:"registrationID"`
+	RegistrationID int64 `json:"-" db:"registrationID"`
 
 	// The status of the validation of this authorization
 	Status AcmeStatus `json:"status,omitempty" db:"status"`

features/features.go

@@ -103,12 +103,26 @@ type Config struct {
 	// This flag should only be used in conjunction with UseKvLimitsForNewOrder.
 	DisableLegacyLimitWrites bool
 
+	// PropagateCancels controls whether the WFE and ocsp-responder allows
+	// cancellation of an inbound request to cancel downstream gRPC and other
+	// queries. In practice, cancellation of an inbound request is achieved by
+	// Nginx closing the connection on which the request was happening. This may
+	// help shed load in overcapacity situations. However, note that in-progress
+	// database queries (for instance, in the SA) are not cancelled. Database
+	// queries waiting for an available connection may be cancelled.
+	PropagateCancels bool
+
 	// InsertAuthzsIndividually causes the SA's NewOrderAndAuthzs method to
 	// create each new authz one at a time, rather than using MultiInserter.
 	// Although this is expected to be a performance penalty, it is necessary to
 	// get the AUTO_INCREMENT ID of each new authz without relying on MariaDB's
 	// unique "INSERT ... RETURNING" functionality.
 	InsertAuthzsIndividually bool
+
+	// IncrementRateLimits uses Redis' IncrBy, instead of Set, for rate limit
+	// accounting. This catches and denies spikes of requests much more
+	// reliably.
+	IncrementRateLimits bool
 }
 
 var fMu = new(sync.RWMutex)