Skip to content

Commit

Permalink
Return OCSP unauthorized status if the certificate is expired (#4380)
Browse files Browse the repository at this point in the history
The ocsp-updater ocspStaleMaxAge config var has to be bumped up to ~7 months so that when it is run after the six-months-ago run it will actually update the ocsp responses generated during that period and mark the certificate status row as expired.

Fixes #4338.
  • Loading branch information
rolandshoemaker committed Aug 1, 2019
1 parent 8b51845 commit db01830
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 8 deletions.
6 changes: 4 additions & 2 deletions cmd/ocsp-responder/main.go
Expand Up @@ -103,6 +103,7 @@ func NewSourceFromDatabase(

type dbResponse struct {
OCSPResponse []byte
IsExpired bool
OCSPLastUpdated time.Time
}

Expand Down Expand Up @@ -143,7 +144,7 @@ func (src *DBSource) Response(req *ocsp.Request) ([]byte, http.Header, error) {
}
err := src.dbMap.WithContext(ctx).SelectOne(
&response,
"SELECT ocspResponse, ocspLastUpdated FROM certificateStatus WHERE serial = :serial",
"SELECT ocspResponse, isExpired, ocspLastUpdated FROM certificateStatus WHERE serial = :serial",
map[string]interface{}{"serial": serialString},
)
if err == sql.ErrNoRows {
Expand All @@ -156,8 +157,9 @@ func (src *DBSource) Response(req *ocsp.Request) ([]byte, http.Header, error) {
if response.OCSPLastUpdated.IsZero() {
src.log.Debugf("OCSP Response not sent (ocspLastUpdated is zero) for CA=%s, Serial=%s", hex.EncodeToString(src.caKeyHash), serialString)
return nil, nil, cfocsp.ErrNotFound
} else if response.IsExpired {
return nil, nil, cfocsp.ErrNotFound
}

return response.OCSPResponse, nil, nil
}

Expand Down
28 changes: 27 additions & 1 deletion cmd/ocsp-responder/main_test.go
Expand Up @@ -27,7 +27,7 @@ import (

var (
req = mustRead("./testdata/ocsp.req")
resp = dbResponse{mustRead("./testdata/ocsp.resp"), time.Now()}
resp = dbResponse{mustRead("./testdata/ocsp.resp"), false, time.Now()}
stats = metrics.NewNoopScope()
)

Expand Down Expand Up @@ -252,3 +252,29 @@ func TestRequiredSerialPrefix(t *testing.T) {
_, _, err = src.Response(ocspReq)
test.AssertNotError(t, err, "src.Response failed with acceptable prefix")
}

type expiredSelector struct {
mockSqlExecutor
}

func (es expiredSelector) SelectOne(obj interface{}, _ string, _ ...interface{}) error {
rows := obj.(*dbResponse)
rows.IsExpired = true
rows.OCSPLastUpdated = time.Time{}.Add(time.Hour)
return nil
}

func (es expiredSelector) WithContext(context.Context) gorp.SqlExecutor {
return es
}

func TestExpiredUnauthorized(t *testing.T) {
src, err := makeDBSource(expiredSelector{}, "./testdata/test-ca.der.pem", []string{"00"}, time.Second, blog.NewMock())
test.AssertNotError(t, err, "makeDBSource failed")

ocspReq, err := ocsp.ParseRequest(req)
test.AssertNotError(t, err, "Failed to parse OCSP request")

_, _, err = src.Response(ocspReq)
test.AssertEquals(t, err, cfocsp.ErrNotFound)
}
2 changes: 1 addition & 1 deletion test/config-next/ocsp-updater.json
Expand Up @@ -8,7 +8,7 @@
"missingSCTBatchSize": 5000,
"parallelGenerateOCSPRequests": 10,
"ocspMinTimeToExpiry": "72h",
"ocspStaleMaxAge": "720h",
"ocspStaleMaxAge": "5040h",
"oldestIssuedSCT": "72h",
"signFailureBackoffFactor": 1.2,
"signFailureBackoffMax": "30m",
Expand Down
2 changes: 1 addition & 1 deletion test/config/ocsp-updater.json
Expand Up @@ -8,7 +8,7 @@
"missingSCTBatchSize": 5000,
"parallelGenerateOCSPRequests": 10,
"ocspMinTimeToExpiry": "72h",
"ocspStaleMaxAge": "720h",
"ocspStaleMaxAge": "5040h",
"oldestIssuedSCT": "72h",
"signFailureBackoffFactor": 1.2,
"signFailureBackoffMax": "30m",
Expand Down
8 changes: 8 additions & 0 deletions test/helpers.py
Expand Up @@ -129,6 +129,14 @@ def setup_twenty_days_ago():
for f in twenty_days_ago_functions:
f()

six_months_ago_functions = []

def register_six_months_ago(f):
six_months_ago_functions.append(f)

def setup_six_months_ago():
[f() for f in six_months_ago_functions]

def waitport(port, prog, perTickCheck=None):
"""Wait until a port on localhost is open."""
for _ in range(1000):
Expand Down
11 changes: 8 additions & 3 deletions test/integration-test.py
Expand Up @@ -239,18 +239,23 @@ def main():
raise Exception("must run at least one of the letsencrypt or chisel tests with --certbot, --chisel, or --custom")

if not args.test_case_filter:
now = datetime.datetime.utcnow()

# In CONFIG_NEXT mode, use the basic, non-next config for setup.
# This lets us test the transition to authz2.
config = default_config_dir
if CONFIG_NEXT:
config = "test/config"
now = datetime.datetime.utcnow()

six_months_ago = now+datetime.timedelta(days=-30*6)
if not startservers.start(race_detection=True, fakeclock=fakeclock(six_months_ago), config_dir=config):
raise Exception("startservers failed (mocking six months ago)")
v1_integration.caa_client = caa_client = chisel.make_client()
setup_six_months_ago()
startservers.stop()

twenty_days_ago = now+datetime.timedelta(days=-20)
if not startservers.start(race_detection=True, fakeclock=fakeclock(twenty_days_ago), config_dir=config):
raise Exception("startservers failed (mocking twenty days ago)")
v1_integration.caa_client = caa_client = chisel.make_client()
setup_twenty_days_ago()
startservers.stop()

Expand Down
32 changes: 32 additions & 0 deletions test/v2_integration.py
Expand Up @@ -1015,5 +1015,37 @@ def test_auth_deactivation_v2():
if resp.body.status is not messages.STATUS_DEACTIVATED:
raise Exception("unexpected authorization status")


expired_cert_name = ""
@register_six_months_ago
def ocsp_exp_unauth_setup():
client = chisel2.make_client(None)
order = chisel2.auth_and_issue([random_domain()], client=client)

cert = OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM, order.fullchain_pem)
cert_file_pem = os.path.join(tempdir, "to-expire.pem")
with open(cert_file_pem, "w") as f:
f.write(OpenSSL.crypto.dump_certificate(
OpenSSL.crypto.FILETYPE_PEM, cert).decode())
verify_ocsp(cert_file_pem, "test/test-ca2.pem", "http://localhost:4002", "good")
global expired_cert_name
expired_cert_name = cert_file_pem

def test_ocsp_exp_unauth():
tries = 0
while True:
try:
verify_ocsp(expired_cert_name, "test/test-ca2.pem", "http://localhost:4002", "XXX")
raise Exception("Unexpected return from verify_ocsp")
except subprocess.CalledProcessError as cpe:
if cpe.output == 'Responder Error: unauthorized (6)\n':
break
except:
pass
if tries is 5:
raise Exception("timed out waiting for unauthorized OCSP response for expired certificate")
tries += 1
time.sleep(0.25)

def run(cmd, **kwargs):
return subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT, **kwargs)

0 comments on commit db01830

Please sign in to comment.