From ba6b85ee126b50d19837a53a52cb6d894594935f Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 28 Feb 2020 19:14:48 -0800 Subject: [PATCH 1/3] Pass authzModel by value, not reference. --- sa/model.go | 4 ++-- sa/sa.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sa/model.go b/sa/model.go index ebd24941c04..9a79c744bbf 100644 --- a/sa/model.go +++ b/sa/model.go @@ -564,7 +564,7 @@ func authzPBToModel(authz *corepb.Authorization) (*authzModel, error) { // populateAttemptedFields takes a challenge and populates it with the validation fields status, // validation records, and error (the latter only if the validation failed) from a authzModel. -func populateAttemptedFields(am *authzModel, challenge *corepb.Challenge) error { +func populateAttemptedFields(am authzModel, challenge *corepb.Challenge) error { if len(am.ValidationError) != 0 { // If the error is non-empty the challenge must be invalid. status := string(core.StatusInvalid) @@ -604,7 +604,7 @@ func populateAttemptedFields(am *authzModel, challenge *corepb.Challenge) error return nil } -func modelToAuthzPB(am *authzModel) (*corepb.Authorization, error) { +func modelToAuthzPB(am authzModel) (*corepb.Authorization, error) { expires := am.Expires.UTC().UnixNano() id := fmt.Sprintf("%d", am.ID) status := uintToStatus[am.Status] diff --git a/sa/sa.go b/sa/sa.go index f44c3591014..aa9131d1a20 100644 --- a/sa/sa.go +++ b/sa/sa.go @@ -1401,7 +1401,7 @@ func (ssa *SQLStorageAuthority) GetAuthorization2(ctx context.Context, id *sapb. if obj == nil { return nil, berrors.NotFoundError("authorization %d not found", *id.Id) } - return modelToAuthzPB(obj.(*authzModel)) + return modelToAuthzPB(*(obj.(*authzModel))) } // authzModelMapToPB converts a mapping of domain name to authzModels into a @@ -1411,7 +1411,7 @@ func authzModelMapToPB(m map[string]authzModel) (*sapb.Authorizations, error) { for k, v := range m { // Make a copy of k because it will be reassigned with each loop. kCopy := k - authzPB, err := modelToAuthzPB(&v) + authzPB, err := modelToAuthzPB(v) if err != nil { return nil, err } @@ -1644,7 +1644,7 @@ func (ssa *SQLStorageAuthority) GetPendingAuthorization2(ctx context.Context, re } return nil, err } - return modelToAuthzPB(&am) + return modelToAuthzPB(am) } // CountPendingAuthorizations2 returns the number of pending, unexpired authorizations From dd5b834b76994aa4bb772842561ceed6ae46458e Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 28 Feb 2020 19:42:59 -0800 Subject: [PATCH 2/3] Add integration test. --- test/v1_integration.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/v1_integration.py b/test/v1_integration.py index 7e133750473..847403a8909 100644 --- a/test/v1_integration.py +++ b/test/v1_integration.py @@ -438,7 +438,11 @@ def caa_recheck_setup(): # Issue a certificate with the clock set back, and save the authzs to check # later that they are valid (200). They should however require rechecking for # CAA purposes. - _, authzs = auth_and_issue([random_domain()], client=caa_recheck_client) + numNames = 10 + # Generate numNames subdomains of a random domain + base_domain = random_domain() + domains = [ "{0}.{1}".format(str(n),base_domain) for n in range(numNames) ] + _, authzs = auth_and_issue(domains, client=caa_recheck_client) for a in authzs: caa_recheck_authzs.append(a) @@ -457,7 +461,9 @@ def test_recheck_caa(): response.status_code)) domain = a.body.identifier.value domains.append(domain) - challSrv.add_caa_issue(domain, ";") + + # Set a forbidding CAA record on just one domain + challSrv.add_caa_issue(domains[3], ";") # Request issuance for the previously-issued domain name, which should # now be denied due to CAA. From a179385c0d694114b9e544322e1cc3f4b027e57c Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 28 Feb 2020 19:51:34 -0800 Subject: [PATCH 3/3] Update test. --- sa/model_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sa/model_test.go b/sa/model_test.go index 56588c9c5b6..c32bc7848bb 100644 --- a/sa/model_test.go +++ b/sa/model_test.go @@ -102,7 +102,7 @@ func TestV2AuthzModel(t *testing.T) { model, err := authzPBToModel(authzPB) test.AssertNotError(t, err, "authzPBToModel failed") - authzPBOut, err := modelToAuthzPB(model) + authzPBOut, err := modelToAuthzPB(*model) test.AssertNotError(t, err, "modelToAuthzPB failed") test.AssertDeepEquals(t, authzPB.Challenges, authzPBOut.Challenges) @@ -114,7 +114,7 @@ func TestV2AuthzModel(t *testing.T) { model, err = authzPBToModel(authzPB) test.AssertNotError(t, err, "authzPBToModel failed") - authzPBOut, err = modelToAuthzPB(model) + authzPBOut, err = modelToAuthzPB(*model) test.AssertNotError(t, err, "modelToAuthzPB failed") test.AssertDeepEquals(t, authzPB.Challenges, authzPBOut.Challenges) @@ -195,7 +195,7 @@ func TestPopulateAttemptedFieldsBadJSON(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Name, func(t *testing.T) { - err := populateAttemptedFields(tc.Model, &corepb.Challenge{}) + err := populateAttemptedFields(*tc.Model, &corepb.Challenge{}) test.AssertError(t, err, "expected error from populateAttemptedFields") badJSONErr, ok := err.(errBadJSON) test.AssertEquals(t, ok, true)