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

Make ra.PerformValidation resilient to va failure (#5028) 

ra.PerformValidation's goroutine surfaces errors not by returning them,
but by accumulating them into the `prob`variable and saving them to
the database. This makes it possible for processing to continue even
in error cases when it should (mostly) halt. This change fixes a bug
where we would try to access a member of the result returned from
va.PerformValidation, even if that function call had returned an error.
This commit is contained in:
Aaron Gable 2020-08-17 12:29:33 -07:00 committed by GitHub
parent 71478020c4
commit 32d56ae1e6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 71 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
ra/ra.go
View File

@ -1622,28 +1622,31 @@ func (ra *RegistrationAuthorityImpl) PerformValidation(
}
res, err := ra.VA.PerformValidation(vaCtx, &req)
challenge := &authz.Challenges[challIndex]
var prob *probs.ProblemDetails
if err != nil {
prob = probs.ServerInternal("Could not communicate with VA")
ra.log.AuditErrf("Could not communicate with VA: %s", err)
} else if res.Problems != nil {
prob, err = bgrpc.PBToProblemDetails(res.Problems)
if err != nil {
prob = probs.ServerInternal("Could not communicate with VA")
ra.log.AuditErrf("Could not communicate with VA: %s", err)
} else {
if res.Problems != nil {
prob, err = bgrpc.PBToProblemDetails(res.Problems)
if err != nil {
prob = probs.ServerInternal("Could not communicate with VA")
ra.log.AuditErrf("Could not communicate with VA: %s", err)
}
}
}
// Save the updated records
challenge := &authz.Challenges[challIndex]
records := make([]core.ValidationRecord, len(res.Records))
for i, r := range res.Records {
records[i], err = bgrpc.PBToValidationRecord(r)
if err != nil {
prob = probs.ServerInternal("Records for validation corrupt")
// Save the updated records
records := make([]core.ValidationRecord, len(res.Records))
for i, r := range res.Records {
records[i], err = bgrpc.PBToValidationRecord(r)
if err != nil {
prob = probs.ServerInternal("Records for validation corrupt")
}
}
challenge.ValidationRecord = records
}
challenge.ValidationRecord = records
if !challenge.RecordsSane() && prob == nil {
prob = probs.ServerInternal("Records for validation failed sanity check")

58
ra/ra_test.go
View File

@ -135,12 +135,13 @@ func numAuthorizations(o *corepb.Order) int {
type DummyValidationAuthority struct {
request chan *vapb.PerformValidationRequest
ResultReturn vapb.ValidationResult
ResultError error
ResultReturn *vapb.ValidationResult
}
func (dva *DummyValidationAuthority) PerformValidation(ctx context.Context, req *vapb.PerformValidationRequest, _ ...grpc.CallOption) (*vapb.ValidationResult, error) {
dva.request <- req
return &dva.ResultReturn, nil
return dva.ResultReturn, dva.ResultError
}
var (
@ -911,7 +912,7 @@ func TestPerformValidationAlreadyValid(t *testing.T) {
fakeHostname := "example.com"
fakePort := "8080"
va.ResultReturn = vapb.ValidationResult{
va.ResultReturn = &vapb.ValidationResult{
Records: []*corepb.ValidationRecord{
{
AddressUsed: []byte("192.168.0.1"),
@ -945,7 +946,7 @@ func TestPerformValidationSuccess(t *testing.T) {
fakeHostname := "example.com"
fakePort := "8080"
va.ResultReturn = vapb.ValidationResult{
va.ResultReturn = &vapb.ValidationResult{
Records: []*corepb.ValidationRecord{
{
AddressUsed: []byte("192.168.0.1"),
@ -989,6 +990,7 @@ func TestPerformValidationSuccess(t *testing.T) {
// Verify that the responses are reflected
test.AssertNotNil(t, vaRequest.Challenge, "Request passed to VA has no challenge")
challIdx = challTypeIndex(t, dbAuthz.Challenges, core.ChallengeTypeDNS01)
fmt.Println(dbAuthz.Challenges[challIdx])
test.Assert(t, dbAuthz.Challenges[challIdx].Status == core.StatusValid, "challenge was not marked as valid")
// The DB authz's expiry should be equal to the current time plus the
@ -997,6 +999,54 @@ func TestPerformValidationSuccess(t *testing.T) {
test.AssertEquals(t, *dbAuthz.Expires, expectedExpires)
}
func TestPerformValidationVAError(t *testing.T) {
va, sa, ra, _, cleanUp := initAuthorities(t)
defer cleanUp()
authz, err := ra.NewAuthorization(ctx, AuthzRequest, Registration.ID)
test.AssertNotError(t, err, "NewAuthorization failed")
challIdx := challTypeIndex(t, authz.Challenges, core.ChallengeTypeDNS01)
va.ResultError = fmt.Errorf("Something went wrong")
authzPB, err := bgrpc.AuthzToPB(authz)
test.AssertNotError(t, err, "AuthzToPB failed")
authzPB, err = ra.PerformValidation(ctx, &rapb.PerformValidationRequest{
Authz: authzPB,
ChallengeIndex: &challIdx,
})
test.AssertNotError(t, err, "PerformValidation completely failed")
authz, err = bgrpc.PBToAuthz(authzPB)
test.AssertNotError(t, err, "PBToAuthz failed")
var vaRequest *vapb.PerformValidationRequest
select {
case r := <-va.request:
vaRequest = r
case <-time.After(time.Second):
t.Fatal("Timed out waiting for DummyValidationAuthority.PerformValidation to complete")
}
// Verify that the VA got the request, and it's the same as the others
test.AssertEquals(t, string(authz.Challenges[challIdx].Type), *vaRequest.Challenge.Type)
test.AssertEquals(t, authz.Challenges[challIdx].Token, *vaRequest.Challenge.Token)
// Sleep so the RA has a chance to write to the SA
time.Sleep(100 * time.Millisecond)
dbAuthz := getAuthorization(t, authz.ID, sa)
t.Log("dbAuthz:", dbAuthz)
// Verify that the responses are reflected
challIdx = challTypeIndex(t, dbAuthz.Challenges, core.ChallengeTypeDNS01)
test.Assert(t, dbAuthz.Challenges[challIdx].Status == core.StatusInvalid, "challenge was not marked as invalid")
test.AssertContains(t, dbAuthz.Challenges[challIdx].Error.Error(), "Could not communicate with VA")
test.Assert(t, dbAuthz.Challenges[challIdx].ValidationRecord == nil, "challenge had a ValidationRecord")
}
func TestCertificateKeyNotEqualAccountKey(t *testing.T) {
_, sa, ra, _, cleanUp := initAuthorities(t)
defer cleanUp()