From a2b8faea1e2aa1a83223a444a455185c887b38fc Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Mon, 5 Dec 2016 16:54:02 -0500 Subject: [PATCH] Only resubmit missing SCTs. (#2342) This PR introduces the ability for the ocsp-updater to only resubmit certificates to logs that we are missing SCTs from. Prior to this commit when a certificate was missing one or more SCTs we would submit it to every log, causing unnecessary overhead for us and the log operator. To accomplish this a new RPC endpoint is added to the Publisher service "SubmitToSingleCT". Unlike the existing "SubmitToCT" this RPC endpoint accepts a log URI and public key in addition to the certificate DER bytes. The certificate is submitted directly to that log, and a cache of constructed resources is maintained so that subsequent submissions to the same log can reuse the stat name, verifier, and submission client. Resolves #1679 --- cmd/ocsp-updater/main.go | 106 ++++++- cmd/ocsp-updater/main_test.go | 315 +++++++++++++++++++-- core/interfaces.go | 1 + features/featureflag_string.go | 4 +- features/features.go | 2 + grpc/wrappers.go | 23 +- mocks/mocks.go | 5 + publisher/mock_publisher/gen.go | 3 + publisher/mock_publisher/mock_publisher.go | 50 ++++ publisher/proto/publisher.pb.go | 68 ++++- publisher/proto/publisher.proto | 3 + publisher/publisher.go | 121 ++++++-- publisher/publisher_test.go | 49 ++++ rpc/rpc-wrappers.go | 8 + test.sh | 6 + test/config-next/ocsp-updater.json | 3 + 16 files changed, 706 insertions(+), 61 deletions(-) create mode 100644 publisher/mock_publisher/gen.go create mode 100644 publisher/mock_publisher/mock_publisher.go diff --git a/cmd/ocsp-updater/main.go b/cmd/ocsp-updater/main.go index b0ce7bbb387..1118811e305 100644 --- a/cmd/ocsp-updater/main.go +++ b/cmd/ocsp-updater/main.go @@ -1,6 +1,7 @@ package main import ( + "crypto/sha256" "crypto/x509" "database/sql" "encoding/base64" @@ -56,8 +57,8 @@ type OCSPUpdater struct { ocspMinTimeToExpiry time.Duration // Used to calculate how far back missing SCT receipts should be looked for oldestIssuedSCT time.Duration - // Number of CT logs we expect to have receipts from - numLogs int + // Logs we expect to have SCT receipts for. Missing logs will be resubmitted to. + logs []*ctLog loops []*looper @@ -75,7 +76,7 @@ func newUpdater( pub core.Publisher, sac core.StorageAuthority, config cmd.OCSPUpdaterConfig, - numLogs int, + logConfigs []cmd.LogDescription, issuerPath string, log blog.Logger, ) (*OCSPUpdater, error) { @@ -90,6 +91,15 @@ func newUpdater( return nil, fmt.Errorf("Loop window sizes must be non-zero") } + logs := make([]*ctLog, len(logConfigs)) + for i, logConfig := range logConfigs { + l, err := newLog(logConfig) + if err != nil { + return nil, err + } + logs[i] = l + } + updater := OCSPUpdater{ stats: stats, clk: clk, @@ -98,7 +108,7 @@ func newUpdater( log: log, sac: sac, pubc: pub, - numLogs: numLogs, + logs: logs, ocspMinTimeToExpiry: config.OCSPMinTimeToExpiry.Duration, oldestIssuedSCT: config.OldestIssuedSCT.Duration, } @@ -474,14 +484,39 @@ func (updater *OCSPUpdater) getSerialsIssuedSince(since time.Time, batchSize int return allSerials, nil } -func (updater *OCSPUpdater) getNumberOfReceipts(serial string) (int, error) { - var count int - err := updater.dbMap.SelectOne( - &count, - "SELECT COUNT(id) FROM sctReceipts WHERE certificateSerial = :serial", +// getSubmittedReceipts returns the IDs of the CT logs that have returned a SCT +// receipt for the given certificate serial +func (updater *OCSPUpdater) getSubmittedReceipts(serial string) ([]string, error) { + var logIDs []string + _, err := updater.dbMap.Select( + &logIDs, + `SELECT logID + FROM sctReceipts + WHERE certificateSerial = :serial`, map[string]interface{}{"serial": serial}, ) - return count, err + return logIDs, err +} + +// missingLogIDs examines a list of log IDs that have given a SCT receipt for +// a certificate and returns a list of the configured logs that are not +// present. This is the set of logs we need to resubmit this certificate to in +// order to obtain a full compliment of SCTs +func (updater *OCSPUpdater) missingLogs(logIDs []string) []*ctLog { + var missingLogs []*ctLog + + presentMap := make(map[string]bool) + for _, logID := range logIDs { + presentMap[logID] = true + } + + for _, l := range updater.logs { + if _, present := presentMap[l.logID]; !present { + missingLogs = append(missingLogs, l) + } + } + + return missingLogs } // missingReceiptsTick looks for certificates without the correct number of SCT @@ -496,20 +531,42 @@ func (updater *OCSPUpdater) missingReceiptsTick(ctx context.Context, batchSize i } for _, serial := range serials { - count, err := updater.getNumberOfReceipts(serial) + // First find the logIDs that have provided a SCT for the serial + logIDs, err := updater.getSubmittedReceipts(serial) if err != nil { - updater.log.AuditErr(fmt.Sprintf("Failed to get number of SCT receipts for certificate: %s", err)) + updater.log.AuditErr(fmt.Sprintf( + "Failed to get CT log IDs of SCT receipts for certificate: %s", err)) continue } - if count >= updater.numLogs { + + // Next, check if any of the configured CT logs are missing from the list of + // logs that have given SCTs for this serial + missingLogs := updater.missingLogs(logIDs) + if len(missingLogs) == 0 { + // If all of the logs have provided a SCT we're done for this serial continue } + + // Otherwise, we need to get the certificate from the SA & submit it to each + // of the missing logs to obtain SCTs. cert, err := updater.sac.GetCertificate(ctx, serial) if err != nil { updater.log.AuditErr(fmt.Sprintf("Failed to get certificate: %s", err)) continue } - _ = updater.pubc.SubmitToCT(ctx, cert.DER) + + // If the feature flag is enabled, only send the certificate to the missing + // logs using the `SubmitToSingleCT` endpoint that was added for this + // purpose + if features.Enabled(features.ResubmitMissingSCTsOnly) { + for _, log := range missingLogs { + _ = updater.pubc.SubmitToSingleCT(ctx, log.uri, log.key, cert.DER) + } + } else { + // Otherwise, use the classic behaviour and submit the certificate to + // every log to get SCTS using the pre-existing `SubmitToCT` endpoint + _ = updater.pubc.SubmitToCT(ctx, cert.DER) + } } return nil } @@ -565,6 +622,25 @@ func (l *looper) loop() error { } } +// a ctLog contains the pre-processed logID and URI for a CT log. The ocsp-updater +// creates these out of cmd.LogDescription's from its config +type ctLog struct { + logID string + key string + uri string +} + +func newLog(logConfig cmd.LogDescription) (*ctLog, error) { + logPK, err := base64.StdEncoding.DecodeString(logConfig.Key) + if err != nil { + return nil, err + } + + logPKHash := sha256.Sum256(logPK) + logID := base64.StdEncoding.EncodeToString(logPKHash[:]) + return &ctLog{logID: logID, key: logConfig.Key, uri: logConfig.URI}, nil +} + const clientName = "OCSP" type config struct { @@ -654,7 +730,7 @@ func main() { sac, // Necessary evil for now conf, - len(c.Common.CT.Logs), + c.Common.CT.Logs, c.Common.IssuerCert, auditlogger, ) diff --git a/cmd/ocsp-updater/main_test.go b/cmd/ocsp-updater/main_test.go index f3950e63c2c..7d3cd9ea320 100644 --- a/cmd/ocsp-updater/main_test.go +++ b/cmd/ocsp-updater/main_test.go @@ -1,21 +1,26 @@ package main import ( + "crypto/sha256" "crypto/x509" "database/sql" + "encoding/base64" "errors" "testing" "time" "golang.org/x/net/context" + "github.com/golang/mock/gomock" "github.com/jmhodges/clock" "gopkg.in/gorp.v1" "github.com/letsencrypt/boulder/cmd" "github.com/letsencrypt/boulder/core" + "github.com/letsencrypt/boulder/features" blog "github.com/letsencrypt/boulder/log" "github.com/letsencrypt/boulder/metrics" + "github.com/letsencrypt/boulder/publisher/mock_publisher" "github.com/letsencrypt/boulder/revocation" "github.com/letsencrypt/boulder/sa" "github.com/letsencrypt/boulder/sa/satest" @@ -37,28 +42,75 @@ func (ca *mockCA) GenerateOCSP(_ context.Context, xferObj core.OCSPSigningReques } type mockPub struct { - sa core.StorageAuthority + sa core.StorageAuthority + logs []cmd.LogDescription +} + +func logPublicKeyToID(logPK string) (string, error) { + logPKBytes, err := base64.StdEncoding.DecodeString(logPK) + if err != nil { + return "", err + } + + logPKHash := sha256.Sum256(logPKBytes) + logID := base64.StdEncoding.EncodeToString(logPKHash[:]) + return logID, nil } func (p *mockPub) SubmitToCT(_ context.Context, _ []byte) error { + // Add an SCT for every configured log + for _, log := range p.logs { + logID, err := logPublicKeyToID(log.Key) + if err != nil { + return err + } + sct := core.SignedCertificateTimestamp{ + SCTVersion: 0, + LogID: logID, + Timestamp: 0, + Extensions: []byte{}, + Signature: []byte{0}, + CertificateSerial: "00", + } + err = p.sa.AddSCTReceipt(ctx, sct) + if err != nil { + return err + } + } + return nil +} + +func (p *mockPub) SubmitToSingleCT(_ context.Context, _, logPublicKey string, _ []byte) error { + logID, err := logPublicKeyToID(logPublicKey) + if err != nil { + return err + } + // Add an SCT for the provided log ID sct := core.SignedCertificateTimestamp{ SCTVersion: 0, - LogID: "id", + LogID: logID, Timestamp: 0, Extensions: []byte{}, Signature: []byte{0}, CertificateSerial: "00", } - err := p.sa.AddSCTReceipt(ctx, sct) - if err != nil { - return err - } - sct.LogID = "another-id" - return p.sa.AddSCTReceipt(ctx, sct) + err = p.sa.AddSCTReceipt(ctx, sct) + return err } var log = blog.UseMock() +const ( + // Each log's test PK is the base64 of "test pk 1" .. "test pk 2" + testLogAPK = "dGVzdCBwayAx" + testLogBPK = "dGVzdCBwayAy" + testLogCPK = "dGVzdCBwayAz" + // Each log's ID is the base64 of the SHA256 sum of the PK above + testLogAID = "27sby+EK3U1YKhUUGi9vBfFskgHvKpRMJ7PtNJzGUF8=" + testLogBID = "EpN+1e1h2jWN6W4IRG4KwjwiY9QIWaep5Qf3s8NLRmc=" + testLogCID = "OOn8yL8QPsMuqENGprtlkOYkJqwhhcAifEHUPevmnCc=" +) + func setup(t *testing.T) (*OCSPUpdater, core.StorageAuthority, *gorp.DbMap, clock.FakeClock, func()) { dbMap, err := sa.NewDbMap(vars.DBConnSA, 0) test.AssertNotError(t, err, "Failed to create dbMap") @@ -72,12 +124,27 @@ func setup(t *testing.T) (*OCSPUpdater, core.StorageAuthority, *gorp.DbMap, cloc cleanUp := test.ResetSATestDatabase(t) + logs := []cmd.LogDescription{ + cmd.LogDescription{ + URI: "test", + Key: testLogAPK, + }, + cmd.LogDescription{ + URI: "test2", + Key: testLogBPK, + }, + cmd.LogDescription{ + URI: "test3", + Key: testLogCPK, + }, + } + updater, err := newUpdater( metrics.NewNoopScope(), fc, dbMap, &mockCA{}, - &mockPub{sa}, + &mockPub{sa, logs}, sa, cmd.OCSPUpdaterConfig{ NewCertificateBatchSize: 1, @@ -87,7 +154,7 @@ func setup(t *testing.T) (*OCSPUpdater, core.StorageAuthority, *gorp.DbMap, cloc OldOCSPWindow: cmd.ConfigDuration{Duration: time.Second}, MissingSCTWindow: cmd.ConfigDuration{Duration: time.Second}, }, - 0, + logs, "", blog.NewMock(), ) @@ -265,27 +332,89 @@ func TestMissingReceiptsTick(t *testing.T) { _, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID) test.AssertNotError(t, err, "Couldn't add test-cert.pem") - updater.numLogs = 1 updater.oldestIssuedSCT = 2 * time.Hour serials, err := updater.getSerialsIssuedSince(fc.Now().Add(-2*time.Hour), 1) test.AssertNotError(t, err, "Failed to retrieve serials") test.AssertEquals(t, len(serials), 1) + // Run the missing receipts tick err = updater.missingReceiptsTick(ctx, 5) test.AssertNotError(t, err, "Failed to run missingReceiptsTick") - count, err := updater.getNumberOfReceipts("00") - test.AssertNotError(t, err, "Couldn't get number of receipts") - test.AssertEquals(t, count, 2) + // We have three logs configured from setup, and with the + // ResubmitMissingSCTsOnly feature flag disabled we expect that we submitted + // to all three logs. + logIDs, err := updater.getSubmittedReceipts("00") + test.AssertNotError(t, err, "Couldn't get submitted receipts for serial 00") + test.AssertEquals(t, len(logIDs), 3) + test.AssertEquals(t, logIDs[0], testLogAID) + test.AssertEquals(t, logIDs[1], testLogBID) + test.AssertEquals(t, logIDs[2], testLogCID) // make sure we don't spin forever after reducing the // number of logs we submit to - updater.numLogs = 1 + logA, err := newLog( + cmd.LogDescription{ + URI: "test", + Key: testLogAPK, + }) + test.AssertNotError(t, err, "Failed to newLog test log A") + updater.logs = []*ctLog{logA} err = updater.missingReceiptsTick(ctx, 10) test.AssertNotError(t, err, "Failed to run missingReceiptsTick") } +func TestMissingOnlyReceiptsTick(t *testing.T) { + updater, sa, _, fc, cleanUp := setup(t) + defer cleanUp() + + reg := satest.CreateWorkingRegistration(t, sa) + parsedCert, err := core.LoadCert("test-cert.pem") + test.AssertNotError(t, err, "Couldn't read test certificate") + fc.Set(parsedCert.NotBefore.Add(time.Minute)) + _, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID) + test.AssertNotError(t, err, "Couldn't add test-cert.pem") + + updater.oldestIssuedSCT = 2 * time.Hour + + serials, err := updater.getSerialsIssuedSince(fc.Now().Add(-2*time.Hour), 1) + test.AssertNotError(t, err, "Failed to retrieve serials") + test.AssertEquals(t, len(serials), 1) + + // Enable the ResubmitMissingSCTsOnly feature flag for this test run + _ = features.Set(map[string]bool{"ResubmitMissingSCTsOnly": true}) + defer features.Reset() + + // Use a mock publisher so we can EXPECT specific calls + ctrl := gomock.NewController(t) + defer ctrl.Finish() + mockPub := mock_publisher.NewMockPublisher(ctrl) + updater.pubc = mockPub + + // Add an SCT for one of the three logs (test2) + sct := core.SignedCertificateTimestamp{ + SCTVersion: 0, + LogID: testLogBID, + Timestamp: 0, + Extensions: []byte{}, + Signature: []byte{0}, + CertificateSerial: core.SerialToString(parsedCert.SerialNumber), + } + err = sa.AddSCTReceipt(ctx, sct) + test.AssertNotError(t, err, "Failed to AddSCTReceipt") + + // We expect that there are only going to be TWO calls to SubmitSingleCT, one + // for each of the missing logs. We do NOT expect a call for "test2" since we + // already added a SCT for that log! + mockPub.EXPECT().SubmitToSingleCT(ctx, "test", testLogAPK, parsedCert.Raw) + mockPub.EXPECT().SubmitToSingleCT(ctx, "test3", testLogCPK, parsedCert.Raw) + + // Run the missing receipts tick, with the correct EXPECT's there should be no errors + err = updater.missingReceiptsTick(ctx, 5) + test.AssertNotError(t, err, "Failed to run missingReceiptsTick") +} + /* * https://github.com/letsencrypt/boulder/issues/1872 identified that the * `getSerialsIssuedSince` function may never terminate if there are always new @@ -318,7 +447,6 @@ func TestMissingReceiptsTickTerminate(t *testing.T) { // conditions that cause the termination bug described in // https://github.com/letsencrypt/boulder/issues/1872 are met updater.dbMap = inexhaustibleDB{} - updater.numLogs = 1 updater.oldestIssuedSCT = 2 * time.Hour // Note: Must use a batch size larger than the # of rows returned by @@ -438,3 +566,158 @@ func TestLoopTickBackoff(t *testing.T) { test.AssertEquals(t, l.failures, 0) test.AssertEquals(t, l.clk.Now(), start) } + +func TestGetSubmittedReceipts(t *testing.T) { + updater, sa, _, fc, cleanUp := setup(t) + defer cleanUp() + + reg := satest.CreateWorkingRegistration(t, sa) + parsedCert, err := core.LoadCert("test-cert.pem") + test.AssertNotError(t, err, "Couldn't read test certificate") + fc.Set(parsedCert.NotBefore.Add(time.Minute)) + _, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID) + test.AssertNotError(t, err, "Couldn't add test-cert.pem") + + // Before adding any SCTs, there should be no receipts or errors for serial 00 + receipts, err := updater.getSubmittedReceipts("00") + test.AssertNotError(t, err, "getSubmittedReceipts('00') failed") + test.AssertEquals(t, len(receipts), 0) + + // Add one SCT + sct := core.SignedCertificateTimestamp{ + SCTVersion: 0, + LogID: testLogAID, + Timestamp: 0, + Extensions: []byte{}, + Signature: []byte{0}, + CertificateSerial: "00", + } + err = sa.AddSCTReceipt(ctx, sct) + test.AssertNotError(t, err, "Failed to AddSCTReceipt") + + // After adding one SCTs, there should be one receipt for log "test" + receipts, err = updater.getSubmittedReceipts("00") + test.AssertNotError(t, err, "getSubmittedReceipts('00') failed") + test.AssertEquals(t, len(receipts), 1) + test.AssertEquals(t, receipts[0], testLogAID) + + // Add another SCT + sct = core.SignedCertificateTimestamp{ + SCTVersion: 0, + LogID: testLogBID, + Timestamp: 0, + Extensions: []byte{}, + Signature: []byte{0}, + CertificateSerial: "00", + } + err = sa.AddSCTReceipt(ctx, sct) + test.AssertNotError(t, err, "Failed to AddSCTReceipt") + + // After adding a second SCTs, there should be two receipts for logs "test" + // and "test2" + receipts, err = updater.getSubmittedReceipts("00") + test.AssertNotError(t, err, "getSubmittedReceipts('00') failed") + test.AssertEquals(t, len(receipts), 2) + test.AssertEquals(t, receipts[0], testLogAID) + test.AssertEquals(t, receipts[1], testLogBID) +} + +func TestMissingLogs(t *testing.T) { + updater, _, _, _, cleanUp := setup(t) + defer cleanUp() + + noLogs := []*ctLog{} + oneLog := []*ctLog{ + &ctLog{ + uri: "test", + key: testLogAPK, + logID: testLogAID, + }, + } + twoLogs := []*ctLog{ + oneLog[0], + &ctLog{ + uri: "test2", + key: testLogBPK, + logID: testLogBID, + }, + } + + testCases := []struct { + Logs []*ctLog + GivenIDs []string + ExpectedMissingLogs []*ctLog + }{ + // With `nil` logs, no log IDs are ever missing + { + Logs: nil, + GivenIDs: []string{testLogAID, testLogBID}, + ExpectedMissingLogs: []*ctLog{}, + }, + // No configured logs, no log IDs are ever missing + { + Logs: noLogs, + GivenIDs: []string{testLogAID, testLogBID}, + ExpectedMissingLogs: []*ctLog{}, + }, + // One configured log, given no log IDs, one is missing + { + Logs: oneLog, + GivenIDs: []string{}, + ExpectedMissingLogs: []*ctLog{oneLog[0]}, + }, + // One configured log, given `nil` log IDs, one is missing + { + Logs: oneLog, + GivenIDs: nil, + ExpectedMissingLogs: []*ctLog{oneLog[0]}, + }, + // One configured log, given that log ID, none are missing + { + Logs: oneLog, + GivenIDs: []string{testLogAID}, + ExpectedMissingLogs: []*ctLog{}, + }, + // Two configured logs, given one log ID, one is missing + { + Logs: twoLogs, + GivenIDs: []string{testLogAID}, + ExpectedMissingLogs: []*ctLog{twoLogs[1]}, + }, + // Two configured logs, given no log IDs, two are missing + { + Logs: twoLogs, + GivenIDs: []string{}, + ExpectedMissingLogs: []*ctLog{twoLogs[0], twoLogs[1]}, + }, + // Two configured logs, given two matching log IDs, none are missing + { + Logs: twoLogs, + GivenIDs: []string{testLogAID, testLogBID}, + ExpectedMissingLogs: []*ctLog{}, + }, + // Two configured logs, given unknown log, two are missing + { + Logs: twoLogs, + GivenIDs: []string{"wha?"}, + ExpectedMissingLogs: []*ctLog{twoLogs[0], twoLogs[1]}, + }, + // Two configured logs, given one unknown log, one known, one is missing + { + Logs: twoLogs, + GivenIDs: []string{"wha?", testLogBID}, + ExpectedMissingLogs: []*ctLog{twoLogs[0]}, + }, + } + + for _, tc := range testCases { + updater.logs = tc.Logs + missingLogs := updater.missingLogs(tc.GivenIDs) + test.AssertEquals(t, len(missingLogs), len(tc.ExpectedMissingLogs)) + for i, expectedLog := range tc.ExpectedMissingLogs { + test.AssertEquals(t, missingLogs[i].uri, expectedLog.uri) + test.AssertEquals(t, missingLogs[i].key, expectedLog.key) + test.AssertEquals(t, missingLogs[i].logID, expectedLog.logID) + } + } +} diff --git a/core/interfaces.go b/core/interfaces.go index c83bf83ff46..7f6f14c2154 100644 --- a/core/interfaces.go +++ b/core/interfaces.go @@ -135,4 +135,5 @@ type StorageAuthority interface { // Publisher defines the public interface for the Boulder Publisher type Publisher interface { SubmitToCT(ctx context.Context, der []byte) error + SubmitToSingleCT(ctx context.Context, logURL, logPublicKey string, der []byte) error } diff --git a/features/featureflag_string.go b/features/featureflag_string.go index 778e969186f..398677ebc32 100644 --- a/features/featureflag_string.go +++ b/features/featureflag_string.go @@ -4,9 +4,9 @@ package features import "fmt" -const _FeatureFlag_name = "unusedIDNASupportAllowAccountDeactivationCertStatusOptimizationsMigratedAllowKeyRollover" +const _FeatureFlag_name = "unusedIDNASupportAllowAccountDeactivationCertStatusOptimizationsMigratedAllowKeyRolloverResubmitMissingSCTsOnly" -var _FeatureFlag_index = [...]uint8{0, 6, 17, 41, 72, 88} +var _FeatureFlag_index = [...]uint8{0, 6, 17, 41, 72, 88, 111} func (i FeatureFlag) String() string { if i < 0 || i >= FeatureFlag(len(_FeatureFlag_index)-1) { diff --git a/features/features.go b/features/features.go index 6b27411b1fe..0c0227a207e 100644 --- a/features/features.go +++ b/features/features.go @@ -16,6 +16,7 @@ const ( AllowAccountDeactivation CertStatusOptimizationsMigrated AllowKeyRollover + ResubmitMissingSCTsOnly ) // List of features and their default value, protected by fMu @@ -25,6 +26,7 @@ var features = map[FeatureFlag]bool{ AllowAccountDeactivation: false, CertStatusOptimizationsMigrated: false, AllowKeyRollover: false, + ResubmitMissingSCTsOnly: false, } var fMu = new(sync.RWMutex) diff --git a/grpc/wrappers.go b/grpc/wrappers.go index 1852f7ade5a..7b57b79edd3 100644 --- a/grpc/wrappers.go +++ b/grpc/wrappers.go @@ -110,12 +110,25 @@ func NewPublisherClientWrapper(inner pubPB.PublisherClient) *PublisherClientWrap return &PublisherClientWrapper{inner} } -// SubmitToCT makes a call to the gRPC version of the publisher +// SubmitToCT makes a call to the gRPC version of the publisher to send the +// provided certificate to all of the configured CT logs func (pc *PublisherClientWrapper) SubmitToCT(ctx context.Context, der []byte) error { _, err := pc.inner.SubmitToCT(ctx, &pubPB.Request{Der: der}) return err } +// SubmitToSingleCT makes a call to the gRPC version of the publisher to send +// the provided certificate to the log specified by log URI and public key +func (pc *PublisherClientWrapper) SubmitToSingleCT(ctx context.Context, logURL, logPublicKey string, der []byte) error { + _, err := pc.inner.SubmitToSingleCT( + ctx, + &pubPB.Request{ + LogURL: &logURL, + LogPublicKey: &logPublicKey, + Der: der}) + return unwrapError(err) +} + // PublisherServerWrapper is a wrapper required to bridge the differences between the // gRPC and previous AMQP interfaces type PublisherServerWrapper struct { @@ -136,6 +149,14 @@ func (pub *PublisherServerWrapper) SubmitToCT(ctx context.Context, request *pubP return &pubPB.Empty{}, pub.inner.SubmitToCT(ctx, request.Der) } +func (pub *PublisherServerWrapper) SubmitToSingleCT(ctx context.Context, request *pubPB.Request) (*pubPB.Empty, error) { + if request == nil || request.Der == nil || request.LogURL == nil || request.LogPublicKey == nil { + return nil, errors.New("incomplete SubmitToSingleCT gRPC message") + } + err := wrapError(pub.inner.SubmitToSingleCT(ctx, *request.LogURL, *request.LogPublicKey, request.Der)) + return &pubPB.Empty{}, err +} + // CertificateAuthorityClientWrapper is the gRPC version of a core.CertificateAuthority client type CertificateAuthorityClientWrapper struct { inner caPB.CertificateAuthorityClient diff --git a/mocks/mocks.go b/mocks/mocks.go index 79480f960be..caa78acb7a3 100644 --- a/mocks/mocks.go +++ b/mocks/mocks.go @@ -374,6 +374,11 @@ func (*Publisher) SubmitToCT(_ context.Context, der []byte) error { return nil } +// SubmitToSingleCT is a mock +func (*Publisher) SubmitToSingleCT(_ context.Context, _, _ string, _ []byte) error { + return nil +} + // Statter is a stat counter that is a no-op except for locally handling Inc // calls (which are most of what we use). type Statter struct { diff --git a/publisher/mock_publisher/gen.go b/publisher/mock_publisher/gen.go new file mode 100644 index 00000000000..e509f8464da --- /dev/null +++ b/publisher/mock_publisher/gen.go @@ -0,0 +1,3 @@ +package mock_publisher + +//go:generate mockgen -package mock_publisher -destination ./mock_publisher.go github.com/letsencrypt/boulder/core Publisher diff --git a/publisher/mock_publisher/mock_publisher.go b/publisher/mock_publisher/mock_publisher.go new file mode 100644 index 00000000000..2ca504e16af --- /dev/null +++ b/publisher/mock_publisher/mock_publisher.go @@ -0,0 +1,50 @@ +// Automatically generated by MockGen. DO NOT EDIT! +// Source: github.com/letsencrypt/boulder/core (interfaces: Publisher) + +package mock_publisher + +import ( + gomock "github.com/golang/mock/gomock" + context "golang.org/x/net/context" +) + +// Mock of Publisher interface +type MockPublisher struct { + ctrl *gomock.Controller + recorder *_MockPublisherRecorder +} + +// Recorder for MockPublisher (not exported) +type _MockPublisherRecorder struct { + mock *MockPublisher +} + +func NewMockPublisher(ctrl *gomock.Controller) *MockPublisher { + mock := &MockPublisher{ctrl: ctrl} + mock.recorder = &_MockPublisherRecorder{mock} + return mock +} + +func (_m *MockPublisher) EXPECT() *_MockPublisherRecorder { + return _m.recorder +} + +func (_m *MockPublisher) SubmitToCT(_param0 context.Context, _param1 []byte) error { + ret := _m.ctrl.Call(_m, "SubmitToCT", _param0, _param1) + ret0, _ := ret[0].(error) + return ret0 +} + +func (_mr *_MockPublisherRecorder) SubmitToCT(arg0, arg1 interface{}) *gomock.Call { + return _mr.mock.ctrl.RecordCall(_mr.mock, "SubmitToCT", arg0, arg1) +} + +func (_m *MockPublisher) SubmitToSingleCT(_param0 context.Context, _param1 string, _param2 string, _param3 []byte) error { + ret := _m.ctrl.Call(_m, "SubmitToSingleCT", _param0, _param1, _param2, _param3) + ret0, _ := ret[0].(error) + return ret0 +} + +func (_mr *_MockPublisherRecorder) SubmitToSingleCT(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { + return _mr.mock.ctrl.RecordCall(_mr.mock, "SubmitToSingleCT", arg0, arg1, arg2, arg3) +} diff --git a/publisher/proto/publisher.pb.go b/publisher/proto/publisher.pb.go index 4dd937e597d..4260ac1c670 100644 --- a/publisher/proto/publisher.pb.go +++ b/publisher/proto/publisher.pb.go @@ -35,8 +35,10 @@ var _ = math.Inf const _ = proto.ProtoPackageIsVersion2 // please upgrade the proto package type Request struct { - Der []byte `protobuf:"bytes,1,opt,name=der" json:"der,omitempty"` - XXX_unrecognized []byte `json:"-"` + Der []byte `protobuf:"bytes,1,opt,name=der" json:"der,omitempty"` + LogURL *string `protobuf:"bytes,2,opt,name=LogURL" json:"LogURL,omitempty"` + LogPublicKey *string `protobuf:"bytes,3,opt,name=LogPublicKey" json:"LogPublicKey,omitempty"` + XXX_unrecognized []byte `json:"-"` } func (m *Request) Reset() { *m = Request{} } @@ -51,6 +53,20 @@ func (m *Request) GetDer() []byte { return nil } +func (m *Request) GetLogURL() string { + if m != nil && m.LogURL != nil { + return *m.LogURL + } + return "" +} + +func (m *Request) GetLogPublicKey() string { + if m != nil && m.LogPublicKey != nil { + return *m.LogPublicKey + } + return "" +} + type Empty struct { XXX_unrecognized []byte `json:"-"` } @@ -77,6 +93,7 @@ const _ = grpc.SupportPackageIsVersion3 type PublisherClient interface { SubmitToCT(ctx context.Context, in *Request, opts ...grpc.CallOption) (*Empty, error) + SubmitToSingleCT(ctx context.Context, in *Request, opts ...grpc.CallOption) (*Empty, error) } type publisherClient struct { @@ -96,10 +113,20 @@ func (c *publisherClient) SubmitToCT(ctx context.Context, in *Request, opts ...g return out, nil } +func (c *publisherClient) SubmitToSingleCT(ctx context.Context, in *Request, opts ...grpc.CallOption) (*Empty, error) { + out := new(Empty) + err := grpc.Invoke(ctx, "/Publisher/SubmitToSingleCT", in, out, c.cc, opts...) + if err != nil { + return nil, err + } + return out, nil +} + // Server API for Publisher service type PublisherServer interface { SubmitToCT(context.Context, *Request) (*Empty, error) + SubmitToSingleCT(context.Context, *Request) (*Empty, error) } func RegisterPublisherServer(s *grpc.Server, srv PublisherServer) { @@ -124,6 +151,24 @@ func _Publisher_SubmitToCT_Handler(srv interface{}, ctx context.Context, dec fun return interceptor(ctx, in, info, handler) } +func _Publisher_SubmitToSingleCT_Handler(srv interface{}, ctx context.Context, dec func(interface{}) error, interceptor grpc.UnaryServerInterceptor) (interface{}, error) { + in := new(Request) + if err := dec(in); err != nil { + return nil, err + } + if interceptor == nil { + return srv.(PublisherServer).SubmitToSingleCT(ctx, in) + } + info := &grpc.UnaryServerInfo{ + Server: srv, + FullMethod: "/Publisher/SubmitToSingleCT", + } + handler := func(ctx context.Context, req interface{}) (interface{}, error) { + return srv.(PublisherServer).SubmitToSingleCT(ctx, req.(*Request)) + } + return interceptor(ctx, in, info, handler) +} + var _Publisher_serviceDesc = grpc.ServiceDesc{ ServiceName: "Publisher", HandlerType: (*PublisherServer)(nil), @@ -132,6 +177,10 @@ var _Publisher_serviceDesc = grpc.ServiceDesc{ MethodName: "SubmitToCT", Handler: _Publisher_SubmitToCT_Handler, }, + { + MethodName: "SubmitToSingleCT", + Handler: _Publisher_SubmitToSingleCT_Handler, + }, }, Streams: []grpc.StreamDesc{}, Metadata: fileDescriptor0, @@ -140,12 +189,15 @@ var _Publisher_serviceDesc = grpc.ServiceDesc{ func init() { proto.RegisterFile("publisher.proto", fileDescriptor0) } var fileDescriptor0 = []byte{ - // 107 bytes of a gzipped FileDescriptorProto + // 155 bytes of a gzipped FileDescriptorProto 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x09, 0x6e, 0x88, 0x02, 0xff, 0xe2, 0xe2, 0x2f, 0x28, 0x4d, 0xca, - 0xc9, 0x2c, 0xce, 0x48, 0x2d, 0xd2, 0x2b, 0x28, 0xca, 0x2f, 0xc9, 0x57, 0x12, 0xe3, 0x62, 0x0f, + 0xc9, 0x2c, 0xce, 0x48, 0x2d, 0xd2, 0x2b, 0x28, 0xca, 0x2f, 0xc9, 0x57, 0xb2, 0xe1, 0x62, 0x0f, 0x4a, 0x2d, 0x2c, 0x4d, 0x2d, 0x2e, 0x11, 0xe2, 0xe6, 0x62, 0x4e, 0x49, 0x2d, 0x92, 0x60, 0x54, - 0x60, 0xd4, 0xe0, 0x51, 0x62, 0xe7, 0x62, 0x75, 0xcd, 0x2d, 0x28, 0xa9, 0x34, 0xd2, 0xe5, 0xe2, - 0x0c, 0x80, 0xe9, 0x11, 0x52, 0xe0, 0xe2, 0x0a, 0x2e, 0x4d, 0xca, 0xcd, 0x2c, 0x09, 0xc9, 0x77, - 0x0e, 0x11, 0xe2, 0xd0, 0x83, 0x6a, 0x95, 0x62, 0xd3, 0x03, 0x2b, 0x56, 0x62, 0x00, 0x04, 0x00, - 0x00, 0xff, 0xff, 0x19, 0x50, 0xa9, 0x22, 0x61, 0x00, 0x00, 0x00, + 0x60, 0xd4, 0xe0, 0x11, 0xe2, 0xe3, 0x62, 0xf3, 0xc9, 0x4f, 0x0f, 0x0d, 0xf2, 0x91, 0x60, 0x52, + 0x60, 0xd4, 0xe0, 0x14, 0x12, 0xe1, 0xe2, 0xf1, 0xc9, 0x4f, 0x0f, 0x00, 0xe9, 0x4e, 0xf6, 0x4e, + 0xad, 0x94, 0x60, 0x06, 0x89, 0x2a, 0xb1, 0x73, 0xb1, 0xba, 0xe6, 0x16, 0x94, 0x54, 0x1a, 0x85, + 0x72, 0x71, 0x06, 0xc0, 0x4c, 0x16, 0x52, 0xe0, 0xe2, 0x0a, 0x2e, 0x4d, 0xca, 0xcd, 0x2c, 0x09, + 0xc9, 0x77, 0x0e, 0x11, 0xe2, 0xd0, 0x83, 0x5a, 0x20, 0xc5, 0xa6, 0x07, 0x56, 0xac, 0xc4, 0x20, + 0xa4, 0xc6, 0x25, 0x00, 0x53, 0x11, 0x9c, 0x99, 0x97, 0x9e, 0x93, 0x8a, 0x5d, 0x1d, 0x20, 0x00, + 0x00, 0xff, 0xff, 0x19, 0x25, 0xca, 0x2f, 0xaf, 0x00, 0x00, 0x00, } diff --git a/publisher/proto/publisher.proto b/publisher/proto/publisher.proto index f8b2f5c56c0..e6d24850426 100644 --- a/publisher/proto/publisher.proto +++ b/publisher/proto/publisher.proto @@ -2,10 +2,13 @@ syntax = "proto2"; service Publisher { rpc SubmitToCT(Request) returns (Empty) {} + rpc SubmitToSingleCT(Request) returns (Empty) {} } message Request { optional bytes der = 1; + optional string LogURL = 2; + optional string LogPublicKey = 3; } message Empty { diff --git a/publisher/publisher.go b/publisher/publisher.go index 469794c6a53..80220f280d2 100644 --- a/publisher/publisher.go +++ b/publisher/publisher.go @@ -7,6 +7,7 @@ import ( "net/http" "net/url" "strings" + "sync" "time" ct "github.com/google/certificate-transparency/go" @@ -20,12 +21,53 @@ import ( // Log contains the CT client and signature verifier for a particular CT log type Log struct { + logID string uri string statName string client *ctClient.LogClient verifier *ct.SignatureVerifier } +// logCache contains a cache of *Log's that are constructed as required by +// `SubmitToSingleCT` +type logCache struct { + sync.RWMutex + logs map[string]*Log +} + +// AddLog adds a *Log to the cache by constructing the statName, client and +// verifier for the given uri & base64 public key. +func (c *logCache) AddLog(uri, b64PK string) (*Log, error) { + // Lock the mutex for reading to check the cache + c.RLock() + log, present := c.logs[b64PK] + c.RUnlock() + + // If we have already added this log, give it back + if present { + return log, nil + } + + // Lock the mutex for writing to add to the cache + c.Lock() + defer c.Unlock() + + // Construct a Log, add it to the cache, and return it to the caller + log, err := NewLog(uri, b64PK) + if err != nil { + return nil, err + } + c.logs[b64PK] = log + return log, nil +} + +// Len returns the number of logs in the logCache +func (c *logCache) Len() int { + c.RLock() + defer c.RUnlock() + return len(c.logs) +} + // NewLog returns an initialized Log struct func NewLog(uri, b64PK string) (*Log, error) { url, err := url.Parse(uri) @@ -54,6 +96,7 @@ func NewLog(uri, b64PK string) (*Log, error) { sanitizedPath = strings.Replace(sanitizedPath, "/", ".", -1) return &Log{ + logID: b64PK, uri: uri, statName: fmt.Sprintf("%s.%s", url.Host, sanitizedPath), client: client, @@ -67,10 +110,13 @@ type ctSubmissionRequest struct { // Impl defines a Publisher type Impl struct { - log blog.Logger - stats metrics.Scope - client *http.Client - issuerBundle []ct.ASN1Cert + log blog.Logger + stats metrics.Scope + client *http.Client + issuerBundle []ct.ASN1Cert + ctLogsCache logCache + // ctLogs is slightly redundant with the logCache, and should be removed. See + // issue https://github.com/letsencrypt/boulder/issues/2357 ctLogs []*Log submissionTimeout time.Duration @@ -93,16 +139,22 @@ func New( return &Impl{ submissionTimeout: submissionTimeout, issuerBundle: bundle, - ctLogs: logs, - log: logger, - stats: stats, - sa: sa, + ctLogsCache: logCache{ + logs: make(map[string]*Log), + }, + ctLogs: logs, + log: logger, + stats: stats, + sa: sa, } } -// SubmitToCT will submit the certificate represented by certDER to any CT -// logs configured in pub.CT.Logs (AMQP RPC method). -func (pub *Impl) SubmitToCT(ctx context.Context, der []byte) error { +// SubmitToSingleCT will submit the certificate represented by certDER to the CT +// log specified by log URL and public key (base64) +func (pub *Impl) SubmitToSingleCT( + ctx context.Context, + logURL, logPublicKey string, + der []byte) error { cert, err := x509.ParseCertificate(der) if err != nil { pub.log.AuditErr(fmt.Sprintf("Failed to parse certificate: %s", err)) @@ -112,21 +164,52 @@ func (pub *Impl) SubmitToCT(ctx context.Context, der []byte) error { localCtx, cancel := context.WithTimeout(ctx, pub.submissionTimeout) defer cancel() chain := append([]ct.ASN1Cert{der}, pub.issuerBundle...) + + // Add a log URL/pubkey to the cache, if already present the + // existing *Log will be returned, otherwise one will be constructed, added + // and returned. + ctLog, err := pub.ctLogsCache.AddLog(logURL, logPublicKey) + if err != nil { + pub.log.AuditErr(fmt.Sprintf("Making Log: %s", err)) + return err + } + + stats := pub.stats.NewScope(ctLog.statName) + stats.Inc("Submits", 1) + start := time.Now() + err = pub.singleLogSubmit( + localCtx, + chain, + core.SerialToString(cert.SerialNumber), + ctLog) + stats.TimingDuration("SubmitLatency", time.Now().Sub(start)) + if err != nil { + pub.log.AuditErr( + fmt.Sprintf("Failed to submit certificate to CT log at %s: %s", ctLog.uri, err)) + stats.Inc("Errors", 1) + } + + return nil +} + +// SubmitToCT will submit the certificate represented by certDER to any CT +// logs configured in pub.CT.Logs (AMQP RPC method). +func (pub *Impl) SubmitToCT(ctx context.Context, der []byte) error { for _, ctLog := range pub.ctLogs { - stats := pub.stats.NewScope(ctLog.statName) - stats.Inc("Submits", 1) - start := time.Now() - err := pub.singleLogSubmit(localCtx, chain, core.SerialToString(cert.SerialNumber), ctLog) - stats.TimingDuration("SubmitLatency", time.Now().Sub(start)) + err := pub.SubmitToSingleCT(ctx, ctLog.uri, ctLog.logID, der) if err != nil { - pub.log.AuditErr(fmt.Sprintf("Failed to submit certificate to CT log at %s: %s", ctLog.uri, err)) - stats.Inc("Errors", 1) + return err } } return nil } -func (pub *Impl) singleLogSubmit(ctx context.Context, chain []ct.ASN1Cert, serial string, ctLog *Log) error { +func (pub *Impl) singleLogSubmit( + ctx context.Context, + chain []ct.ASN1Cert, + serial string, + ctLog *Log) error { + sct, err := ctLog.client.AddChainWithContext(ctx, chain) if err != nil { return err diff --git a/publisher/publisher_test.go b/publisher/publisher_test.go index 3fdcbb762b2..21742389aa1 100644 --- a/publisher/publisher_test.go +++ b/publisher/publisher_test.go @@ -433,3 +433,52 @@ func TestBadServer(t *testing.T) { test.AssertNotError(t, err, "Certificate submission failed") test.AssertEquals(t, len(log.GetAllMatching("failed to verify ecdsa signature")), 1) } + +func TestLogCache(t *testing.T) { + cache := logCache{ + logs: make(map[string]*Log), + } + + // Adding a log with an invalid base64 public key should error + _, err := cache.AddLog("www.test.com", "1234") + test.AssertError(t, err, "AddLog() with invalid base64 pk didn't error") + + // Adding a log with an invalid URI should error + _, err = cache.AddLog(":", "") + test.AssertError(t, err, "AddLog() with an invalid log URI didn't error") + + // Create one keypair & base 64 public key + k1, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + test.AssertNotError(t, err, "ecdsa.GenerateKey() failed for k1") + der1, err := x509.MarshalPKIXPublicKey(&k1.PublicKey) + test.AssertNotError(t, err, "x509.MarshalPKIXPublicKey(der1) failed") + k1b64 := base64.StdEncoding.EncodeToString(der1) + + // Create a second keypair & base64 public key + k2, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + test.AssertNotError(t, err, "ecdsa.GenerateKey() failed for k2") + der2, err := x509.MarshalPKIXPublicKey(&k2.PublicKey) + test.AssertNotError(t, err, "x509.MarshalPKIXPublicKey(der2) failed") + k2b64 := base64.StdEncoding.EncodeToString(der2) + + // Adding the first log should not produce an error + l1, err := cache.AddLog("http://log.one.example.com", k1b64) + test.AssertNotError(t, err, "cache.AddLog() failed for log 1") + test.AssertEquals(t, cache.Len(), 1) + test.AssertEquals(t, l1.uri, "http://log.one.example.com") + test.AssertEquals(t, l1.logID, k1b64) + + // Adding it again should not produce any errors, or increase the Len() + l1, err = cache.AddLog("http://log.one.example.com", k1b64) + test.AssertNotError(t, err, "cache.AddLog() failed for second add of log 1") + test.AssertEquals(t, cache.Len(), 1) + test.AssertEquals(t, l1.uri, "http://log.one.example.com") + test.AssertEquals(t, l1.logID, k1b64) + + // Adding a second log should not error and should increase the Len() + l2, err := cache.AddLog("http://log.two.example.com", k2b64) + test.AssertNotError(t, err, "cache.AddLog() failed for log 2") + test.AssertEquals(t, cache.Len(), 2) + test.AssertEquals(t, l2.uri, "http://log.two.example.com") + test.AssertEquals(t, l2.logID, k2b64) +} diff --git a/rpc/rpc-wrappers.go b/rpc/rpc-wrappers.go index f33711714ca..72e8cc22382 100644 --- a/rpc/rpc-wrappers.go +++ b/rpc/rpc-wrappers.go @@ -670,6 +670,14 @@ func (pub PublisherClient) SubmitToCT(ctx context.Context, der []byte) (err erro return } +// The only consumer of the publisher service's `SubmitToSingleCT` func is the +// `ocsp-updater`. Since it will *only* use gRPC to communicate with the +// Publisher we *do not* implement `SubmitToSingleCT` for AQMP. This method is +// here only to satisfy the publisher interface +func (pub PublisherClient) SubmitToSingleCT(ctx context.Context, logURL, logPublicKey string, der []byte) (err error) { + return fmt.Errorf("SubmitToSingleCT is not implemented for AQMP publisher client") +} + // NewCertificateAuthorityServer constructs an RPC server // // CertificateAuthorityClient / Server diff --git a/test.sh b/test.sh index aab2c32e24c..d29bdc36ff9 100755 --- a/test.sh +++ b/test.sh @@ -243,6 +243,12 @@ if [[ "$RUN" =~ "generate" ]] ; then go install ./probs go install google.golang.org/grpc/codes run_and_expect_silence go generate ${TESTPATHS} + # Because the `mock` package we use to generate mocks does not properly + # support vendored dependencies[0] we are forced to sed out any references to + # the vendor directory that sneak into generated resources. + # [0] - https://github.com/golang/mock/issues/30 + goSrcFiles=$(find . -name "*.go" -not -path "./vendor/*" -print) + run_and_expect_silence sed -i 's/github.com\/letsencrypt\/boulder\/vendor\///g' ${goSrcFiles} run_and_expect_silence git diff --exit-code $(ls | grep -v Godeps) end_context #"generate" fi diff --git a/test/config-next/ocsp-updater.json b/test/config-next/ocsp-updater.json index aeaae6549d8..dc7c31b5a66 100644 --- a/test/config-next/ocsp-updater.json +++ b/test/config-next/ocsp-updater.json @@ -35,6 +35,9 @@ "clientCertificatePath": "test/grpc-creds/boulder-client/cert.pem", "clientKeyPath": "test/grpc-creds/boulder-client/key.pem", "timeout": "15s" + }, + "features": { + "ResubmitMissingSCTsOnly": true } },