Merge branch 'master' into gc-stats-fix

cmd/ocsp-responder/main.go

@@ -8,6 +8,7 @@ package main
 import (
 	"bytes"
 	"crypto/x509"
+	"database/sql"
 	"encoding/hex"
 	"errors"
 	"fmt"
@@ -20,7 +21,6 @@ import (
 	"github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/facebookgo/httpdown"
 	"github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock"
 	"github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/crypto/ocsp"
-	gorp "github.com/letsencrypt/boulder/Godeps/_workspace/src/gopkg.in/gorp.v1"
 	"github.com/letsencrypt/boulder/metrics"
 
 	"github.com/letsencrypt/boulder/cmd"
@@ -55,34 +55,41 @@ serialNumber field, since we will always query on it.
 
 */
 type DBSource struct {
-	dbMap     *gorp.DbMap
+	dbMap     dbSelector
 	caKeyHash []byte
+	log       *blog.AuditLogger
+}
+
+// Since the only thing we use from gorp is the SelectOne method on the
+// gorp.DbMap object, we just define the interface an interface with that method
+// instead of importing all of gorp. This also allows us to simulate MySQL failures
+// by mocking the interface.
+type dbSelector interface {
+	SelectOne(holder interface{}, query string, args ...interface{}) error
 }
 
 // NewSourceFromDatabase produces a DBSource representing the binding of a
 // given DB schema to a CA key.
-func NewSourceFromDatabase(dbMap *gorp.DbMap, caKeyHash []byte) (src *DBSource, err error) {
+func NewSourceFromDatabase(dbMap dbSelector, caKeyHash []byte, log *blog.AuditLogger) (src *DBSource, err error) {
-	src = &DBSource{dbMap: dbMap, caKeyHash: caKeyHash}
+	src = &DBSource{dbMap: dbMap, caKeyHash: caKeyHash, log: log}
 	return
 }
 
 // Response is called by the HTTP server to handle a new OCSP request.
 func (src *DBSource) Response(req *ocsp.Request) ([]byte, bool) {
-	log := blog.GetAuditLogger()
-
 	// Check that this request is for the proper CA
 	if bytes.Compare(req.IssuerKeyHash, src.caKeyHash) != 0 {
-		log.Debug(fmt.Sprintf("Request intended for CA Cert ID: %s", hex.EncodeToString(req.IssuerKeyHash)))
+		src.log.Debug(fmt.Sprintf("Request intended for CA Cert ID: %s", hex.EncodeToString(req.IssuerKeyHash)))
 		return nil, false
 	}
 
 	serialString := core.SerialToString(req.SerialNumber)
-	log.Debug(fmt.Sprintf("Searching for OCSP issued by us for serial %s", serialString))
+	src.log.Debug(fmt.Sprintf("Searching for OCSP issued by us for serial %s", serialString))
 
 	var response []byte
 	defer func() {
 		if len(response) != 0 {
-			log.Info(fmt.Sprintf("OCSP Response sent for CA=%s, Serial=%s", hex.EncodeToString(src.caKeyHash), serialString))
+			src.log.Info(fmt.Sprintf("OCSP Response sent for CA=%s, Serial=%s", hex.EncodeToString(src.caKeyHash), serialString))
 		}
 	}()
 	// Note: we first check for an OCSP response in the certificateStatus table (
@@ -94,6 +101,9 @@ func (src *DBSource) Response(req *ocsp.Request) ([]byte, bool) {
 		"SELECT ocspResponse FROM certificateStatus WHERE serial = :serial",
 		map[string]interface{}{"serial": serialString},
 	)
+	if err != nil && err != sql.ErrNoRows {
+		src.log.Err(fmt.Sprintf("Failed to retrieve response from certificateStatus table: %s", err))
+	}
 	// TODO(#970): Delete this ocspResponses check once the table has been removed
 	if len(response) == 0 {
 		// Ignoring possible error, if response hasn't been filled, attempt to find
@@ -103,6 +113,9 @@ func (src *DBSource) Response(req *ocsp.Request) ([]byte, bool) {
 			"SELECT response from ocspResponses WHERE serial = :serial ORDER BY id DESC LIMIT 1;",
 			map[string]interface{}{"serial": serialString},
 		)
+		if err != nil && err != sql.ErrNoRows {
+			src.log.Err(fmt.Sprintf("Failed to retrieve response from ocspResponses table: %s", err))
+		}
 	}
 	if err != nil {
 		return nil, false
@@ -111,14 +124,7 @@ func (src *DBSource) Response(req *ocsp.Request) ([]byte, bool) {
 	return response, true
 }
 
-func makeDBSource(dbConnect, issuerCert string, sqlDebug bool) (*DBSource, error) {
+func makeDBSource(dbMap dbSelector, issuerCert string, log *blog.AuditLogger) (*DBSource, error) {
-	// Configure DB
-	dbMap, err := sa.NewDbMap(dbConnect)
-	if err != nil {
-		return nil, fmt.Errorf("Could not connect to database: %s", err)
-	}
-	sa.SetSQLDebug(dbMap, sqlDebug)
-
 	// Load the CA's key so we can store its SubjectKey in the DB
 	caCertDER, err := cmd.LoadCert(issuerCert)
 	if err != nil {
@@ -133,7 +139,7 @@ func makeDBSource(dbConnect, issuerCert string, sqlDebug bool) (*DBSource, error
 	}
 
 	// Construct source from DB
-	return NewSourceFromDatabase(dbMap, caCert.SubjectKeyId)
+	return NewSourceFromDatabase(dbMap, caCert.SubjectKeyId, log)
 }
 
 func main() {
@@ -163,7 +169,12 @@ func main() {
 
 		if url.Scheme == "mysql+tcp" {
 			auditlogger.Info(fmt.Sprintf("Loading OCSP Database for CA Cert: %s", c.Common.IssuerCert))
-			source, err = makeDBSource(config.Source, c.Common.IssuerCert, c.SQL.SQLDebug)
+			dbMap, err := sa.NewDbMap(config.Source)
+			cmd.FailOnError(err, "Could not connect to database")
+			if c.SQL.SQLDebug {
+				sa.SetSQLDebug(dbMap, true)
+			}
+			source, err = makeDBSource(dbMap, c.Common.IssuerCert, auditlogger)
 			cmd.FailOnError(err, "Couldn't load OCSP DB")
 		} else if url.Scheme == "file" {
 			filename := url.Path

cmd/ocsp-responder/main_test.go

@@ -12,7 +12,10 @@ import (
 
 	cfocsp "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/cloudflare/cfssl/ocsp"
 	"github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/crypto/ocsp"
+
 	"github.com/letsencrypt/boulder/core"
+	blog "github.com/letsencrypt/boulder/log"
+	"github.com/letsencrypt/boulder/mocks"
 	"github.com/letsencrypt/boulder/sa"
 	"github.com/letsencrypt/boulder/test"
 )
@@ -62,11 +65,14 @@ func TestHandler(t *testing.T) {
 }
 
 func TestDBHandler(t *testing.T) {
-	src, err := makeDBSource("mysql+tcp://ocsp_resp@localhost:3306/boulder_sa_test", "./testdata/test-ca.der.pem", false)
+	dbMap, err := sa.NewDbMap("mysql+tcp://ocsp_resp@localhost:3306/boulder_sa_test")
+	test.AssertNotError(t, err, "Could not connect to database")
+	src, err := makeDBSource(dbMap, "./testdata/test-ca.der.pem", blog.GetAuditLogger())
 	if err != nil {
 		t.Fatalf("makeDBSource: %s", err)
 	}
 	defer test.ResetSATestDatabase(t)
+
 	ocspResp, err := ocsp.ParseResponse(resp, nil)
 	if err != nil {
 		t.Fatalf("ocsp.ParseResponse: %s", err)
@@ -99,7 +105,31 @@ func TestDBHandler(t *testing.T) {
 	if !bytes.Equal(w.Body.Bytes(), resp) {
 		t.Errorf("Mismatched body: want %#v, got %#v", resp, w.Body.Bytes())
 	}
+}
 
+// brokenSelector allows us to test what happens when gorp SelectOne statements
+// throw errors and satisfies the dbSelector interface
+type brokenSelector struct{}
+
+func (bs brokenSelector) SelectOne(_ interface{}, _ string, _ ...interface{}) error {
+	return fmt.Errorf("Failure!")
+}
+
+func TestErrorLog(t *testing.T) {
+	src, err := makeDBSource(brokenSelector{}, "./testdata/test-ca.der.pem", blog.GetAuditLogger())
+	test.AssertNotError(t, err, "Failed to create broken dbMap")
+
+	src.log.SyslogWriter = mocks.NewSyslogWriter()
+	mockLog := src.log.SyslogWriter.(*mocks.SyslogWriter)
+
+	ocspReq, err := ocsp.ParseRequest(req)
+	test.AssertNotError(t, err, "Failed to parse OCSP request")
+
+	_, found := src.Response(ocspReq)
+	test.Assert(t, !found, "Somehow found OCSP response")
+
+	test.AssertEquals(t, len(mockLog.GetAllMatching("Failed to retrieve response from certificateStatus table")), 1)
+	test.AssertEquals(t, len(mockLog.GetAllMatching("Failed to retrieve response from ocspResponses table")), 1)
 }
 
 func mustRead(path string) []byte {

cmd/ocsp-updater/main.go

@@ -227,7 +227,7 @@ func (updater *OCSPUpdater) generateRevokedResponse(status core.CertificateStatu
 	return &status, nil
 }
 
-func (updater *OCSPUpdater) storeResponse(status *core.CertificateStatus, statusGuard core.OCSPStatus) error {
+func (updater *OCSPUpdater) storeResponse(status *core.CertificateStatus) error {
 	// Update the certificateStatus table with the new OCSP response, the status
 	// WHERE is used make sure we don't overwrite a revoked response with a one
 	// containing a 'good' status and that we don't do the inverse when the OCSP
@@ -240,7 +240,7 @@ func (updater *OCSPUpdater) storeResponse(status *core.CertificateStatus, status
 		status.OCSPResponse,
 		status.OCSPLastUpdated,
 		status.Serial,
-		string(statusGuard),
+		string(status.Status),
 	)
 	return err
 }
@@ -291,7 +291,7 @@ func (updater *OCSPUpdater) revokedCertificatesTick(batchSize int) {
 			updater.stats.Inc("OCSP.Errors.RevokedResponseGeneration", 1, 1.0)
 			continue
 		}
-		err = updater.storeResponse(meta, core.OCSPStatusRevoked)
+		err = updater.storeResponse(meta)
 		if err != nil {
 			updater.stats.Inc("OCSP.Errors.StoreRevokedResponse", 1, 1.0)
 			updater.log.AuditErr(fmt.Errorf("Failed to store OCSP response: %s", err))
@@ -309,7 +309,7 @@ func (updater *OCSPUpdater) generateOCSPResponses(statuses []core.CertificateSta
 			continue
 		}
 		updater.stats.Inc("OCSP.GeneratedResponses", 1, 1.0)
-		err = updater.storeResponse(meta, core.OCSPStatusGood)
+		err = updater.storeResponse(meta)
 		if err != nil {
 			updater.log.AuditErr(fmt.Errorf("Failed to store OCSP response: %s", err))
 			updater.stats.Inc("OCSP.Errors.StoreResponse", 1, 1.0)

cmd/ocsp-updater/main_test.go

@@ -93,12 +93,12 @@ func TestGenerateAndStoreOCSPResponse(t *testing.T) {
 
 	meta, err := updater.generateResponse(status)
 	test.AssertNotError(t, err, "Couldn't generate OCSP response")
-	err = updater.storeResponse(meta, core.OCSPStatusGood)
+	err = updater.storeResponse(meta)
 	test.AssertNotError(t, err, "Couldn't store certificate status")
 
 	secondMeta, err := updater.generateRevokedResponse(status)
 	test.AssertNotError(t, err, "Couldn't generate revoked OCSP response")
-	err = updater.storeResponse(secondMeta, core.OCSPStatusGood)
+	err = updater.storeResponse(secondMeta)
 	test.AssertNotError(t, err, "Couldn't store certificate status")
 
 	newStatus, err := sa.GetCertificateStatus(status.Serial)
@@ -152,7 +152,7 @@ func TestFindStaleOCSPResponses(t *testing.T) {
 
 	meta, err := updater.generateResponse(status)
 	test.AssertNotError(t, err, "Couldn't generate OCSP response")
-	err = updater.storeResponse(meta, core.OCSPStatusGood)
+	err = updater.storeResponse(meta)
 	test.AssertNotError(t, err, "Couldn't store OCSP response")
 
 	certs, err = updater.findStaleOCSPResponses(earliest, 10)
@@ -290,18 +290,27 @@ func TestStoreResponseGuard(t *testing.T) {
 	status, err := sa.GetCertificateStatus(core.SerialToString(parsedCert.SerialNumber))
 	test.AssertNotError(t, err, "Failed to get certificate status")
 
-	status.OCSPResponse = []byte{0}
+	err = sa.MarkCertificateRevoked(core.SerialToString(parsedCert.SerialNumber), 0)
-	err = updater.storeResponse(&status, core.OCSPStatusRevoked)
+	test.AssertNotError(t, err, "Failed to revoked certificate")
+
+	// Attempt to update OCSP response where status.Status is good but stored status
+	// is revoked, this should fail silently
+	status.OCSPResponse = []byte{0, 1, 1}
+	err = updater.storeResponse(&status)
 	test.AssertNotError(t, err, "Failed to update certificate status")
 
+	// Make sure the OCSP response hasn't actually changed
 	unchangedStatus, err := sa.GetCertificateStatus(core.SerialToString(parsedCert.SerialNumber))
 	test.AssertNotError(t, err, "Failed to get certificate status")
 	test.AssertEquals(t, len(unchangedStatus.OCSPResponse), 0)
 
-	err = updater.storeResponse(&status, core.OCSPStatusGood)
+	// Changing the status to the stored status should allow the update to occur
+	status.Status = core.OCSPStatusRevoked
+	err = updater.storeResponse(&status)
 	test.AssertNotError(t, err, "Failed to updated certificate status")
 
+	// Make sure the OCSP response has been updated
 	changedStatus, err := sa.GetCertificateStatus(core.SerialToString(parsedCert.SerialNumber))
 	test.AssertNotError(t, err, "Failed to get certificate status")
-	test.AssertEquals(t, len(changedStatus.OCSPResponse), 1)
+	test.AssertEquals(t, len(changedStatus.OCSPResponse), 3)
 }