Skip to content

Commit

Permalink
gRPC: allow nil ValidationRecords in FinalizeAuthz2 (letsencrypt#4270)
Browse files Browse the repository at this point in the history
The SA `FinalizeAuthorization2` RPC is used with
`FinalizeAuthorizationRequest` objects that may have a nil
`ValidationRecords` field (notably for DNS-01 challenges that
failed). The RPC wrapper should not reject such messages as incomplete.

We don't typically unit test gRPC wrappers, and adding
an integration test for this will likely conflict with
letsencrypt#4241 so I tested this
fix manually using Certbot and a local Boulder instance configured
with the authz2 feature flag.

Before applying the fix, failing a DNS-01 challenge left the
authorization stuck in pending state and Certbot would poll until
it gave up. On the server-side a 500 error matching what we observed
in staging is logged: > boulder-ra [AUDIT] Could not record updated
validation: err=[rpc error: code = Unknown desc = Incomplete gRPC
request message] regID=[xxx] authzID=[xxxx]

After applying the fix failing a DNS-01 challenge caused the associated
authorization to be marked invalid immediately. No 500 errors are
logged.

Resolves letsencrypt#4269
  • Loading branch information
Daniel McCarney authored and jsha committed Jun 19, 2019
1 parent 1fc22cf commit de8baa4
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion grpc/sa-wrappers.go
Original file line number Diff line number Diff line change
Expand Up @@ -1209,7 +1209,7 @@ func (sas StorageAuthorityServerWrapper) GetAuthorizations2(ctx context.Context,
}

func (sas StorageAuthorityServerWrapper) FinalizeAuthorization2(ctx context.Context, req *sapb.FinalizeAuthorizationRequest) (*corepb.Empty, error) {
if req == nil || req.ValidationRecords == nil || req.Status == nil || req.Attempted == nil || req.Expires == nil || req.Id == nil {
if req == nil || req.Status == nil || req.Attempted == nil || req.Expires == nil || req.Id == nil {
return nil, errIncompleteRequest
}

Expand Down

0 comments on commit de8baa4

Please sign in to comment.