Check for duplicate certs before adding to db (letsencrypt#5497)

andygabby · web-flow · commit ad34089cdaf1 · 2021-07-08T12:51:57.000-06:00
* Check for duplicate certs before adding to db Error at SA if the certificate or precertificate already exist in the database Fixes: letsencrypt#5468
diff --git a/sa/precertificates.go b/sa/precertificates.go
@@ -61,6 +61,16 @@ func (ssa *SQLStorageAuthority) AddPrecertificate(ctx context.Context, req *sapb
 	}
 
 	_, overallError := db.WithTransaction(ctx, ssa.dbMap, func(txWithCtx db.Executor) (interface{}, error) {
+		// Select to see if precert exists
+		var row struct {
+			Count int64
+		}
+		if err := txWithCtx.SelectOne(&row, "SELECT count(1) as count FROM precertificates WHERE serial=?", serialHex); err != nil {
+			return nil, err
+		}
+		if row.Count > 0 {
+			return nil, berrors.DuplicateError("cannot add a duplicate cert")
+		}
 		if err := txWithCtx.Insert(preCertModel); err != nil {
 			return nil, err
 		}
diff --git a/sa/precertificates_test.go b/sa/precertificates_test.go
@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/letsencrypt/boulder/db"
+	berrors "github.com/letsencrypt/boulder/errors"
 	sapb "github.com/letsencrypt/boulder/sa/proto"
 	"github.com/letsencrypt/boulder/sa/satest"
 	"github.com/letsencrypt/boulder/test"
@@ -87,6 +88,32 @@ func TestAddPrecertificate(t *testing.T) {
 	addPrecert(true)
 }
 
+func TestAddPreCertificateDuplicate(t *testing.T) {
+	sa, clk, cleanUp := initSA(t)
+	defer cleanUp()
+
+	reg := satest.CreateWorkingRegistration(t, sa)
+
+	_, testCert := test.ThrowAwayCert(t, 1)
+
+	_, err := sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
+		Der:      testCert.Raw,
+		Issued:   clk.Now().UnixNano(),
+		RegID:    reg.ID,
+		IssuerID: 1,
+	})
+	test.AssertNotError(t, err, "Couldn't add test certificate")
+
+	_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
+		Der:      testCert.Raw,
+		Issued:   clk.Now().UnixNano(),
+		RegID:    reg.ID,
+		IssuerID: 1,
+	})
+	test.AssertDeepEquals(t, err, berrors.DuplicateError("cannot add a duplicate cert"))
+
+}
+
 func TestAddPrecertificateIncomplete(t *testing.T) {
 	sa, _, cleanUp := initSA(t)
 	defer cleanUp()
diff --git a/sa/sa.go b/sa/sa.go
@@ -417,12 +417,19 @@ func (ssa *SQLStorageAuthority) AddCertificate(
 	}
 
 	isRenewalRaw, overallError := db.WithTransaction(ctx, ssa.dbMap, func(txWithCtx db.Executor) (interface{}, error) {
+		// Select to see if cert exists
+		var row struct {
+			Count int64
+		}
+		if err := txWithCtx.SelectOne(&row, "SELECT count(1) as count FROM certificates WHERE serial=?", serial); err != nil {
+			return nil, err
+		}
+		if row.Count > 0 {
+			return nil, berrors.DuplicateError("cannot add a duplicate cert")
+		}
 		// Save the final certificate
 		err = txWithCtx.Insert(cert)
 		if err != nil {
-			if db.IsDuplicate(err) {
-				return nil, berrors.DuplicateError("cannot add a duplicate cert")
-			}
 			return nil, err
 		}
 
diff --git a/sa/sa_test.go b/sa/sa_test.go
@@ -235,6 +235,23 @@ func TestAddCertificate(t *testing.T) {
 	test.AssertNotError(t, err, "Couldn't add test-cert2.der")
 }
 
+func TestAddCertificateDuplicate(t *testing.T) {
+	sa, clk, cleanUp := initSA(t)
+	defer cleanUp()
+
+	reg := satest.CreateWorkingRegistration(t, sa)
+
+	_, testCert := test.ThrowAwayCert(t, 1)
+
+	issuedTime := clk.Now()
+	_, err := sa.AddCertificate(ctx, testCert.Raw, reg.ID, nil, &issuedTime)
+	test.AssertNotError(t, err, "Couldn't add test certificate")
+
+	_, err = sa.AddCertificate(ctx, testCert.Raw, reg.ID, nil, &issuedTime)
+	test.AssertDeepEquals(t, err, berrors.DuplicateError("cannot add a duplicate cert"))
+
+}
+
 func TestCountCertificatesByNames(t *testing.T) {
 	sa, clk, cleanUp := initSA(t)
 	defer cleanUp()
