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:
parent
a64928bc3d
commit
db01b0b5bc
129
ra/ra.go
129
ra/ra.go
|
@ -11,7 +11,6 @@ import (
|
||||||
"reflect"
|
"reflect"
|
||||||
"sort"
|
"sort"
|
||||||
"strings"
|
"strings"
|
||||||
"sync"
|
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/jmhodges/clock"
|
"github.com/jmhodges/clock"
|
||||||
|
@ -161,80 +160,6 @@ func (ra *RegistrationAuthorityImpl) rateLimitPoliciesLoadError(err error) {
|
||||||
ra.log.Errf("error reloading rate limit policy: %s", err)
|
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
|
// certificateRequestAuthz is a struct for holding information about a valid
|
||||||
// authz referenced during a certificateRequestEvent. It holds both the
|
// authz referenced during a certificateRequestEvent. It holds both the
|
||||||
// authorization ID and the challenge type that made the authorization valid. We
|
// 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
|
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 {
|
func (ra *RegistrationAuthorityImpl) validateContacts(ctx context.Context, contacts *[]string) error {
|
||||||
if contacts == nil || len(*contacts) == 0 {
|
if contacts == nil || len(*contacts) == 0 {
|
||||||
return nil // Nothing to validate
|
return nil // Nothing to validate
|
||||||
|
@ -413,21 +347,50 @@ func (ra *RegistrationAuthorityImpl) validateContacts(ctx context.Context, conta
|
||||||
contact,
|
contact,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
if err := validateEmail(parsed.Opaque); err != nil {
|
||||||
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)
|
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
ra.stats.Inc("ValidateEmail.Successes", 1)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return nil
|
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 {
|
func (ra *RegistrationAuthorityImpl) checkPendingAuthorizationLimit(ctx context.Context, regID int64) error {
|
||||||
limit := ra.rlPolicies.PendingAuthorizationsPerAccount()
|
limit := ra.rlPolicies.PendingAuthorizationsPerAccount()
|
||||||
if limit.Enabled() {
|
if limit.Enabled() {
|
||||||
|
|
|
@ -330,6 +330,8 @@ func TestValidateContacts(t *testing.T) {
|
||||||
otherValidEmail := "mailto:other-admin@email.com"
|
otherValidEmail := "mailto:other-admin@email.com"
|
||||||
malformedEmail := "mailto:admin.com"
|
malformedEmail := "mailto:admin.com"
|
||||||
nonASCII := "mailto:señor@email.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{})
|
err := ra.validateContacts(context.Background(), &[]string{})
|
||||||
test.AssertNotError(t, err, "No Contacts")
|
test.AssertNotError(t, err, "No Contacts")
|
||||||
|
@ -351,49 +353,12 @@ func TestValidateContacts(t *testing.T) {
|
||||||
|
|
||||||
err = ra.validateContacts(context.Background(), &[]string{nonASCII})
|
err = ra.validateContacts(context.Background(), &[]string{nonASCII})
|
||||||
test.AssertError(t, err, "Non ASCII email")
|
test.AssertError(t, err, "Non ASCII email")
|
||||||
}
|
|
||||||
|
|
||||||
func TestValidateEmail(t *testing.T) {
|
err = ra.validateContacts(context.Background(), &[]string{unparseable})
|
||||||
testFailures := []struct {
|
test.AssertError(t, err, "Unparseable email")
|
||||||
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"},
|
|
||||||
}
|
|
||||||
|
|
||||||
testSuccesses := []string{
|
err = ra.validateContacts(context.Background(), &[]string{forbidden})
|
||||||
"a@email.com",
|
test.AssertError(t, err, "Forbidden email")
|
||||||
"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)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestNewRegistration(t *testing.T) {
|
func TestNewRegistration(t *testing.T) {
|
||||||
|
|
|
@ -63,18 +63,6 @@ func (s *ChallSrv) dnsHandler(w dns.ResponseWriter, r *dns.Msg) {
|
||||||
}
|
}
|
||||||
record.A = net.ParseIP(fakeDNS)
|
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)
|
m.Answer = append(m.Answer, record)
|
||||||
case dns.TypeTXT:
|
case dns.TypeTXT:
|
||||||
values := s.GetDNSOneChallenge(q.Name)
|
values := s.GetDNSOneChallenge(q.Name)
|
||||||
|
|
Loading…
Reference in New Issue