diff --git a/ra/ra.go b/ra/ra.go index e9d92917c88..3006d55b967 100644 --- a/ra/ra.go +++ b/ra/ra.go @@ -1138,9 +1138,16 @@ func (ra *RegistrationAuthorityImpl) issueCertificateOuter( logEvent.PreviousCertificateIssued = timestamps.Timestamps[0].AsTime() } + // Ensure that we always provide a profile name to the CA, even if the order + // didn't request a specific profile. + profileName := order.CertificateProfileName + if profileName == "" { + profileName = ra.defaultProfileName + } + // Step 3: Issue the Certificate cert, cpId, err := ra.issueCertificateInner( - ctx, csr, isRenewal, order.CertificateProfileName, accountID(order.RegistrationID), orderID(order.Id)) + ctx, csr, isRenewal, profileName, accountID(order.RegistrationID), orderID(order.Id)) // Step 4: Fail the order if necessary, and update metrics and log fields var result string diff --git a/ra/ra_test.go b/ra/ra_test.go index faa05fd36b7..d9d0c2852bf 100644 --- a/ra/ra_test.go +++ b/ra/ra_test.go @@ -10,6 +10,7 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/asn1" + "encoding/hex" "encoding/json" "encoding/pem" "errors" @@ -3366,11 +3367,12 @@ func (sa *mockSAWithFinalize) FQDNSetTimestampsForWindow(ctx context.Context, in }, nil } -func TestIssueCertificateInnerWithProfile(t *testing.T) { +func TestIssueCertificateOuter(t *testing.T) { _, _, ra, _, fc, cleanup := initAuthorities(t) defer cleanup() + ra.SA = &mockSAWithFinalize{} - // Generate a reasonable-looking CSR and cert to pass the matchesCSR check. + // Create a CSR to submit and a certificate for the fake CA to return. testKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) test.AssertNotError(t, err, "generating test key") csrDER, err := x509.CreateCertificateRequest(rand.Reader, &x509.CertificateRequest{DNSNames: []string{"example.com"}}, testKey) @@ -3387,71 +3389,68 @@ func TestIssueCertificateInnerWithProfile(t *testing.T) { test.AssertNotError(t, err, "creating test cert") certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) - // Use a mock CA that will record the profile name and profile hash included - // in the RA's request messages. Populate it with the cert generated above. - mockCA := MockCARecordingProfile{inner: &mocks.MockCA{PEM: certPEM}} - ra.CA = &mockCA - - ra.SA = &mockSAWithFinalize{} - - // Call issueCertificateInner with the CSR generated above and the profile - // name "default", which will cause the mockCA to return a specific hash. - _, cpId, err := ra.issueCertificateInner(context.Background(), csr, false, "default", 1, 1) - test.AssertNotError(t, err, "issuing cert with profile name") - test.AssertEquals(t, mockCA.profileName, cpId.name) - test.AssertByteEquals(t, mockCA.profileHash, cpId.hash) -} - -func TestIssueCertificateOuter(t *testing.T) { - _, sa, ra, _, fc, cleanup := initAuthorities(t) - defer cleanup() - - // Make some valid authorizations for some names - names := []string{"not-example.com", "www.not-example.com", "still.not-example.com", "definitely.not-example.com"} - exp := ra.clk.Now().Add(ra.validationProfiles[ra.defaultProfileName].orderLifetime) - var authzIDs []int64 - for _, name := range names { - authzIDs = append(authzIDs, createFinalizedAuthorization(t, sa, name, exp, core.ChallengeTypeHTTP01, ra.clk.Now())) - } - - // Create a pending order for all of the names - order, err := sa.NewOrderAndAuthzs(context.Background(), &sapb.NewOrderAndAuthzsRequest{ - NewOrder: &sapb.NewOrderRequest{ - RegistrationID: Registration.Id, - Expires: timestamppb.New(exp), - DnsNames: names, - V2Authorizations: authzIDs, - CertificateProfileName: "philsProfile", + for _, tc := range []struct { + name string + profile string + wantProfile string + wantHash string + }{ + { + name: "select default profile when none specified", + wantProfile: "test", // matches ra.defaultProfileName + wantHash: "9f86d081884c7d65", }, - }) - test.AssertNotError(t, err, "Could not add test order with finalized authz IDs") + { + name: "default profile specified", + profile: "test", + wantProfile: "test", + wantHash: "9f86d081884c7d65", + }, + { + name: "other profile specified", + profile: "other", + wantProfile: "other", + wantHash: "d9298a10d1b07358", + }, + } { + t.Run(tc.name, func(t *testing.T) { + // Use a mock CA that will record the profile name and profile hash included + // in the RA's request messages. Populate it with the cert generated above. + mockCA := MockCARecordingProfile{inner: &mocks.MockCA{PEM: certPEM}} + ra.CA = &mockCA - testKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - test.AssertNotError(t, err, "generating test key") - csrDER, err := x509.CreateCertificateRequest(rand.Reader, &x509.CertificateRequest{DNSNames: []string{"example.com"}}, testKey) - test.AssertNotError(t, err, "creating test csr") - csr, err := x509.ParseCertificateRequest(csrDER) - test.AssertNotError(t, err, "parsing test csr") - certDER, err := x509.CreateCertificate(rand.Reader, &x509.Certificate{ - SerialNumber: big.NewInt(1), - DNSNames: []string{"example.com"}, - NotBefore: fc.Now(), - BasicConstraintsValid: true, - ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}, - }, &x509.Certificate{}, testKey.Public(), testKey) - test.AssertNotError(t, err, "creating test cert") - certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) + order := &corepb.Order{ + RegistrationID: Registration.Id, + Expires: timestamppb.New(fc.Now().Add(24 * time.Hour)), + DnsNames: []string{"example.com"}, + CertificateProfileName: tc.profile, + } - // Use a mock CA that will record the profile name and profile hash included - // in the RA's request messages. Populate it with the cert generated above. - mockCA := MockCARecordingProfile{inner: &mocks.MockCA{PEM: certPEM}} - ra.CA = &mockCA + order, err = ra.issueCertificateOuter(context.Background(), order, csr, certificateRequestEvent{}) - ra.SA = &mockSAWithFinalize{} + // The resulting order should have new fields populated + if order.Status != string(core.StatusValid) { + t.Errorf("order.Status = %+v, want %+v", order.Status, core.StatusValid) + } + if order.CertificateSerial != core.SerialToString(big.NewInt(1)) { + t.Errorf("CertificateSerial = %+v, want %+v", order.CertificateSerial, 1) + } - _, err = ra.issueCertificateOuter(context.Background(), order, csr, certificateRequestEvent{}) - test.AssertNotError(t, err, "Could not issue certificate") - test.AssertMetricWithLabelsEquals(t, ra.newCertCounter, prometheus.Labels{"profileName": mockCA.profileName, "profileHash": fmt.Sprintf("%x", mockCA.profileHash)}, 1) + // The recorded profile and profile hash should match what we expect. + if mockCA.profileName != tc.wantProfile { + t.Errorf("recorded profileName = %+v, want %+v", mockCA.profileName, tc.wantProfile) + } + wantHash, err := hex.DecodeString(tc.wantHash) + if err != nil { + t.Fatalf("decoding test hash: %s", err) + } + if !bytes.Equal(mockCA.profileHash, wantHash) { + t.Errorf("recorded profileName = %x, want %x", mockCA.profileHash, wantHash) + } + test.AssertMetricWithLabelsEquals(t, ra.newCertCounter, prometheus.Labels{"profileName": tc.wantProfile, "profileHash": tc.wantHash}, 1) + ra.newCertCounter.Reset() + }) + } } func TestNewOrderMaxNames(t *testing.T) {