RA: Use Serial+IssuerID for revocation (#5376)
Update the RA's `revokeCertificate` method to identify the certificate to be revoked using its serial and issuer ID, rather than its full DER-encoded bytes. This removes one of the two remaining places that the certDER codepath is used. Also update the admin-revoker tests to properly set up an actual issuer, so that revocation works. Part of #5079
This commit is contained in:
		
							parent
							
								
									795348483e
								
							
						
					
					
						commit
						ab6d0b848a
					
				| 
						 | 
				
			
			@ -13,8 +13,10 @@ import (
 | 
			
		|||
	"time"
 | 
			
		||||
 | 
			
		||||
	"github.com/jmhodges/clock"
 | 
			
		||||
	akamaipb "github.com/letsencrypt/boulder/akamai/proto"
 | 
			
		||||
	capb "github.com/letsencrypt/boulder/ca/proto"
 | 
			
		||||
	"github.com/letsencrypt/boulder/core"
 | 
			
		||||
	corepb "github.com/letsencrypt/boulder/core/proto"
 | 
			
		||||
	"github.com/letsencrypt/boulder/goodkey"
 | 
			
		||||
	"github.com/letsencrypt/boulder/issuance"
 | 
			
		||||
	blog "github.com/letsencrypt/boulder/log"
 | 
			
		||||
| 
						 | 
				
			
			@ -37,6 +39,12 @@ func (ca *mockCA) GenerateOCSP(context.Context, *capb.GenerateOCSPRequest, ...gr
 | 
			
		|||
	return &capb.OCSPResponse{}, nil
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
type mockPurger struct{}
 | 
			
		||||
 | 
			
		||||
func (mp *mockPurger) Purge(context.Context, *akamaipb.PurgeRequest, ...grpc.CallOption) (*corepb.Empty, error) {
 | 
			
		||||
	return &corepb.Empty{}, nil
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
func TestRevokeBatch(t *testing.T) {
 | 
			
		||||
	log := blog.UseMock()
 | 
			
		||||
	fc := clock.NewFake()
 | 
			
		||||
| 
						 | 
				
			
			@ -53,6 +61,9 @@ func TestRevokeBatch(t *testing.T) {
 | 
			
		|||
	defer test.ResetSATestDatabase(t)
 | 
			
		||||
	reg := satest.CreateWorkingRegistration(t, ssa)
 | 
			
		||||
 | 
			
		||||
	issuer, err := core.LoadCert("../../test/test-ca.pem")
 | 
			
		||||
	test.AssertNotError(t, err, "Failed to load test issuer")
 | 
			
		||||
 | 
			
		||||
	ra := ra.NewRegistrationAuthorityImpl(fc,
 | 
			
		||||
		log,
 | 
			
		||||
		metrics.NoopRegisterer,
 | 
			
		||||
| 
						 | 
				
			
			@ -66,8 +77,8 @@ func TestRevokeBatch(t *testing.T) {
 | 
			
		|||
		nil,
 | 
			
		||||
		0,
 | 
			
		||||
		nil,
 | 
			
		||||
		nil,
 | 
			
		||||
		[]*issuance.Certificate{{Certificate: &x509.Certificate{}}},
 | 
			
		||||
		&mockPurger{},
 | 
			
		||||
		[]*issuance.Certificate{{Certificate: issuer}},
 | 
			
		||||
	)
 | 
			
		||||
	ra.SA = ssa
 | 
			
		||||
	ra.CA = &mockCA{}
 | 
			
		||||
| 
						 | 
				
			
			@ -84,7 +95,7 @@ func TestRevokeBatch(t *testing.T) {
 | 
			
		|||
			SerialNumber: serial,
 | 
			
		||||
			DNSNames:     []string{"asd"},
 | 
			
		||||
		}
 | 
			
		||||
		der, err := x509.CreateCertificate(rand.Reader, template, template, &k.PublicKey, k)
 | 
			
		||||
		der, err := x509.CreateCertificate(rand.Reader, template, issuer, &k.PublicKey, k)
 | 
			
		||||
		test.AssertNotError(t, err, "failed to generate test cert")
 | 
			
		||||
		_, err = ssa.AddPrecertificate(context.Background(), &sapb.AddCertificateRequest{
 | 
			
		||||
			Der:      der,
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
							
								
								
									
										17
									
								
								ra/ra.go
								
								
								
								
							
							
						
						
									
										17
									
								
								ra/ra.go
								
								
								
								
							| 
						 | 
				
			
			@ -1692,10 +1692,17 @@ func revokeEvent(state, serial, cn string, names []string, revocationCode revoca
 | 
			
		|||
// revokeCertificate generates a revoked OCSP response for the given certificate, stores
 | 
			
		||||
// the revocation information, and purges OCSP request URLs from Akamai.
 | 
			
		||||
func (ra *RegistrationAuthorityImpl) revokeCertificate(ctx context.Context, cert x509.Certificate, code revocation.Reason, revokedBy int64, source string, comment string) error {
 | 
			
		||||
	issuer, ok := ra.issuers[issuance.GetIssuerNameID(&cert)]
 | 
			
		||||
	if !ok {
 | 
			
		||||
		return fmt.Errorf("unable to identify issuer of certificate to revoke: %v", cert)
 | 
			
		||||
	}
 | 
			
		||||
	serial := core.SerialToString(cert.SerialNumber)
 | 
			
		||||
	reason := int32(code)
 | 
			
		||||
	revokedAt := ra.clk.Now().UnixNano()
 | 
			
		||||
 | 
			
		||||
	ocspResponse, err := ra.CA.GenerateOCSP(ctx, &capb.GenerateOCSPRequest{
 | 
			
		||||
		CertDER:   cert.Raw,
 | 
			
		||||
		Serial:    serial,
 | 
			
		||||
		IssuerID:  int64(issuer.ID()),
 | 
			
		||||
		Status:    string(core.OCSPStatusRevoked),
 | 
			
		||||
		Reason:    reason,
 | 
			
		||||
		RevokedAt: revokedAt,
 | 
			
		||||
| 
						 | 
				
			
			@ -1703,7 +1710,7 @@ func (ra *RegistrationAuthorityImpl) revokeCertificate(ctx context.Context, cert
 | 
			
		|||
	if err != nil {
 | 
			
		||||
		return err
 | 
			
		||||
	}
 | 
			
		||||
	serial := core.SerialToString(cert.SerialNumber)
 | 
			
		||||
 | 
			
		||||
	err = ra.SA.RevokeCertificate(ctx, &sapb.RevokeCertificateRequest{
 | 
			
		||||
		Serial:   serial,
 | 
			
		||||
		Reason:   int64(code),
 | 
			
		||||
| 
						 | 
				
			
			@ -1713,6 +1720,7 @@ func (ra *RegistrationAuthorityImpl) revokeCertificate(ctx context.Context, cert
 | 
			
		|||
	if err != nil {
 | 
			
		||||
		return err
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	if reason == ocsp.KeyCompromise {
 | 
			
		||||
		digest, err := core.KeyDigest(cert.PublicKey)
 | 
			
		||||
		if err != nil {
 | 
			
		||||
| 
						 | 
				
			
			@ -1733,10 +1741,7 @@ func (ra *RegistrationAuthorityImpl) revokeCertificate(ctx context.Context, cert
 | 
			
		|||
			return err
 | 
			
		||||
		}
 | 
			
		||||
	}
 | 
			
		||||
	issuer, ok := ra.issuers[issuance.GetIssuerNameID(&cert)]
 | 
			
		||||
	if !ok {
 | 
			
		||||
		return fmt.Errorf("unable to identify issuer of revoked certificate: %v", cert)
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	purgeURLs, err := akamai.GeneratePurgeURLs(&cert, issuer.Certificate)
 | 
			
		||||
	if err != nil {
 | 
			
		||||
		return err
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in New Issue