ca: make orderID mandatory (#8119)

It was allowed to be empty for ACMEv1 requests, but those are long gone.

Also, move the IsAnyNilOrZero checks up to the RPC entry point.
This commit is contained in:
Jacob Hoffman-Andrews 2025-04-15 14:56:28 -07:00 committed by GitHub
parent 7a3feb2ceb
commit 3ddaa6770f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 28 additions and 34 deletions

View File

@ -307,11 +307,6 @@ var ocspStatusToCode = map[string]int{
//
// [issuance cycle]: https://github.com/letsencrypt/boulder/blob/main/docs/ISSUANCE-CYCLE.md
func (ca *certificateAuthorityImpl) issuePrecertificate(ctx context.Context, certProfile *certProfileWithID, issueReq *capb.IssueCertificateRequest) ([]byte, error) {
// issueReq.orderID may be zero, for ACMEv1 requests.
if core.IsAnyNilOrZero(issueReq, issueReq.Csr, issueReq.RegistrationID) {
return nil, berrors.InternalServerError("Incomplete issue certificate request")
}
serialBigInt, err := ca.generateSerialNumber()
if err != nil {
return nil, err
@ -345,6 +340,10 @@ func (ca *certificateAuthorityImpl) issuePrecertificate(ctx context.Context, cer
}
func (ca *certificateAuthorityImpl) IssueCertificate(ctx context.Context, issueReq *capb.IssueCertificateRequest) (*capb.IssueCertificateResponse, error) {
if core.IsAnyNilOrZero(issueReq, issueReq.Csr, issueReq.RegistrationID, issueReq.OrderID) {
return nil, berrors.InternalServerError("Incomplete issue certificate request")
}
if ca.sctClient == nil {
return nil, errors.New("IssueCertificate called with a nil SCT service")
}
@ -398,13 +397,9 @@ func (ca *certificateAuthorityImpl) issueCertificateForPrecertificate(ctx contex
certProfile *certProfileWithID,
precertDER []byte,
sctBytes [][]byte,
regID int64, //nolint: unparam // unparam says "regID` always receives `arbitraryRegID` (`1001`)", which is wrong; that's just what happens in the unittests.
orderID int64, //nolint: unparam // same as above
regID int64,
orderID int64,
) ([]byte, error) {
if core.IsAnyNilOrZero(certProfile, precertDER, sctBytes, regID) {
return nil, berrors.InternalServerError("Incomplete cert for precertificate request")
}
precert, err := x509.ParseCertificate(precertDER)
if err != nil {
return nil, err

View File

@ -11,6 +11,7 @@ import (
"errors"
"fmt"
"math/big"
mrand "math/rand"
"os"
"strings"
"testing"
@ -92,8 +93,6 @@ var (
OIDExtensionSCTList = asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 11129, 2, 4, 2}
)
const arbitraryRegID int64 = 1001
func mustRead(path string) []byte {
return must.Do(os.ReadFile(path))
}
@ -332,7 +331,7 @@ func TestIssuePrecertificate(t *testing.T) {
t.Parallel()
req, err := x509.ParseCertificateRequest(testCase.csr)
test.AssertNotError(t, err, "Certificate request failed to parse")
issueReq := &capb.IssueCertificateRequest{Csr: testCase.csr, RegistrationID: arbitraryRegID}
issueReq := &capb.IssueCertificateRequest{Csr: testCase.csr, RegistrationID: mrand.Int63(), OrderID: mrand.Int63()}
profile := ca.certProfiles.profileByName["legacy"]
certDER, err := ca.issuePrecertificate(ctx, profile, issueReq)
@ -446,7 +445,7 @@ func TestMultipleIssuers(t *testing.T) {
// Test that an RSA CSR gets issuance from an RSA issuer.
profile := ca.certProfiles.profileByName["legacy"]
issuedCertDER, err := ca.issuePrecertificate(ctx, profile, &capb.IssueCertificateRequest{Csr: CNandSANCSR, RegistrationID: arbitraryRegID})
issuedCertDER, err := ca.issuePrecertificate(ctx, profile, &capb.IssueCertificateRequest{Csr: CNandSANCSR, RegistrationID: mrand.Int63(), OrderID: mrand.Int63()})
test.AssertNotError(t, err, "Failed to issue certificate")
cert, err := x509.ParseCertificate(issuedCertDER)
test.AssertNotError(t, err, "Certificate failed to parse")
@ -462,7 +461,7 @@ func TestMultipleIssuers(t *testing.T) {
test.AssertMetricWithLabelsEquals(t, ca.metrics.signatureCount, prometheus.Labels{"purpose": "precertificate", "status": "success"}, 1)
// Test that an ECDSA CSR gets issuance from an ECDSA issuer.
issuedCertDER, err = ca.issuePrecertificate(ctx, profile, &capb.IssueCertificateRequest{Csr: ECDSACSR, RegistrationID: arbitraryRegID, CertProfileName: "legacy"})
issuedCertDER, err = ca.issuePrecertificate(ctx, profile, &capb.IssueCertificateRequest{Csr: ECDSACSR, RegistrationID: mrand.Int63(), OrderID: mrand.Int63(), CertProfileName: "legacy"})
test.AssertNotError(t, err, "Failed to issue certificate")
cert, err = x509.ParseCertificate(issuedCertDER)
test.AssertNotError(t, err, "Certificate failed to parse")
@ -527,7 +526,7 @@ func TestUnpredictableIssuance(t *testing.T) {
// trials, the probability that all 20 issuances come from the same issuer is
// 0.5 ^ 20 = 9.5e-7 ~= 1e-6 = 1 in a million, so we do not consider this test
// to be flaky.
req := &capb.IssueCertificateRequest{Csr: ECDSACSR, RegistrationID: arbitraryRegID, CertProfileName: "legacy"}
req := &capb.IssueCertificateRequest{Csr: ECDSACSR, RegistrationID: mrand.Int63(), OrderID: mrand.Int63()}
seenE2 := false
seenR3 := false
profile := ca.certProfiles.profileByName["legacy"]
@ -713,7 +712,7 @@ func TestInvalidCSRs(t *testing.T) {
t.Parallel()
serializedCSR := mustRead(testCase.csrPath)
profile := ca.certProfiles.profileByName["legacy"]
issueReq := &capb.IssueCertificateRequest{Csr: serializedCSR, RegistrationID: arbitraryRegID, CertProfileName: "legacy"}
issueReq := &capb.IssueCertificateRequest{Csr: serializedCSR, RegistrationID: mrand.Int63(), OrderID: mrand.Int63(), CertProfileName: "legacy"}
_, err = ca.issuePrecertificate(ctx, profile, issueReq)
test.AssertErrorIs(t, err, testCase.errorType)
@ -751,7 +750,7 @@ func TestRejectValidityTooLong(t *testing.T) {
// Test that the CA rejects CSRs that would expire after the intermediate cert
profile := ca.certProfiles.profileByName["legacy"]
_, err = ca.issuePrecertificate(ctx, profile, &capb.IssueCertificateRequest{Csr: CNandSANCSR, RegistrationID: arbitraryRegID, CertProfileName: "legacy"})
_, err = ca.issuePrecertificate(ctx, profile, &capb.IssueCertificateRequest{Csr: CNandSANCSR, RegistrationID: mrand.Int63(), OrderID: mrand.Int63(), CertProfileName: "legacy"})
test.AssertError(t, err, "Cannot issue a certificate that expires after the intermediate certificate")
test.AssertErrorIs(t, err, berrors.InternalServer)
}
@ -844,7 +843,7 @@ func TestIssueCertificateForPrecertificate(t *testing.T) {
test.AssertNotError(t, err, "Failed to create CA")
profile := ca.certProfiles.profileByName["legacy"]
issueReq := capb.IssueCertificateRequest{Csr: CNandSANCSR, RegistrationID: arbitraryRegID, OrderID: 0, CertProfileName: "legacy"}
issueReq := capb.IssueCertificateRequest{Csr: CNandSANCSR, RegistrationID: mrand.Int63(), OrderID: mrand.Int63(), CertProfileName: "legacy"}
precertDER, err := ca.issuePrecertificate(ctx, profile, &issueReq)
test.AssertNotError(t, err, "Failed to issue precert")
parsedPrecert, err := x509.ParseCertificate(precertDER)
@ -868,8 +867,8 @@ func TestIssueCertificateForPrecertificate(t *testing.T) {
profile,
precertDER,
sctBytes,
arbitraryRegID,
0)
mrand.Int63(),
mrand.Int63())
test.AssertNotError(t, err, "Failed to issue cert from precert")
parsedCert, err := x509.ParseCertificate(certDER)
test.AssertNotError(t, err, "Failed to parse cert")
@ -911,8 +910,8 @@ func TestIssueCertificateForPrecertificateWithSpecificCertificateProfile(t *test
issueReq := capb.IssueCertificateRequest{
Csr: CNandSANCSR,
RegistrationID: arbitraryRegID,
OrderID: 0,
RegistrationID: mrand.Int63(),
OrderID: mrand.Int63(),
CertProfileName: selectedProfile,
}
precertDER, err := ca.issuePrecertificate(ctx, certProfile, &issueReq)
@ -938,8 +937,8 @@ func TestIssueCertificateForPrecertificateWithSpecificCertificateProfile(t *test
certProfile,
precertDER,
sctBytes,
arbitraryRegID,
0)
mrand.Int63(),
mrand.Int63())
test.AssertNotError(t, err, "Failed to issue cert from precert")
parsedCert, err := x509.ParseCertificate(certDER)
test.AssertNotError(t, err, "Failed to parse cert")
@ -1026,7 +1025,7 @@ func TestIssueCertificateForPrecertificateDuplicateSerial(t *testing.T) {
}
profile := ca.certProfiles.profileByName["legacy"]
issueReq := capb.IssueCertificateRequest{Csr: CNandSANCSR, RegistrationID: arbitraryRegID, OrderID: 0, CertProfileName: "legacy"}
issueReq := capb.IssueCertificateRequest{Csr: CNandSANCSR, RegistrationID: mrand.Int63(), OrderID: mrand.Int63(), CertProfileName: "legacy"}
precertDER, err := ca.issuePrecertificate(ctx, profile, &issueReq)
test.AssertNotError(t, err, "Failed to issue precert")
test.AssertMetricWithLabelsEquals(t, ca.metrics.signatureCount, prometheus.Labels{"purpose": "precertificate", "status": "success"}, 1)
@ -1034,9 +1033,8 @@ func TestIssueCertificateForPrecertificateDuplicateSerial(t *testing.T) {
profile,
precertDER,
sctBytes,
arbitraryRegID,
0,
)
mrand.Int63(),
mrand.Int63())
if err == nil {
t.Error("Expected error issuing duplicate serial but got none.")
}
@ -1068,8 +1066,8 @@ func TestIssueCertificateForPrecertificateDuplicateSerial(t *testing.T) {
profile,
precertDER,
sctBytes,
arbitraryRegID,
0)
mrand.Int63(),
mrand.Int63())
if err == nil {
t.Fatal("Expected error issuing duplicate serial but got none.")
}

View File

@ -4,6 +4,7 @@ import (
"context"
"crypto/x509"
"encoding/hex"
mrand "math/rand"
"testing"
"time"
@ -47,7 +48,7 @@ func TestOCSP(t *testing.T) {
profile := ca.certProfiles.profileByName["legacy"]
// Issue a certificate from an RSA issuer, request OCSP from the same issuer,
// and make sure it works.
rsaCertDER, err := ca.issuePrecertificate(ctx, profile, &capb.IssueCertificateRequest{Csr: CNandSANCSR, RegistrationID: arbitraryRegID, CertProfileName: "legacy"})
rsaCertDER, err := ca.issuePrecertificate(ctx, profile, &capb.IssueCertificateRequest{Csr: CNandSANCSR, RegistrationID: mrand.Int63(), OrderID: mrand.Int63(), CertProfileName: "legacy"})
test.AssertNotError(t, err, "Failed to issue certificate")
rsaCert, err := x509.ParseCertificate(rsaCertDER)
test.AssertNotError(t, err, "Failed to parse rsaCert")
@ -70,7 +71,7 @@ func TestOCSP(t *testing.T) {
// Issue a certificate from an ECDSA issuer, request OCSP from the same issuer,
// and make sure it works.
ecdsaCertDER, err := ca.issuePrecertificate(ctx, profile, &capb.IssueCertificateRequest{Csr: ECDSACSR, RegistrationID: arbitraryRegID, CertProfileName: "legacy"})
ecdsaCertDER, err := ca.issuePrecertificate(ctx, profile, &capb.IssueCertificateRequest{Csr: ECDSACSR, RegistrationID: mrand.Int63(), OrderID: mrand.Int63(), CertProfileName: "legacy"})
test.AssertNotError(t, err, "Failed to issue certificate")
ecdsaCert, err := x509.ParseCertificate(ecdsaCertDER)
test.AssertNotError(t, err, "Failed to parse ecdsaCert")