From 4e1051bfdc5144dd9de97f978a2c24e58fc6ada5 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Mon, 27 Jul 2015 16:40:04 -0700 Subject: [PATCH] Fix OCSP updating. Fixes https://github.com/letsencrypt/boulder/issues/539. Passes a pointer to tx.Update() in the SA, resolving the gorp error we were previously receiving in UpdateOCSP. Fixes CA code to properly receive the error from UpdateOCSP, so future errors will be logged correctly. --- ca/certificate-authority.go | 2 +- ca/certificate-authority_test.go | 5 +++-- sa/storage-authority.go | 2 +- sa/storage-authority_test.go | 29 +++++++++++++++++++++++++++++ 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/ca/certificate-authority.go b/ca/certificate-authority.go index 43a23c25e..0b274e6d7 100644 --- a/ca/certificate-authority.go +++ b/ca/certificate-authority.go @@ -474,7 +474,7 @@ func (ca *CertificateAuthorityImpl) IssueCertificate(csr x509.CertificateRequest return cert, nil } - ca.SA.UpdateOCSP(serial, ocspResponse) + err = ca.SA.UpdateOCSP(serial, ocspResponse) if err != nil { ca.log.Warning(fmt.Sprintf("Post-Issuance OCSP failed storing: %s", err)) return cert, nil diff --git a/ca/certificate-authority_test.go b/ca/certificate-authority_test.go index 92c568df1..2b41246bc 100644 --- a/ca/certificate-authority_test.go +++ b/ca/certificate-authority_test.go @@ -436,8 +436,9 @@ func TestRevoke(t *testing.T) { test.AssertNotError(t, err, "Failed to get cert status") test.AssertEquals(t, status.Status, core.OCSPStatusRevoked) - test.Assert(t, time.Now().Sub(status.OCSPLastUpdated) > time.Second, - fmt.Sprintf("OCSP LastUpdated was wrong: %v", status.OCSPLastUpdated)) + secondAgo := time.Now().Add(-time.Second) + test.Assert(t, status.OCSPLastUpdated.After(secondAgo), + fmt.Sprintf("OCSP LastUpdated was more than a second old: %v", status.OCSPLastUpdated)) } func TestIssueCertificate(t *testing.T) { diff --git a/sa/storage-authority.go b/sa/storage-authority.go index 6d2013080..4b83b29f7 100644 --- a/sa/storage-authority.go +++ b/sa/storage-authority.go @@ -266,7 +266,7 @@ func (ssa *SQLStorageAuthority) UpdateOCSP(serial string, ocspResponse []byte) ( // Reset the update clock status.OCSPLastUpdated = timeStamp - _, err = tx.Update(status) + _, err = tx.Update(&status) if err != nil { tx.Rollback() return err diff --git a/sa/storage-authority_test.go b/sa/storage-authority_test.go index 149ee350c..228415e2a 100644 --- a/sa/storage-authority_test.go +++ b/sa/storage-authority_test.go @@ -211,3 +211,32 @@ func TestDeniedCSR(t *testing.T) { test.AssertNotError(t, err, "AlreadyDeniedCSR failed") test.Assert(t, !exists, "Found non-existent CSR") } + +func TestUpdateOCSP(t *testing.T) { + sa := initSA(t) + + // Add a cert to the DB to test with. + certDER, err := ioutil.ReadFile("www.eff.org.der") + test.AssertNotError(t, err, "Couldn't read example cert DER") + _, err = sa.AddCertificate(certDER, 1) + test.AssertNotError(t, err, "Couldn't add www.eff.org.der") + + serial := "00000000000000000000000000021bd4" + const ocspResponse = "this is a fake OCSP response" + err = sa.UpdateOCSP(serial, []byte(ocspResponse)) + test.AssertNotError(t, err, "UpdateOCSP failed") + + certificateStatusObj, err := sa.dbMap.Get(core.CertificateStatus{}, serial) + certificateStatus := certificateStatusObj.(*core.CertificateStatus) + test.AssertNotError(t, err, "Failed to fetch certificate status") + test.Assert(t, + certificateStatus.OCSPLastUpdated.After(time.Now().Add(-time.Second)), + "OCSP last updated too old.") + + var fetchedOcspResponse core.OCSPResponse + err = sa.dbMap.SelectOne(&fetchedOcspResponse, + `SELECT * from ocspResponses where serial = ? order by createdAt DESC limit 1;`, + serial) + test.AssertNotError(t, err, "Failed to fetch OCSP response") + test.AssertEquals(t, ocspResponse, string(fetchedOcspResponse.Response)) +}