Address review comments

rolandshoemaker · rolandshoemaker · commit 8d1ea7291fce · 2015-10-09T15:48:09.000-07:00
OCSP-Responder attempts to read the OCSP response from the certificateStatus table,
if it cannot find a response there it reads the ocspResponses table to try to find a
response, if neither contains a response the not found bool is passed back to the
Responder.
diff --git a/cmd/ocsp-responder/main.go b/cmd/ocsp-responder/main.go
@@ -8,6 +8,7 @@ package main
 import (
 	"bytes"
 	"crypto/x509"
+	"database/sql"
 	"encoding/hex"
 	"errors"
 	"fmt"
@@ -80,20 +81,33 @@ func (src *DBSource) Response(req *ocsp.Request) ([]byte, bool) {
 	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))
+		}
+	}()
 	// Note: we order by id rather than createdAt, because otherwise we sometimes
 	// get the wrong result if a certificate is revoked in the same second as its
 	// last update (e.g. client issues and instant revokes).
 	err := src.dbMap.SelectOne(
 		&response,
-		"SELECT ocspResponse from certificateStatus WHERE serial = :serial",
+		"SELECT ocspResponse FROM certificateStatus WHERE serial = :serial",
 		map[string]interface{}{"serial": serialString},
 	)
 	if err != nil || len(response) == 0 {
+		if err == sql.ErrNoRows || len(response) == 0 {
+			err := src.dbMap.SelectOne(
+				&response,
+				"SELECT response from ocspResponses WHERE serial = :serial ORDER BY id DESC LIMIT 1;",
+				map[string]interface{}{"serial": serialString},
+			)
+			if err == nil && len(response) != 0 {
+				return response, true
+			}
+		}
 		return nil, false
 	}
 
-	log.Info(fmt.Sprintf("OCSP Response sent for CA=%s, Serial=%s", hex.EncodeToString(src.caKeyHash), serialString))
-
 	return response, true
 }
 
diff --git a/cmd/ocsp-updater/main.go b/cmd/ocsp-updater/main.go
@@ -230,7 +230,7 @@ func (updater *OCSPUpdater) findRevokedCertificates(batchSize int) ([]core.Certi
 		&statuses,
 		`SELECT * FROM certificateStatus
 		 WHERE status = :revoked
-		 AND ocspLastUpdated < revokedDate
+		 AND ocspLastUpdated <= revokedDate
 		 LIMIT :limit`,
 		map[string]interface{}{
 			"revoked": string(core.OCSPStatusRevoked),
diff --git a/sa/_db/migrations/20151008132352_MoveOCSPResponseToCertificateStatus.sql b/sa/_db/migrations/20151008132352_MoveOCSPResponseToCertificateStatus.sql
@@ -3,21 +3,8 @@
 -- SQL in section 'Up' is executed when this migration is applied
 
 ALTER TABLE `certificateStatus` ADD COLUMN (`ocspResponse` blob);
-UPDATE certificateStatus,ocspResponses SET certificateStatus.ocspResponse = ocspResponses.Response;
-DROP TABLE `ocspResponses`;
 
 -- +goose Down
 -- SQL section 'Down' is executed when this migration is rolled back
 
-CREATE TABLE `ocspResponses` (
-  `id` int(11) NOT NULL AUTO_INCREMENT,
-  `serial` varchar(255) NOT NULL,
-  `createdAt` datetime NOT NULL,
-  `response` blob NOT NULL,
-  PRIMARY KEY (`id`),
-  KEY `SERIAL` (`serial`) COMMENT 'Actual lookup mechanism'
-) ENGINE=InnoDB DEFAULT CHARSET=utf8;
-UPDATE certificateStatus,ocspResponses
-SET ocspResponses.ocspResponse = certificateStatus.Response
-AND ocspResponses.createdAt = certificateStatus.ocspLastUpdated;
 ALTER TABLE `certificateStatus` DROP COLUMN `ocspResponse`;
