RA: Remove email DNS validations. (#3851)
Performing DNS lookups to check the A/AAAA/MX records of a provided contact e-mail address adds variability to the RA's NewRegistration/UpdateRegistration functions and requires that the RA be able to reach out to the EFN. Since this is simply a convenience to prevent some classes of registration errors we can remove it to improve performance and to tighten up our security posture slightly. Resolves https://github.com/letsencrypt/boulder/issues/3849
This commit is contained in:
Daniel McCarney
Roland Bracewell Shoemaker
parent
a64928bc3d
commit
db01b0b5bc
129
ra/ra.go
129
ra/ra.go
|
|
@ -11,7 +11,6 @@ import (
|
|||
"reflect"
|
||||
"sort"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/jmhodges/clock"
|
||||
|
|
@ -161,80 +160,6 @@ func (ra *RegistrationAuthorityImpl) rateLimitPoliciesLoadError(err error) {
|
|||
ra.log.Errf("error reloading rate limit policy: %s", err)
|
||||
}
|
||||
|
||||
var (
|
||||
unparseableEmailError = berrors.InvalidEmailError("not a valid e-mail address")
|
||||
emptyDNSResponseError = berrors.InvalidEmailError(
|
||||
"empty DNS response validating email domain - no MX/A records")
|
||||
multipleAddressError = berrors.InvalidEmailError("more than one e-mail address")
|
||||
)
|
||||
|
||||
func problemIsTimeout(err error) bool {
|
||||
if dnsErr, ok := err.(*bdns.DNSError); ok && dnsErr.Timeout() {
|
||||
return true
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
// forbiddenMailDomains is a map of domain names we do not allow after the
|
||||
// @ symbol in contact mailto addresses. These are frequently used when
|
||||
// copy-pasting example configurations and would not result in expiration
|
||||
// messages and subscriber communications reaching the user that created the
|
||||
// registration if allowed.
|
||||
var forbiddenMailDomains = map[string]bool{
|
||||
// https://tools.ietf.org/html/rfc2606#section-3
|
||||
"example.com": true,
|
||||
"example.net": true,
|
||||
"example.org": true,
|
||||
}
|
||||
|
||||
func validateEmail(ctx context.Context, address string, resolver bdns.DNSClient) error {
|
||||
email, err := mail.ParseAddress(address)
|
||||
if err != nil {
|
||||
return unparseableEmailError
|
||||
}
|
||||
splitEmail := strings.SplitN(email.Address, "@", -1)
|
||||
domain := strings.ToLower(splitEmail[len(splitEmail)-1])
|
||||
if forbiddenMailDomains[domain] {
|
||||
return berrors.InvalidEmailError(
|
||||
"invalid contact domain. Contact emails @%s are forbidden",
|
||||
domain)
|
||||
}
|
||||
var resultMX []string
|
||||
var resultA []net.IP
|
||||
var errMX, errA error
|
||||
var wg sync.WaitGroup
|
||||
wg.Add(2)
|
||||
go func() {
|
||||
resultMX, errMX = resolver.LookupMX(ctx, domain)
|
||||
wg.Done()
|
||||
}()
|
||||
go func() {
|
||||
resultA, errA = resolver.LookupHost(ctx, domain)
|
||||
wg.Done()
|
||||
}()
|
||||
wg.Wait()
|
||||
|
||||
// We treat timeouts as non-failures for best-effort email validation
|
||||
// See: https://github.com/letsencrypt/boulder/issues/2260
|
||||
if problemIsTimeout(errMX) || problemIsTimeout(errA) {
|
||||
return nil
|
||||
}
|
||||
|
||||
if errMX != nil {
|
||||
return berrors.InvalidEmailError(errMX.Error())
|
||||
} else if len(resultMX) > 0 {
|
||||
return nil
|
||||
}
|
||||
if errA != nil {
|
||||
return berrors.InvalidEmailError(errA.Error())
|
||||
} else if len(resultA) > 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
return emptyDNSResponseError
|
||||
}
|
||||
|
||||
// certificateRequestAuthz is a struct for holding information about a valid
|
||||
// authz referenced during a certificateRequestEvent. It holds both the
|
||||
// authorization ID and the challenge type that made the authorization valid. We
|
||||
|
|
@ -384,6 +309,15 @@ func (ra *RegistrationAuthorityImpl) NewRegistration(ctx context.Context, init c
|
|||
return reg, nil
|
||||
}
|
||||
|
||||
// validateContacts checks the provided list of contacts, returning an error if
|
||||
// any are not acceptable. Unacceptable contacts lists include:
|
||||
// * An empty list
|
||||
// * A list has more than maxContactsPerReg contacts
|
||||
// * A list containing an empty contact
|
||||
// * A list containing a contact that does not parse as a URL
|
||||
// * A list containing a contact that has a URL scheme other than mailto
|
||||
// * A list containing a contact that has non-ascii characters
|
||||
// * A list containing a contact that doesn't pass `validateEmail`
|
||||
func (ra *RegistrationAuthorityImpl) validateContacts(ctx context.Context, contacts *[]string) error {
|
||||
if contacts == nil || len(*contacts) == 0 {
|
||||
return nil // Nothing to validate
|
||||
|
|
@ -413,21 +347,50 @@ func (ra *RegistrationAuthorityImpl) validateContacts(ctx context.Context, conta
|
|||
contact,
|
||||
)
|
||||
}
|
||||
|
||||
start := ra.clk.Now()
|
||||
ra.stats.Inc("ValidateEmail.Calls", 1)
|
||||
err = validateEmail(ctx, parsed.Opaque, ra.DNSClient)
|
||||
ra.stats.TimingDuration("ValidateEmail.Latency", ra.clk.Now().Sub(start))
|
||||
if err != nil {
|
||||
ra.stats.Inc("ValidateEmail.Errors", 1)
|
||||
if err := validateEmail(parsed.Opaque); err != nil {
|
||||
return err
|
||||
}
|
||||
ra.stats.Inc("ValidateEmail.Successes", 1)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
var (
|
||||
// unparseableEmailError is returned by validateEmail when the given address
|
||||
// is not parseable.
|
||||
unparseableEmailError = berrors.InvalidEmailError("not a valid e-mail address")
|
||||
)
|
||||
|
||||
// forbiddenMailDomains is a map of domain names we do not allow after the
|
||||
// @ symbol in contact mailto addresses. These are frequently used when
|
||||
// copy-pasting example configurations and would not result in expiration
|
||||
// messages and subscriber communications reaching the user that created the
|
||||
// registration if allowed.
|
||||
var forbiddenMailDomains = map[string]bool{
|
||||
// https://tools.ietf.org/html/rfc2606#section-3
|
||||
"example.com": true,
|
||||
"example.net": true,
|
||||
"example.org": true,
|
||||
}
|
||||
|
||||
// validateEmail returns an error if the given address is not parseable as an
|
||||
// email address or if the domain portion of the email address is a member of
|
||||
// the forbiddenMailDomains map.
|
||||
func validateEmail(address string) error {
|
||||
email, err := mail.ParseAddress(address)
|
||||
if err != nil {
|
||||
return unparseableEmailError
|
||||
}
|
||||
splitEmail := strings.SplitN(email.Address, "@", -1)
|
||||
domain := strings.ToLower(splitEmail[len(splitEmail)-1])
|
||||
if forbiddenMailDomains[domain] {
|
||||
return berrors.InvalidEmailError(
|
||||
"invalid contact domain. Contact emails @%s are forbidden",
|
||||
domain)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (ra *RegistrationAuthorityImpl) checkPendingAuthorizationLimit(ctx context.Context, regID int64) error {
|
||||
limit := ra.rlPolicies.PendingAuthorizationsPerAccount()
|
||||
if limit.Enabled() {
|
||||
|
|
|
|||
|
|
@ -330,6 +330,8 @@ func TestValidateContacts(t *testing.T) {
|
|||
otherValidEmail := "mailto:other-admin@email.com"
|
||||
malformedEmail := "mailto:admin.com"
|
||||
nonASCII := "mailto:señor@email.com"
|
||||
unparseable := "mailto:a@email.com, b@email.com"
|
||||
forbidden := "mailto:a@example.org"
|
||||
|
||||
err := ra.validateContacts(context.Background(), &[]string{})
|
||||
test.AssertNotError(t, err, "No Contacts")
|
||||
|
|
@ -351,49 +353,12 @@ func TestValidateContacts(t *testing.T) {
|
|||
|
||||
err = ra.validateContacts(context.Background(), &[]string{nonASCII})
|
||||
test.AssertError(t, err, "Non ASCII email")
|
||||
}
|
||||
|
||||
func TestValidateEmail(t *testing.T) {
|
||||
testFailures := []struct {
|
||||
input string
|
||||
expected string
|
||||
}{
|
||||
{"an email`", unparseableEmailError.Error()},
|
||||
{"a@always.invalid", emptyDNSResponseError.Error()},
|
||||
{"a@email.com, b@email.com", unparseableEmailError.Error()},
|
||||
{"a@always.error", "DNS problem: networking error looking up A for always.error"},
|
||||
{"a@example.com", "invalid contact domain. Contact emails @example.com are forbidden"},
|
||||
{"a@example.net", "invalid contact domain. Contact emails @example.net are forbidden"},
|
||||
{"a@example.org", "invalid contact domain. Contact emails @example.org are forbidden"},
|
||||
}
|
||||
err = ra.validateContacts(context.Background(), &[]string{unparseable})
|
||||
test.AssertError(t, err, "Unparseable email")
|
||||
|
||||
testSuccesses := []string{
|
||||
"a@email.com",
|
||||
"b@email.only",
|
||||
// A timeout during email validation is treated as a success. We treat email
|
||||
// validation during registration as a best-effort. See
|
||||
// https://github.com/letsencrypt/boulder/issues/2260 for more
|
||||
"a@always.timeout",
|
||||
}
|
||||
|
||||
for _, tc := range testFailures {
|
||||
err := validateEmail(context.Background(), tc.input, &bdns.MockDNSClient{})
|
||||
if !berrors.Is(err, berrors.InvalidEmail) {
|
||||
t.Errorf("validateEmail(%q): got error %#v, expected type berrors.InvalidEmail", tc.input, err)
|
||||
}
|
||||
|
||||
if err.Error() != tc.expected {
|
||||
t.Errorf("validateEmail(%q): got %#v, expected %#v",
|
||||
tc.input, err.Error(), tc.expected)
|
||||
}
|
||||
}
|
||||
|
||||
for _, addr := range testSuccesses {
|
||||
if err := validateEmail(context.Background(), addr, &bdns.MockDNSClient{}); err != nil {
|
||||
t.Errorf("validateEmail(%q): expected success, but it failed: %#v",
|
||||
addr, err)
|
||||
}
|
||||
}
|
||||
err = ra.validateContacts(context.Background(), &[]string{forbidden})
|
||||
test.AssertError(t, err, "Forbidden email")
|
||||
}
|
||||
|
||||
func TestNewRegistration(t *testing.T) {
|
||||
|
|
|
|||
|
|
@ -63,18 +63,6 @@ func (s *ChallSrv) dnsHandler(w dns.ResponseWriter, r *dns.Msg) {
|
|||
}
|
||||
record.A = net.ParseIP(fakeDNS)
|
||||
|
||||
m.Answer = append(m.Answer, record)
|
||||
case dns.TypeMX:
|
||||
record := new(dns.MX)
|
||||
record.Hdr = dns.RR_Header{
|
||||
Name: q.Name,
|
||||
Rrtype: dns.TypeMX,
|
||||
Class: dns.ClassINET,
|
||||
Ttl: 0,
|
||||
}
|
||||
record.Mx = "mail." + q.Name
|
||||
record.Preference = 10
|
||||
|
||||
m.Answer = append(m.Answer, record)
|
||||
case dns.TypeTXT:
|
||||
values := s.GetDNSOneChallenge(q.Name)
|
||||
|
|
|
|||
Loading…
Reference in New Issue