letsencrypt
/
boulder
mirror of https://github.com/letsencrypt/boulder.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Harmonize order expiry with assoc. authz expiry. (#3567) 

Prior to this commit an order's expiry was set based on
ra.orderLiftime while pending and valid authorization expiry was set
based on ra.pendingAuthorizationLifetime and
ra.authorizationLifetime. Since orders reused existing valid/pending
authorizations this can lead to a case where an order has an expiry
beyond the associated authorization expiries. In this case when an
authorization expired the order becomes inactionable and the extra order
lifetime is not useful.

This commit addresses this problem in two ways:

1. The SA GetAuthorizations function used to find authzs to reuse for
ra.NewOrder is adjusted to only return authorizations at min 24hr
away from expiry.
2. Order expiry is now calculated by the RA in newOrder
as the min of the order's own expiry or the soonest
authorization expiry. This properly reflects the order's true
lifetime based on the authorization lifetime.

The RA/SA unit tests are updated accordingly.

Resolves #3498
This commit is contained in:
Daniel McCarney 2018-03-16 16:42:21 -04:00 committed by Roland Bracewell Shoemaker
parent dad6cc0095
commit 21a17f0baf
3 changed files with 199 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
ra/ra.go
View File

@ -1709,11 +1709,9 @@ func (ra *RegistrationAuthorityImpl) DeactivateAuthorization(ctx context.Context
// NewOrder creates a new order object
func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.NewOrderRequest) (*corepb.Order, error) {
expires := ra.clk.Now().Add(ra.orderLifetime).UnixNano()
order := &corepb.Order{
RegistrationID: req.RegistrationID,
Names: core.UniqueLowerNames(req.Names),
Expires: &expires,
}
// Validate that our policy allows issuing for each of the names in the order
@ -1757,15 +1755,21 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New
return nil, err
}
// Check whether there are existing non-expired authorizations for the set of
// order names
now := ra.clk.Now().UnixNano()
// An order's lifetime is effectively bound by the shortest remaining lifetime
// of its associated authorizations. For that reason it would be Uncool if
// `sa.GetAuthorizations` returned an authorization that was very close to
// expiry. The resulting pending order that references it would itself end up
// expiring very soon.
// To prevent this we only return authorizations that are at least 1 day away
// from expiring.
authzExpiryCutoff := ra.clk.Now().AddDate(0, 0, 1).UnixNano()
// We do not want any legacy V1 API authorizations not associated with an
// order to be returned from the SA so we set requireV2Authzs to true
requireV2Authzs := true
existingAuthz, err := ra.SA.GetAuthorizations(ctx, &sapb.GetAuthorizationsRequest{
RegistrationID: order.RegistrationID,
Now: &now,
Now: &authzExpiryCutoff,
Domains: order.Names,
RequireV2Authzs: &requireV2Authzs,
})
@ -1806,6 +1810,8 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New
continue
}
// Delete the authz from the nameToExistingAuthz map since we are not reusing it.
delete(nameToExistingAuthz, name)
// If we reached this point then the existing authz was not acceptable for
// reuse and we need to mark the name as requiring a new pending authz
missingAuthzNames = append(missingAuthzNames, name)
@ -1837,14 +1843,47 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New
newAuthzs = append(newAuthzs, pb)
}
// Start with the order's own expiry as the minExpiry. We only care
// about authz expiries that are sooner than the order's expiry
minExpiry := ra.clk.Now().Add(ra.orderLifetime)
// Check the reused authorizations to see if any have an expiry before the
// minExpiry (the order's lifetime)
for _, authz := range nameToExistingAuthz {
// An authz without an expiry is an unexpected internal server event
if authz.Expires == nil {
return nil, berrors.InternalServerError(
"SA.GetAuthorizations returned an authz (%d) with nil expiry",
*authz.Id)
}
// If the reused authorization expires before the minExpiry, it's expiry
// is the new minExpiry.
authzExpiry := time.Unix(0, *authz.Expires)
if authzExpiry.Before(minExpiry) {
minExpiry = authzExpiry
}
}
// If new authorizations are needed, call AddPendingAuthorizations. Also check
// whether the newly created pending authz's have an expiry lower than minExpiry
if len(newAuthzs) > 0 {
authzIDs, err := ra.SA.AddPendingAuthorizations(ctx, &sapb.AddPendingAuthorizationsRequest{Authz: newAuthzs})
if err != nil {
return nil, err
}
order.Authorizations = append(order.Authorizations, authzIDs.Ids...)
// If the newly created pending authz's have an expiry closer than the
// minExpiry the minExpiry is the pending authz expiry.
newPendingAuthzExpires := ra.clk.Now().Add(ra.pendingAuthorizationLifetime)
if newPendingAuthzExpires.Before(minExpiry) {
minExpiry = newPendingAuthzExpires
}
}
// Set the order's expiry to the minimum expiry
minExpiryTS := minExpiry.UnixNano()
order.Expires = &minExpiryTS
storedOrder, err := ra.SA.NewOrder(ctx, order)
if err != nil {
return nil, err

108
ra/ra_test.go
View File

@ -2636,6 +2636,114 @@ func TestNewOrderWildcard(t *testing.T) {
test.AssertEquals(t, dupeOrder.Authorizations[0], order.Authorizations[0])
}
// mockSANearExpiredAuthz is a mock SA that always returns an authz near expiry
// to test orders expiry calculations
type mockSANearExpiredAuthz struct {
mocks.StorageAuthority
expiry time.Time
}
// GetAuthorizations is a mock that always returns a valid authorization for
// "zombo.com" very near to expiry
func (sa *mockSANearExpiredAuthz) GetAuthorizations(
ctx context.Context,
req *sapb.GetAuthorizationsRequest) (*sapb.Authorizations, error) {
authzs := map[string]*core.Authorization{
"zombo.com": &core.Authorization{
// A static fake ID we can check for in a unit test
ID: "near-expired-authz",
Identifier: core.AcmeIdentifier{Type: "dns", Value: "zombo.com"},
RegistrationID: *req.RegistrationID,
Expires: &sa.expiry,
Status: "valid",
Challenges: []core.Challenge{
core.Challenge{
Type: core.ChallengeTypeHTTP01,
Status: core.StatusValid,
},
},
Combinations: [][]int{{0}, {1}},
},
}
// We can't easily access sa.authzMapToPB so we "inline" it for the mock :-)
resp := &sapb.Authorizations{}
for k, v := range authzs {
authzPB, err := sagrpc.AuthzToPB(*v)
if err != nil {
return nil, err
}
// Make a copy of k because it will be reassigned with each loop.
kCopy := k
resp.Authz = append(resp.Authz, &sapb.Authorizations_MapElement{Domain: &kCopy, Authz: authzPB})
}
return resp, nil
}
// AddPendingAuthorizations is a mock that just returns a fake pending authz ID
// that is != "near-expired-authz"
func (sa *mockSANearExpiredAuthz) AddPendingAuthorizations(
_ context.Context,
_ *sapb.AddPendingAuthorizationsRequest) (*sapb.AuthorizationIDs, error) {
return &sapb.AuthorizationIDs{
Ids: []string{
"abcdefg",
},
}, nil
}
func TestNewOrderExpiry(t *testing.T) {
_, _, ra, clk, cleanUp := initAuthorities(t)
defer cleanUp()
ctx := context.Background()
regA := int64(1)
names := []string{"zombo.com"}
// Set the order lifetime to 48 hours.
ra.orderLifetime = 48 * time.Hour
// Use an expiry that is sooner than the configured order expiry but greater
// than 24 hours away.
fakeAuthzExpires := clk.Now().Add(35 * time.Hour)
// Use a mock SA that always returns a soon-to-be-expired valid authz for
// "zombo.com".
ra.SA = &mockSANearExpiredAuthz{expiry: fakeAuthzExpires}
// Create an initial request with regA and names
orderReq := &rapb.NewOrderRequest{
RegistrationID: &regA,
Names: names,
}
// Create an order for that request
order, err := ra.NewOrder(ctx, orderReq)
// It shouldn't fail
test.AssertNotError(t, err, "Adding an order for regA failed")
// There should be one authorization
test.AssertEquals(t, len(order.Authorizations), 1)
// It should be the fake near-expired-authz authz
test.AssertEquals(t, order.Authorizations[0], "near-expired-authz")
// The order's expiry should be the fake authz's expiry since it is sooner
// than the order's own expiry.
test.AssertEquals(t, *order.Expires, fakeAuthzExpires.UnixNano())
// Set the order lifetime to be lower than the fakeAuthzLifetime
ra.orderLifetime = 12 * time.Hour
expectedOrderExpiry := clk.Now().Add(ra.orderLifetime).UnixNano()
// Create the order again
order, err = ra.NewOrder(ctx, orderReq)
// It shouldn't fail
test.AssertNotError(t, err, "Adding an order for regA failed")
// There should be one authorization
test.AssertEquals(t, len(order.Authorizations), 1)
// It should be the fake near-expired-authz authz
test.AssertEquals(t, order.Authorizations[0], "near-expired-authz")
// The order's expiry should be the order's own expiry since it is sooner than
// the fake authz's expiry.
test.AssertEquals(t, *order.Expires, expectedOrderExpiry)
}
func TestFinalizeOrder(t *testing.T) {
_, sa, ra, _, cleanUp := initAuthorities(t)
defer cleanUp()

69
sa/sa_test.go
View File

@ -1488,7 +1488,6 @@ func TestOrder(t *testing.T) {
})
test.AssertNotError(t, err, "Couldn't create test registration")
// Add one pending authz
authzExpires := fc.Now().Add(time.Hour)
newAuthz := core.Authorization{
Identifier: core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "example.com"},
@ -1499,7 +1498,8 @@ func TestOrder(t *testing.T) {
authz, err := sa.NewPendingAuthorization(ctx, newAuthz)
test.AssertNotError(t, err, "Couldn't create new pending authorization")
expires := time.Now().Truncate(time.Second).UnixNano()
// Set the order to expire in two hours
expires := fc.Now().Add(2 * time.Hour).UnixNano()
empty := ""
inputOrder := &corepb.Order{
@ -1509,31 +1509,37 @@ func TestOrder(t *testing.T) {
Authorizations: []string{authz.ID},
}
// Create the order
order, err := sa.NewOrder(context.Background(), inputOrder)
test.AssertNotError(t, err, "sa.NewOrder failed")
test.AssertEquals(t, *order.Id, int64(1))
pendingStatus := string(core.StatusPending)
falseBool := false
one := int64(1)
nowTS := sa.clk.Now().UnixNano()
// The Order from GetOrder should match the following expected order
expectedOrder := &corepb.Order{
// The expected order matches the input order
// The registration ID, authorizations, expiry, and names should match the
// input to NewOrder
RegistrationID: inputOrder.RegistrationID,
Expires: inputOrder.Expires,
Names: inputOrder.Names,
Authorizations: inputOrder.Authorizations,
// And should also have an empty serial and the correct status and
// processing state, and an ID of 1, and a created timestamp
Id: &one,
Names: inputOrder.Names,
Expires: inputOrder.Expires,
// The ID should have been set to 1 by the SA
Id: &one,
// The status should be pending
Status: &pendingStatus,
// The serial should be empty since this is a pending order
CertificateSerial: &empty,
BeganProcessing: &falseBool,
Status: &pendingStatus,
Created: &nowTS,
// We should not be processing it
BeganProcessing: &falseBool,
// The created timestamp should have been set to the current time
Created: &nowTS,
}
// Fetch the order by its ID and make sure it matches the expected
storedOrder, err := sa.GetOrder(context.Background(), &sapb.OrderRequest{Id: order.Id})
test.AssertNotError(t, err, "sa.Order failed")
test.AssertNotError(t, err, "sa.GetOrder failed")
test.AssertDeepEquals(t, storedOrder, expectedOrder)
}
@ -1623,11 +1629,13 @@ func TestGetAuthorizations(t *testing.T) {
defer cleanup()
reg := satest.CreateWorkingRegistration(t, sa)
exp := fc.Now().AddDate(0, 0, 1)
exp := fc.Now().AddDate(0, 0, 10)
identA := "aaa"
identB := "bbb"
idents := []string{identA, identB}
identC := "ccc"
identD := "ddd"
idents := []string{identA, identB, identC}
// Create an authorization template for a pending authorization with a dummy identifier
pa := core.Authorization{
@ -1655,33 +1663,48 @@ func TestGetAuthorizations(t *testing.T) {
err = sa.FinalizeAuthorization(ctx, paB)
test.AssertNotError(t, err, "Couldn't finalize pending authorization with ID "+paB.ID)
// Adjust the template to have an expiry in 1 hour from now.
nearbyExpires := fc.Now().Add(time.Hour)
pa.Expires = &nearbyExpires
pa.Identifier.Value = identC
// Add the template to create pending authorization C
paC, err := sa.NewPendingAuthorization(ctx, pa)
// There should be no error
test.AssertNotError(t, err, "Couldn't create new pending authorization")
test.Assert(t, paC.ID != "", "ID shouldn't be blank")
// Don't require V2 authorizations because the above pending authorizations
// aren't associated with orders, and therefore are seen as legacy V1
// authorizations.
requireV2Authzs := false
now := fc.Now().UnixNano()
// Set an expiry cut off of 1 day in the future similar to `RA.NewOrder`. This
// should exclude pending authorization C based on its nearbyExpires expiry
// value.
expiryCutoff := fc.Now().AddDate(0, 0, 1).UnixNano()
// Get authorizations for the names used above.
authz, err := sa.GetAuthorizations(context.Background(), &sapb.GetAuthorizationsRequest{
RegistrationID: &reg.ID,
Domains: idents,
Now: &now,
Now: &expiryCutoff,
RequireV2Authzs: &requireV2Authzs,
})
// It should not fail
test.AssertNotError(t, err, "sa.GetAuthorizations failed")
// We should get back two authorizations
// We should get back two authorizations since one of the three authorizations
// created above expires too soon.
test.AssertEquals(t, len(authz.Authz), 2)
// Get authorizations for the names used above, and one name that doesn't exist
authz, err = sa.GetAuthorizations(context.Background(), &sapb.GetAuthorizationsRequest{
RegistrationID: &reg.ID,
Domains: append(idents, "ccc"),
Now: &now,
Domains: append(idents, identD),
Now: &expiryCutoff,
RequireV2Authzs: &requireV2Authzs,
})
// It should not fail
test.AssertNotError(t, err, "sa.GetAuthorizations failed")
// It should return two authorizations
// It should still return only two authorizations
test.AssertEquals(t, len(authz.Authz), 2)
// Get authorizations for the names used above, but this time enforce that no
@ -1690,7 +1713,7 @@ func TestGetAuthorizations(t *testing.T) {
authz, err = sa.GetAuthorizations(context.Background(), &sapb.GetAuthorizationsRequest{
RegistrationID: &reg.ID,
Domains: idents,
Now: &now,
Now: &expiryCutoff,
RequireV2Authzs: &requireV2Authzs,
})
// It should not fail
@ -1714,7 +1737,7 @@ func TestGetAuthorizations(t *testing.T) {
authz, err = sa.GetAuthorizations(context.Background(), &sapb.GetAuthorizationsRequest{
RegistrationID: &reg.ID,
Domains: idents,
Now: &now,
Now: &expiryCutoff,
RequireV2Authzs: &requireV2Authzs,
})
// It should not fail