Skip to content

Commit 699c7e4

Browse files
jshacpu
jsha
authored and
cpu
committed
Add a DNS problem type. (letsencrypt#3625)
As specified in ACME. Also, include problem type in the stats. Fixes letsencrypt#3613.
1 parent 4951153 commit 699c7e4

File tree

5 files changed

+35
-19
lines changed

5 files changed

+35
-19
lines changed

‎probs/probs.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ const (
1919
RejectedIdentifierProblem = ProblemType("rejectedIdentifier")
2020
AccountDoesNotExistProblem = ProblemType("accountDoesNotExist")
2121
CAAProblem = ProblemType("caa")
22+
DNSProblem = ProblemType("dns")
2223

2324
V1ErrorNS = "urn:acme:error:"
2425
V2ErrorNS = "urn:ietf:params:acme:error:"
@@ -244,3 +245,12 @@ func CAA(detail string) *ProblemDetails {
244245
HTTPStatus: http.StatusForbidden,
245246
}
246247
}
248+
249+
// DNS returns a ProblemDetails representing a DNSProblem
250+
func DNS(detail string) *ProblemDetails {
251+
return &ProblemDetails{
252+
Type: DNSProblem,
253+
Detail: detail,
254+
HTTPStatus: http.StatusBadRequest,
255+
}
256+
}

‎va/caa.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func (va *ValidationAuthorityImpl) checkCAA(
4242
identifier core.AcmeIdentifier) *probs.ProblemDetails {
4343
present, valid, err := va.checkCAARecords(ctx, identifier)
4444
if err != nil {
45-
return probs.ConnectionFailure(err.Error())
45+
return probs.DNS(err.Error())
4646
}
4747
va.log.AuditInfo(fmt.Sprintf(
4848
"Checked CAA records for %s, [Present: %t, Valid for issuance: %t]",

‎va/caa_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,8 @@ func TestCAATimeout(t *testing.T) {
134134
va, _ := setup(nil, 0)
135135
va.dnsClient = caaMockDNS{}
136136
err := va.checkCAA(ctx, core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "caa-timeout.com"})
137-
if err.Type != probs.ConnectionProblem {
138-
t.Errorf("Expected timeout error type %s, got %s", probs.ConnectionProblem, err.Type)
137+
if err.Type != probs.DNSProblem {
138+
t.Errorf("Expected timeout error type %s, got %s", probs.DNSProblem, err.Type)
139139
}
140140
expected := "error"
141141
if err.Detail != expected {

‎va/va.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func initMetrics(stats metrics.Scope) *vaMetrics {
7070
Help: "Time taken to validate a challenge",
7171
Buckets: metrics.InternetFacingBuckets,
7272
},
73-
[]string{"type", "result"})
73+
[]string{"type", "result", "problemType"})
7474
stats.MustRegister(validationTime)
7575
remoteValidationTime := prometheus.NewHistogramVec(
7676
prometheus.HistogramOpts{
@@ -162,8 +162,7 @@ type verificationRequestEvent struct {
162162
func (va ValidationAuthorityImpl) getAddr(ctx context.Context, hostname string) (net.IP, []net.IP, *probs.ProblemDetails) {
163163
addrs, err := va.dnsClient.LookupHost(ctx, hostname)
164164
if err != nil {
165-
va.log.Debug(fmt.Sprintf("%s DNS failure: %s", hostname, err))
166-
problem := probs.ConnectionFailure(err.Error())
165+
problem := probs.DNS(err.Error())
167166
return net.IP{}, nil, problem
168167
}
169168

@@ -683,7 +682,7 @@ func (va *ValidationAuthorityImpl) validateDNS01(ctx context.Context, identifier
683682
if err != nil {
684683
va.log.Info(fmt.Sprintf("Failed to lookup TXT records for %s. err=[%#v] errStr=[%s]", identifier, err, err))
685684

686-
return nil, probs.ConnectionFailure(err.Error())
685+
return nil, probs.DNS(err.Error())
687686
}
688687

689688
// If there weren't any TXT records return a distinct error message to allow
@@ -872,7 +871,9 @@ func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, domain
872871
prob = probs.ServerInternal("Records for validation failed sanity check")
873872
}
874873

874+
var problemType string
875875
if prob != nil {
876+
problemType = string(prob.Type)
876877
challenge.Status = core.StatusInvalid
877878
challenge.Error = prob
878879
logEvent.Error = prob.Error()
@@ -896,8 +897,9 @@ func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, domain
896897
logEvent.Challenge = challenge
897898

898899
va.metrics.validationTime.With(prometheus.Labels{
899-
"type": string(challenge.Type),
900-
"result": string(challenge.Status),
900+
"type": string(challenge.Type),
901+
"result": string(challenge.Status),
902+
"problemType": problemType,
901903
}).Observe(time.Since(vStart).Seconds())
902904

903905
va.log.AuditObject("Validation result", logEvent)

‎va/va_test.go‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -772,8 +772,9 @@ func TestPerformValidationInvalid(t *testing.T) {
772772
test.Assert(t, prob != nil, "validation succeeded")
773773

774774
samples := test.CountHistogramSamples(va.metrics.validationTime.With(prometheus.Labels{
775-
"type": "dns-01",
776-
"result": "invalid",
775+
"type": "dns-01",
776+
"result": "invalid",
777+
"problemType": "unauthorized",
777778
}))
778779
if samples != 1 {
779780
t.Errorf("Wrong number of samples for invalid validation. Expected 1, got %d", samples)
@@ -792,8 +793,9 @@ func TestDNSValidationEmpty(t *testing.T) {
792793
test.AssertEquals(t, prob.Error(), "unauthorized :: No TXT record found at _acme-challenge.empty-txts.com")
793794

794795
samples := test.CountHistogramSamples(va.metrics.validationTime.With(prometheus.Labels{
795-
"type": "dns-01",
796-
"result": "invalid",
796+
"type": "dns-01",
797+
"result": "invalid",
798+
"problemType": "unauthorized",
797799
}))
798800
if samples != 1 {
799801
t.Errorf("Wrong number of samples for invalid validation. Expected 1, got %d", samples)
@@ -856,8 +858,9 @@ func TestPerformValidationValid(t *testing.T) {
856858
test.Assert(t, prob == nil, fmt.Sprintf("validation failed: %#v", prob))
857859

858860
samples := test.CountHistogramSamples(va.metrics.validationTime.With(prometheus.Labels{
859-
"type": "dns-01",
860-
"result": "valid",
861+
"type": "dns-01",
862+
"result": "valid",
863+
"problemType": "",
861864
}))
862865
if samples != 1 {
863866
t.Errorf("Wrong number of samples for successful validation. Expected 1, got %d", samples)
@@ -885,8 +888,9 @@ func TestPerformValidationWildcard(t *testing.T) {
885888
test.Assert(t, prob == nil, fmt.Sprintf("validation failed: %#v", prob))
886889

887890
samples := test.CountHistogramSamples(va.metrics.validationTime.With(prometheus.Labels{
888-
"type": "dns-01",
889-
"result": "valid",
891+
"type": "dns-01",
892+
"result": "valid",
893+
"problemType": "",
890894
}))
891895
if samples != 1 {
892896
t.Errorf("Wrong number of samples for successful validation. Expected 1, got %d", samples)
@@ -971,7 +975,7 @@ func TestDNSValidationServFail(t *testing.T) {
971975

972976
_, prob := va.validateChallenge(ctx, dnsi("servfail.com"), chalDNS)
973977

974-
test.AssertEquals(t, prob.Type, probs.ConnectionProblem)
978+
test.AssertEquals(t, prob.Type, probs.DNSProblem)
975979
}
976980

977981
func TestDNSValidationNoServer(t *testing.T) {
@@ -987,7 +991,7 @@ func TestDNSValidationNoServer(t *testing.T) {
987991

988992
_, prob := va.validateChallenge(ctx, dnsi("localhost"), chalDNS)
989993

990-
test.AssertEquals(t, prob.Type, probs.ConnectionProblem)
994+
test.AssertEquals(t, prob.Type, probs.DNSProblem)
991995
}
992996

993997
func TestDNSValidationOK(t *testing.T) {

0 commit comments

Comments
 (0)