From 400ffede3d73c7d10353d730de4ef4b7fe415f0f Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Tue, 9 Jan 2018 20:48:16 -0800 Subject: [PATCH] More fixes --- csr/csr_test.go | 1 + ra/ra.go | 10 ++--- ra/ra_test.go | 10 ++--- sa/sa.go | 97 ++++++++++++++----------------------------------- 4 files changed, 39 insertions(+), 79 deletions(-) diff --git a/csr/csr_test.go b/csr/csr_test.go index d710af254..9fc0be6c0 100644 --- a/csr/csr_test.go +++ b/csr/csr_test.go @@ -37,6 +37,7 @@ func (pa *mockPA) WillingToIssue(id core.AcmeIdentifier) error { func (pa *mockPA) WillingToIssueWildcard(id core.AcmeIdentifier) error { return nil } + func (pa *mockPA) ChallengeTypeEnabled(t string) bool { return true } diff --git a/ra/ra.go b/ra/ra.go index e8bec749b..c8658e3c8 100644 --- a/ra/ra.go +++ b/ra/ra.go @@ -532,7 +532,7 @@ func (ra *RegistrationAuthorityImpl) NewAuthorization(ctx context.Context, reque ra.log.Warning(fmt.Sprintf("%s: %s", outErr.Error(), existingAuthz.ID)) return core.Authorization{}, outErr } - if !features.Enabled(features.EnforceChallengeDisable) || ra.validChallengeStillGood(&populatedAuthz) { + if !features.Enabled(features.EnforceChallengeDisable) || ra.authzValidChallengeEnabled(&populatedAuthz) { // The existing authorization must not expire within the next 24 hours for // it to be OK for reuse reuseCutOff := ra.clk.Now().Add(time.Hour * 24) @@ -722,8 +722,8 @@ func (ra *RegistrationAuthorityImpl) checkAuthorizationsCAA( // Ensure that CAA is rechecked for this name recheckNames = append(recheckNames, name) } - if authz != nil && features.Enabled(features.EnforceChallengeDisable) && !ra.validChallengeStillGood(authz) { - return berrors.UnauthorizedError("challenge used to validate authorization with ID %q no longer allowed", authz.ID) + if authz != nil && features.Enabled(features.EnforceChallengeDisable) && !ra.authzValidChallengeEnabled(authz) { + return berrors.UnauthorizedError("challenge used to validate authorization with ID %q forbidden by policy", authz.ID) } } @@ -1757,9 +1757,9 @@ func (ra *RegistrationAuthorityImpl) createPendingAuthz(ctx context.Context, reg return authz, nil } -// validChallengeStillGood checks whether the valid challenge in an authorization uses a type +// authzValidChallengeEnabled checks whether the valid challenge in an authorization uses a type // which is still enabled -func (ra *RegistrationAuthorityImpl) validChallengeStillGood(authz *core.Authorization) bool { +func (ra *RegistrationAuthorityImpl) authzValidChallengeEnabled(authz *core.Authorization) bool { for _, chall := range authz.Challenges { if chall.Status == core.StatusValid { fmt.Println("TYPE", chall.Type) diff --git a/ra/ra_test.go b/ra/ra_test.go index 8cd1a8ced..ac19e5e2c 100644 --- a/ra/ra_test.go +++ b/ra/ra_test.go @@ -2914,7 +2914,7 @@ func TestDisabledChallengeValidAuthz(t *testing.T) { 0, fc.Now(), ) - test.AssertError(t, err, "RA didn't prevent use of an authorization which used an disabled challenge type") + test.AssertError(t, err, "RA didn't prevent use of an authorization which used a disabled challenge type") err = ra.checkAuthorizationsCAA( context.Background(), @@ -2942,11 +2942,11 @@ func TestValidChallengeStillGood(t *testing.T) { _ = features.Set(map[string]bool{"EnforceChallengeDisable": true}) - test.Assert(t, !ra.validChallengeStillGood(&core.Authorization{}), "ra.validChallengeStillGood didn't fail with empty authorization") - test.Assert(t, !ra.validChallengeStillGood(&core.Authorization{Challenges: []core.Challenge{{Status: core.StatusPending}}}), "ra.validChallengeStillGood didn't fail with no valid challenges") - test.Assert(t, !ra.validChallengeStillGood(&core.Authorization{Challenges: []core.Challenge{{Status: core.StatusValid, Type: core.ChallengeTypeHTTP01}}}), "ra.validChallengeStillGood didn't fail with disabled challenge") + test.Assert(t, !ra.authzValidChallengeEnabled(&core.Authorization{}), "ra.authzValidChallengeEnabled didn't fail with empty authorization") + test.Assert(t, !ra.authzValidChallengeEnabled(&core.Authorization{Challenges: []core.Challenge{{Status: core.StatusPending}}}), "ra.authzValidChallengeEnabled didn't fail with no valid challenges") + test.Assert(t, !ra.authzValidChallengeEnabled(&core.Authorization{Challenges: []core.Challenge{{Status: core.StatusValid, Type: core.ChallengeTypeHTTP01}}}), "ra.authzValidChallengeEnabled didn't fail with disabled challenge") - test.Assert(t, ra.validChallengeStillGood(&core.Authorization{Challenges: []core.Challenge{{Status: core.StatusValid, Type: core.ChallengeTypeTLSSNI01}}}), "ra.validChallengeStillGood failed with enabled challenge") + test.Assert(t, ra.authzValidChallengeEnabled(&core.Authorization{Challenges: []core.Challenge{{Status: core.StatusValid, Type: core.ChallengeTypeTLSSNI01}}}), "ra.authzValidChallengeEnabled failed with enabled challenge") } func TestUpdateAuthorizationBadChallengeType(t *testing.T) { diff --git a/sa/sa.go b/sa/sa.go index 39aca336b..2ebf7fbed 100644 --- a/sa/sa.go +++ b/sa/sa.go @@ -222,24 +222,10 @@ func (ssa *SQLStorageAuthority) GetAuthorization(ctx context.Context, id string) authz = pa.Authorization } - var challObjs []challModel - _, err = tx.Select( - &challObjs, - getChallengesQuery, - map[string]interface{}{"authID": authz.ID}, - ) + authz.Challenges, err = ssa.getChallenges(authz.ID) if err != nil { - return authz, Rollback(tx, err) + return authz, err } - var challs []core.Challenge - for _, c := range challObjs { - chall, err := modelToChallenge(&c) - if err != nil { - return authz, Rollback(tx, err) - } - challs = append(challs, chall) - } - authz.Challenges = challs return authz, tx.Commit() } @@ -1545,26 +1531,11 @@ func (ssa *SQLStorageAuthority) GetOrderAuthorizations( } existing, present := byName[auth.Identifier.Value] if !present || auth.Expires.After(*existing.Expires) { - - // Retrieve challenges for the authzvar challObjs []challModel - var challObjs []challModel - _, err = ssa.dbMap.Select( - &challObjs, - getChallengesQuery, - map[string]interface{}{"authID": auth.ID}, - ) + // Retrieve challenges for the authz + auth.Challenges, err = ssa.getChallenges(auth.ID) if err != nil { return nil, err } - var challs []core.Challenge - for _, c := range challObjs { - chall, err := modelToChallenge(&c) - if err != nil { - return nil, err - } - challs = append(challs, chall) - } - auth.Challenges = challs byName[auth.Identifier.Value] = auth } @@ -1648,25 +1619,8 @@ func (ssa *SQLStorageAuthority) getAuthorizations(ctx context.Context, table str } existing, present := byName[auth.Identifier.Value] if !present || auth.Expires.After(*existing.Expires) { - // Retrieve challenges for the authzvar challObjs []challModel - var challObjs []challModel - _, err = ssa.dbMap.Select( - &challObjs, - getChallengesQuery, - map[string]interface{}{"authID": auth.ID}, - ) - if err != nil { - return nil, err - } - var challs []core.Challenge - for _, c := range challObjs { - chall, err := modelToChallenge(&c) - if err != nil { - return nil, err - } - challs = append(challs, chall) - } - auth.Challenges = challs + // Retrieve challenges for the authz + auth.Challenges, err = ssa.getChallenges(auth.ID) byName[auth.Identifier.Value] = auth } @@ -1734,23 +1688,7 @@ func (ssa *SQLStorageAuthority) GetAuthorizations(ctx context.Context, req *sapb if features.Enabled(features.WildcardDomains) { // Fetch each of the authorizations' associated challenges for _, authz := range authzMap { - var challObjs []challModel - _, err = ssa.dbMap.Select( - &challObjs, - getChallengesQuery, - map[string]interface{}{"authID": authz.ID}, - ) - if err != nil { - return nil, err - } - authz.Challenges = make([]core.Challenge, len(challObjs)) - for i, c := range challObjs { - chall, err := modelToChallenge(&c) - if err != nil { - return nil, err - } - authz.Challenges[i] = chall - } + authz.Challenges, err = ssa.getChallenges(authz.ID) } } return authzMapToPB(authzMap) @@ -1772,3 +1710,24 @@ func (ssa *SQLStorageAuthority) AddPendingAuthorizations(ctx context.Context, re } return &sapb.AuthorizationIDs{Ids: ids}, nil } + +func (ssa *SQLStorageAuthority) getChallenges(authID string) ([]core.Challenge, error) { + var challObjs []challModel + _, err := ssa.dbMap.Select( + &challObjs, + getChallengesQuery, + map[string]interface{}{"authID": authID}, + ) + if err != nil { + return nil, err + } + var challs []core.Challenge + for _, c := range challObjs { + chall, err := modelToChallenge(&c) + if err != nil { + return nil, err + } + challs = append(challs, chall) + } + return challs, nil +}