Switch GenerateOCSP to directly use protos instead of wrapper (#4549)

This commit is contained in:
Roland Bracewell Shoemaker 2019-11-14 11:10:33 -08:00 committed by GitHub
parent ef18f4c1a1
commit b8ee84da7b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 100 additions and 129 deletions

View File

@ -392,8 +392,8 @@ var ocspStatusToCode = map[string]int{
}
// GenerateOCSP produces a new OCSP response and returns it
func (ca *CertificateAuthorityImpl) GenerateOCSP(ctx context.Context, xferObj core.OCSPSigningRequest) ([]byte, error) {
cert, err := x509.ParseCertificate(xferObj.CertDER)
func (ca *CertificateAuthorityImpl) GenerateOCSP(ctx context.Context, req *caPB.GenerateOCSPRequest) (*caPB.OCSPResponse, error) {
cert, err := x509.ParseCertificate(req.CertDER)
if err != nil {
ca.log.AuditErr(err.Error())
return nil, err
@ -414,14 +414,14 @@ func (ca *CertificateAuthorityImpl) GenerateOCSP(ctx context.Context, xferObj co
now := ca.clk.Now().Truncate(time.Hour)
tbsResponse := ocsp.Response{
Status: ocspStatusToCode[xferObj.Status],
Status: ocspStatusToCode[*req.Status],
SerialNumber: cert.SerialNumber,
ThisUpdate: now,
NextUpdate: now.Add(ca.ocspLifetime),
}
if tbsResponse.Status == ocsp.Revoked {
tbsResponse.RevokedAt = xferObj.RevokedAt
tbsResponse.RevocationReason = int(xferObj.Reason)
tbsResponse.RevokedAt = time.Unix(0, *req.RevokedAt)
tbsResponse.RevocationReason = int(*req.Reason)
}
ocspResponse, err := ocsp.CreateResponse(issuer.cert, issuer.cert, tbsResponse, issuer.ocspSigner)
@ -429,7 +429,7 @@ func (ca *CertificateAuthorityImpl) GenerateOCSP(ctx context.Context, xferObj co
if err == nil {
ca.signatureCount.With(prometheus.Labels{"purpose": "ocsp"}).Inc()
}
return ocspResponse, err
return &caPB.OCSPResponse{Response: ocspResponse}, err
}
func (ca *CertificateAuthorityImpl) IssuePrecertificate(ctx context.Context, issueReq *caPB.IssueCertificateRequest) (*caPB.IssuePrecertificateResponse, error) {
@ -458,9 +458,10 @@ func (ca *CertificateAuthorityImpl) IssuePrecertificate(ctx context.Context, iss
return nil, err
}
ocspResp, err := ca.GenerateOCSP(ctx, core.OCSPSigningRequest{
status := string(core.OCSPStatusGood)
ocspResp, err := ca.GenerateOCSP(ctx, &caPB.GenerateOCSPRequest{
CertDER: precertDER,
Status: string(core.OCSPStatusGood),
Status: &status,
})
if err != nil {
err = berrors.InternalServerError(err.Error())
@ -471,7 +472,7 @@ func (ca *CertificateAuthorityImpl) IssuePrecertificate(ctx context.Context, iss
_, err = ca.sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
Der: precertDER,
RegID: &regID,
Ocsp: ocspResp,
Ocsp: ocspResp.Response,
Issued: &nowNanos,
})
if err != nil {
@ -482,7 +483,7 @@ func (ca *CertificateAuthorityImpl) IssuePrecertificate(ctx context.Context, iss
ca.queueOrphan(&orphanedCert{
DER: precertDER,
RegID: regID,
OCSPResp: ocspResp,
OCSPResp: ocspResp.Response,
Precert: true,
})
}

View File

@ -24,14 +24,14 @@ import (
"github.com/cloudflare/cfssl/helpers"
"github.com/cloudflare/cfssl/signer"
"github.com/cloudflare/cfssl/signer/local"
"github.com/google/certificate-transparency-go"
ct "github.com/google/certificate-transparency-go"
cttls "github.com/google/certificate-transparency-go/tls"
"github.com/jmhodges/clock"
"github.com/prometheus/client_golang/prometheus"
"github.com/zmap/zlint/lints"
"golang.org/x/crypto/ocsp"
"github.com/letsencrypt/boulder/ca/config"
ca_config "github.com/letsencrypt/boulder/ca/config"
caPB "github.com/letsencrypt/boulder/ca/proto"
"github.com/letsencrypt/boulder/cmd"
"github.com/letsencrypt/boulder/core"
@ -505,21 +505,22 @@ func TestOCSP(t *testing.T) {
test.AssertNotError(t, err, "Failed to issue")
parsedCert, err := x509.ParseCertificate(cert.DER)
test.AssertNotError(t, err, "Failed to parse cert")
ocspResp, err := ca.GenerateOCSP(ctx, core.OCSPSigningRequest{
status := string(core.OCSPStatusGood)
ocspResp, err := ca.GenerateOCSP(ctx, &caPB.GenerateOCSPRequest{
CertDER: cert.DER,
Status: string(core.OCSPStatusGood),
Status: &status,
})
test.AssertNotError(t, err, "Failed to generate OCSP")
parsed, err := ocsp.ParseResponse(ocspResp, caCert)
parsed, err := ocsp.ParseResponse(ocspResp.Response, caCert)
test.AssertNotError(t, err, "Failed to parse validate OCSP")
test.AssertEquals(t, parsed.Status, 0)
test.AssertEquals(t, parsed.RevocationReason, 0)
test.AssertEquals(t, parsed.SerialNumber.Cmp(parsedCert.SerialNumber), 0)
// Test that signatures are checked.
_, err = ca.GenerateOCSP(ctx, core.OCSPSigningRequest{
_, err = ca.GenerateOCSP(ctx, &caPB.GenerateOCSPRequest{
CertDER: append(cert.DER, byte(0)),
Status: string(core.OCSPStatusGood),
Status: &status,
})
test.AssertError(t, err, "Generated OCSP for cert with bad signature")
@ -560,22 +561,22 @@ func TestOCSP(t *testing.T) {
// ocspResp2 is a second OCSP response for `cert` (issued by caCert), and
// should be signed by caCert.
ocspResp2, err := ca.GenerateOCSP(ctx, core.OCSPSigningRequest{
ocspResp2, err := ca.GenerateOCSP(ctx, &caPB.GenerateOCSPRequest{
CertDER: append(cert.DER),
Status: string(core.OCSPStatusGood),
Status: &status,
})
test.AssertNotError(t, err, "Failed to sign second OCSP response")
_, err = ocsp.ParseResponse(ocspResp2, caCert)
_, err = ocsp.ParseResponse(ocspResp2.Response, caCert)
test.AssertNotError(t, err, "Failed to parse / validate second OCSP response")
// newCertOcspResp is an OCSP response for `newCert` (issued by newIssuer),
// and should be signed by newIssuer.
newCertOcspResp, err := ca.GenerateOCSP(ctx, core.OCSPSigningRequest{
newCertOcspResp, err := ca.GenerateOCSP(ctx, &caPB.GenerateOCSPRequest{
CertDER: newCert.DER,
Status: string(core.OCSPStatusGood),
Status: &status,
})
test.AssertNotError(t, err, "Failed to generate OCSP")
parsedNewCertOcspResp, err := ocsp.ParseResponse(newCertOcspResp, newIssuerCert)
parsedNewCertOcspResp, err := ocsp.ParseResponse(newCertOcspResp.Response, newIssuerCert)
test.AssertNotError(t, err, "Failed to parse / validate OCSP for newCert")
test.AssertEquals(t, parsedNewCertOcspResp.Status, 0)
test.AssertEquals(t, parsedNewCertOcspResp.RevocationReason, 0)

View File

@ -149,7 +149,7 @@ func main() {
caConn, err := bgrpc.ClientSetup(c.RA.CAService, tlsConfig, clientMetrics, clk)
cmd.FailOnError(err, "Unable to create CA client")
cac := bgrpc.NewCertificateAuthorityClient(caPB.NewCertificateAuthorityClient(caConn), nil)
cac := bgrpc.NewCertificateAuthorityClient(caPB.NewCertificateAuthorityClient(caConn))
var ctp *ctpolicy.CTPolicy
conn, err := bgrpc.ClientSetup(c.RA.PublisherService, tlsConfig, clientMetrics, clk)

View File

@ -42,7 +42,7 @@ type OCSPUpdater struct {
dbMap ocspDB
cac core.CertificateAuthority
ogc capb.OCSPGeneratorClient
sac core.StorageAuthority
// Used to calculate how far back stale OCSP responses should be looked for
@ -64,7 +64,7 @@ func newUpdater(
stats metrics.Scope,
clk clock.Clock,
dbMap ocspDB,
ca core.CertificateAuthority,
ogc capb.OCSPGeneratorClient,
sac core.StorageAuthority,
apc akamaipb.AkamaiPurgerClient,
config OCSPUpdaterConfig,
@ -90,7 +90,7 @@ func newUpdater(
stats: stats,
clk: clk,
dbMap: dbMap,
cac: ca,
ogc: ogc,
log: log,
sac: sac,
ocspMinTimeToExpiry: config.OCSPMinTimeToExpiry.Duration,
@ -177,20 +177,21 @@ func (updater *OCSPUpdater) generateResponse(ctx context.Context, status core.Ce
}
}
signRequest := core.OCSPSigningRequest{
reason := int32(status.RevokedReason)
statusStr := string(status.Status)
revokedAt := status.RevokedDate.UnixNano()
ocspResponse, err := updater.ogc.GenerateOCSP(ctx, &capb.GenerateOCSPRequest{
CertDER: cert.DER,
Reason: status.RevokedReason,
Status: string(status.Status),
RevokedAt: status.RevokedDate,
}
ocspResponse, err := updater.cac.GenerateOCSP(ctx, signRequest)
Reason: &reason,
Status: &statusStr,
RevokedAt: &revokedAt,
})
if err != nil {
return nil, err
}
status.OCSPLastUpdated = updater.clk.Now()
status.OCSPResponse = ocspResponse
status.OCSPResponse = ocspResponse.Response
return &status, nil
}
@ -388,7 +389,7 @@ type OCSPUpdaterConfig struct {
}
func setupClients(c OCSPUpdaterConfig, stats metrics.Scope, clk clock.Clock) (
core.CertificateAuthority,
capb.OCSPGeneratorClient,
core.StorageAuthority,
akamaipb.AkamaiPurgerClient,
) {
@ -404,7 +405,7 @@ func setupClients(c OCSPUpdaterConfig, stats metrics.Scope, clk clock.Clock) (
// Make a CA client that is only capable of signing OCSP.
// TODO(jsha): Once we've fully moved to gRPC, replace this
// with a plain caPB.NewOCSPGeneratorClient.
cac := bgrpc.NewCertificateAuthorityClient(nil, capb.NewOCSPGeneratorClient(caConn))
ogc := capb.NewOCSPGeneratorClient(caConn)
saConn, err := bgrpc.ClientSetup(c.SAService, tls, clientMetrics, clk)
cmd.FailOnError(err, "Failed to load credentials and create gRPC connection to SA")
@ -417,7 +418,7 @@ func setupClients(c OCSPUpdaterConfig, stats metrics.Scope, clk clock.Clock) (
apc = akamaipb.NewAkamaiPurgerClient(apcConn)
}
return cac, sac, apc
return ogc, sac, apc
}
func main() {
@ -450,13 +451,13 @@ func main() {
sa.InitDBMetrics(dbMap, scope)
clk := cmd.Clock()
cac, sac, apc := setupClients(conf, scope, clk)
ogc, sac, apc := setupClients(conf, scope, clk)
updater, err := newUpdater(
scope,
clk,
dbMap,
cac,
ogc,
sac,
apc,
// Necessary evil for now

View File

@ -23,31 +23,19 @@ import (
"github.com/letsencrypt/boulder/sa/satest"
"github.com/letsencrypt/boulder/test"
"github.com/letsencrypt/boulder/test/vars"
"google.golang.org/grpc"
"gopkg.in/go-gorp/gorp.v2"
)
var ctx = context.Background()
type mockCA struct {
type mockOCSP struct {
sleepTime time.Duration
}
func (ca *mockCA) IssueCertificate(_ context.Context, _ *caPB.IssueCertificateRequest) (core.Certificate, error) {
return core.Certificate{}, nil
}
func (ca *mockCA) IssuePrecertificate(_ context.Context, _ *caPB.IssueCertificateRequest) (*caPB.IssuePrecertificateResponse, error) {
return nil, errors.New("IssuePrecertificate is not implemented by mockCA")
}
func (ca *mockCA) IssueCertificateForPrecertificate(_ context.Context, _ *caPB.IssueCertificateForPrecertificateRequest) (core.Certificate, error) {
return core.Certificate{}, errors.New("IssueCertificateForPrecertificate is not implemented by mockCA")
}
func (ca *mockCA) GenerateOCSP(_ context.Context, xferObj core.OCSPSigningRequest) (ocsp []byte, err error) {
ocsp = []byte{1, 2, 3}
func (ca *mockOCSP) GenerateOCSP(_ context.Context, req *caPB.GenerateOCSPRequest, _ ...grpc.CallOption) (*caPB.OCSPResponse, error) {
time.Sleep(ca.sleepTime)
return
return &caPB.OCSPResponse{Response: []byte{1, 2, 3}}, nil
}
var log = blog.UseMock()
@ -80,7 +68,7 @@ func setup(t *testing.T) (*OCSPUpdater, core.StorageAuthority, *gorp.DbMap, cloc
metrics.NewNoopScope(),
fc,
dbMap,
&mockCA{},
&mockOCSP{},
sa,
nil,
OCSPUpdaterConfig{
@ -155,7 +143,7 @@ func TestGenerateOCSPResponses(t *testing.T) {
// Note that this test also tests the basic functionality of
// generateOCSPResponses.
start := time.Now()
updater.cac = &mockCA{time.Second}
updater.ogc = &mockOCSP{time.Second}
updater.parallelGenerateOCSPRequests = 10
err = updater.generateOCSPResponses(ctx, certs, metrics.NewNoopScope())
test.AssertNotError(t, err, "Couldn't generate OCSP responses")
@ -458,7 +446,7 @@ func TestGenerateOCSPResponsePrecert(t *testing.T) {
// Directly call generateResponse with the result, when the PrecertificateOCSP
// feature flag is disabled we expect this to error because no matching
// certificates row will be found.
updater.cac = &mockCA{time.Second}
updater.ogc = &mockOCSP{time.Second}
_, err = updater.generateResponse(ctx, certs[0])
test.AssertError(t, err, "generateResponse for precert without PrecertificateOCSP did not error")

View File

@ -99,7 +99,7 @@ type CertificateAuthority interface {
// [RegistrationAuthority]
IssueCertificateForPrecertificate(ctx context.Context, req *caPB.IssueCertificateForPrecertificateRequest) (Certificate, error)
GenerateOCSP(ctx context.Context, ocspReq OCSPSigningRequest) ([]byte, error)
GenerateOCSP(ctx context.Context, ocspReq *caPB.GenerateOCSPRequest) (*caPB.OCSPResponse, error)
}
// PolicyAuthority defines the public interface for the Boulder PA

View File

@ -8,36 +8,22 @@ package grpc
import (
"context"
"errors"
"time"
"google.golang.org/grpc"
caPB "github.com/letsencrypt/boulder/ca/proto"
capb "github.com/letsencrypt/boulder/ca/proto"
"github.com/letsencrypt/boulder/core"
corepb "github.com/letsencrypt/boulder/core/proto"
"github.com/letsencrypt/boulder/revocation"
"google.golang.org/grpc"
)
// CertificateAuthorityClientWrapper is the gRPC version of a
// core.CertificateAuthority client. It composites a CertificateAuthorityClient
// and OCSPGeneratorClient, either of which may be nil if the calling code
// doesn't intend to use the relevant functions. Once we've fully moved to gRPC,
// calling code will do away with this wrapper and directly instantiate exactly
// the type of client it needs.
type CertificateAuthorityClientWrapper struct {
inner caPB.CertificateAuthorityClient
innerOCSP caPB.OCSPGeneratorClient
inner capb.CertificateAuthorityClient
}
func NewCertificateAuthorityClient(inner caPB.CertificateAuthorityClient, innerOCSP caPB.OCSPGeneratorClient) *CertificateAuthorityClientWrapper {
return &CertificateAuthorityClientWrapper{inner, innerOCSP}
func NewCertificateAuthorityClient(inner capb.CertificateAuthorityClient) *CertificateAuthorityClientWrapper {
return &CertificateAuthorityClientWrapper{inner}
}
func (cac CertificateAuthorityClientWrapper) IssuePrecertificate(ctx context.Context, issueReq *caPB.IssueCertificateRequest) (*caPB.IssuePrecertificateResponse, error) {
if cac.inner == nil {
return nil, errors.New("this CA client does not support issuing precertificates")
}
func (cac CertificateAuthorityClientWrapper) IssuePrecertificate(ctx context.Context, issueReq *capb.IssueCertificateRequest) (*capb.IssuePrecertificateResponse, error) {
resp, err := cac.inner.IssuePrecertificate(ctx, issueReq)
if err != nil {
return nil, err
@ -48,10 +34,7 @@ func (cac CertificateAuthorityClientWrapper) IssuePrecertificate(ctx context.Con
return resp, nil
}
func (cac CertificateAuthorityClientWrapper) IssueCertificateForPrecertificate(ctx context.Context, req *caPB.IssueCertificateForPrecertificateRequest) (core.Certificate, error) {
if cac.inner == nil {
return core.Certificate{}, errors.New("this CA client does not support issuing precertificates")
}
func (cac CertificateAuthorityClientWrapper) IssueCertificateForPrecertificate(ctx context.Context, req *capb.IssueCertificateForPrecertificateRequest) (core.Certificate, error) {
res, err := cac.inner.IssueCertificateForPrecertificate(ctx, req)
if err != nil {
return core.Certificate{}, err
@ -59,30 +42,34 @@ func (cac CertificateAuthorityClientWrapper) IssueCertificateForPrecertificate(c
return PBToCert(res)
}
func (cac CertificateAuthorityClientWrapper) GenerateOCSP(ctx context.Context, ocspReq core.OCSPSigningRequest) ([]byte, error) {
var inner interface {
GenerateOCSP(context.Context, *caPB.GenerateOCSPRequest, ...grpc.CallOption) (*caPB.OCSPResponse, error)
}
if cac.innerOCSP == nil {
inner = cac.inner
} else {
inner = cac.innerOCSP
}
reason := int32(ocspReq.Reason)
revokedAt := ocspReq.RevokedAt.UnixNano()
res, err := inner.GenerateOCSP(ctx, &caPB.GenerateOCSPRequest{
CertDER: ocspReq.CertDER,
Status: &ocspReq.Status,
Reason: &reason,
RevokedAt: &revokedAt,
})
func (cac CertificateAuthorityClientWrapper) GenerateOCSP(ctx context.Context, req *capb.GenerateOCSPRequest) (*capb.OCSPResponse, error) {
res, err := cac.inner.GenerateOCSP(ctx, req)
if err != nil {
return nil, err
}
if res == nil || res.Response == nil {
return nil, errIncompleteResponse
}
return res.Response, nil
return res, nil
}
type OCSPGeneratorClientWrapper struct {
inner capb.OCSPGeneratorClient
}
func NewOCSPGeneratorClient(inner capb.OCSPGeneratorClient) *OCSPGeneratorClientWrapper {
return &OCSPGeneratorClientWrapper{inner}
}
func (ogc OCSPGeneratorClientWrapper) GenerateOCSP(ctx context.Context, req *capb.GenerateOCSPRequest, _ ...grpc.CallOption) (*capb.OCSPResponse, error) {
res, err := ogc.inner.GenerateOCSP(ctx, req)
if err != nil {
return nil, err
}
if res == nil || res.Response == nil {
return nil, errIncompleteResponse
}
return res, nil
}
// CertificateAuthorityServerWrapper is the gRPC version of a core.CertificateAuthority server
@ -94,14 +81,14 @@ func NewCertificateAuthorityServer(inner core.CertificateAuthority) *Certificate
return &CertificateAuthorityServerWrapper{inner}
}
func (cas *CertificateAuthorityServerWrapper) IssuePrecertificate(ctx context.Context, request *caPB.IssueCertificateRequest) (*caPB.IssuePrecertificateResponse, error) {
func (cas *CertificateAuthorityServerWrapper) IssuePrecertificate(ctx context.Context, request *capb.IssueCertificateRequest) (*capb.IssuePrecertificateResponse, error) {
if request == nil || request.Csr == nil || request.OrderID == nil || request.RegistrationID == nil {
return nil, errIncompleteRequest
}
return cas.inner.IssuePrecertificate(ctx, request)
}
func (cas *CertificateAuthorityServerWrapper) IssueCertificateForPrecertificate(ctx context.Context, req *caPB.IssueCertificateForPrecertificateRequest) (*corepb.Certificate, error) {
func (cas *CertificateAuthorityServerWrapper) IssueCertificateForPrecertificate(ctx context.Context, req *capb.IssueCertificateForPrecertificateRequest) (*corepb.Certificate, error) {
if req == nil || req.DER == nil || req.OrderID == nil || req.RegistrationID == nil || req.SCTs == nil {
return nil, errIncompleteRequest
}
@ -112,18 +99,9 @@ func (cas *CertificateAuthorityServerWrapper) IssueCertificateForPrecertificate(
return CertToPB(cert), nil
}
func (cas *CertificateAuthorityServerWrapper) GenerateOCSP(ctx context.Context, req *caPB.GenerateOCSPRequest) (*caPB.OCSPResponse, error) {
func (cas *CertificateAuthorityServerWrapper) GenerateOCSP(ctx context.Context, req *capb.GenerateOCSPRequest) (*capb.OCSPResponse, error) {
if req.CertDER == nil || req.Status == nil || req.Reason == nil || req.RevokedAt == nil {
return nil, errIncompleteRequest
}
res, err := cas.inner.GenerateOCSP(ctx, core.OCSPSigningRequest{
CertDER: req.CertDER,
Status: *req.Status,
Reason: revocation.Reason(*req.Reason),
RevokedAt: time.Unix(0, *req.RevokedAt),
})
if err != nil {
return nil, err
}
return &caPB.OCSPResponse{Response: res}, nil
return cas.inner.GenerateOCSP(ctx, req)
}

View File

@ -53,8 +53,8 @@ func (ca *MockCA) IssueCertificateForPrecertificate(ctx context.Context, req *ca
}
// GenerateOCSP is a mock
func (ca *MockCA) GenerateOCSP(ctx context.Context, xferObj core.OCSPSigningRequest) (ocsp []byte, err error) {
return
func (ca *MockCA) GenerateOCSP(ctx context.Context, req *caPB.GenerateOCSPRequest) (*caPB.OCSPResponse, error) {
return nil, nil
}
// RevokeCertificate is a mock

View File

@ -1649,25 +1649,27 @@ 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) error {
now := time.Now()
signRequest := core.OCSPSigningRequest{
status := string(core.OCSPStatusRevoked)
reason := int32(code)
revokedAt := time.Now().UnixNano()
ocspResponse, err := ra.CA.GenerateOCSP(ctx, &caPB.GenerateOCSPRequest{
CertDER: cert.Raw,
Status: string(core.OCSPStatusRevoked),
Reason: code,
RevokedAt: now,
}
ocspResponse, err := ra.CA.GenerateOCSP(ctx, signRequest)
Status: &status,
Reason: &reason,
RevokedAt: &revokedAt,
})
if err != nil {
return err
}
serial := core.SerialToString(cert.SerialNumber)
nowUnix := now.UnixNano()
reason := int64(code)
// for some reason we use int32 and int64 for the reason in different
// protobuf messages, so we have to re-cast it here.
reason64 := int64(reason)
err = ra.SA.RevokeCertificate(ctx, &sapb.RevokeCertificateRequest{
Serial: &serial,
Reason: &reason,
Date: &nowUnix,
Response: ocspResponse,
Reason: &reason64,
Date: &revokedAt,
Response: ocspResponse.Response,
})
if err != nil {
return err