diff --git a/core/util.go b/core/util.go index cae357663..98929a163 100644 --- a/core/util.go +++ b/core/util.go @@ -42,6 +42,12 @@ var BuildTime string // Errors +// InternalServerError indicates that something has gone wrong unrelated to the +// user's input, and will be considered by the Load Balancer as an indication +// that this Boulder instance may be malfunctioning. Minimally, returning this +// will cause an error page to be generated at the CDN/LB for the client. +// Consequently, you should only use this error when Boulder's internal +// constraints have been violated. type InternalServerError string type NotSupportedError string type MalformedRequestError string diff --git a/ra/registration-authority.go b/ra/registration-authority.go index 62b39703a..9122a2f8c 100644 --- a/ra/registration-authority.go +++ b/ra/registration-authority.go @@ -61,17 +61,32 @@ func validateEmail(address string) (err error) { domain := strings.ToLower(splitEmail[len(splitEmail)-1]) var mx []*net.MX mx, err = net.LookupMX(domain) - if err != nil { - err = core.InternalServerError(err.Error()) - return - } - if len(mx) == 0 { + if err != nil || len(mx) == 0 { err = core.MalformedRequestError(fmt.Sprintf("No MX record for domain %s", domain)) return } return } +func validateContacts(contacts []core.AcmeURL) (err error) { + for _, contact := range contacts { + switch contact.Scheme { + case "tel": + continue + case "mailto": + err = validateEmail(contact.Opaque) + if err != nil { + return + } + default: + err = core.MalformedRequestError(fmt.Sprintf("Contact method %s is not supported", contact.Scheme)) + return + } + } + + return +} + type certificateRequestEvent struct { ID string `json:",omitempty"` Requester int64 `json:",omitempty"` @@ -98,24 +113,16 @@ func (ra *RegistrationAuthorityImpl) NewRegistration(init core.Registration) (re } reg.MergeUpdate(init) - for _, contact := range reg.Contact { - switch contact.Scheme { - case "tel": - continue - case "mailto": - err = validateEmail(contact.Opaque) - if err != nil { - return - } - default: - err = core.MalformedRequestError(fmt.Sprintf("Contact method %s is not supported", contact.Scheme)) - return - } + err = validateContacts(reg.Contact) + if err != nil { + return } // Store the authorization object, then return it reg, err = ra.SA.NewRegistration(reg) if err != nil { + // InternalServerError since the user-data was validated before being + // passed to the SA. err = core.InternalServerError(err.Error()) } @@ -124,7 +131,7 @@ func (ra *RegistrationAuthorityImpl) NewRegistration(init core.Registration) (re func (ra *RegistrationAuthorityImpl) NewAuthorization(request core.Authorization, regID int64) (authz core.Authorization, err error) { if regID <= 0 { - err = core.InternalServerError("Invalid registration ID") + err = core.MalformedRequestError(fmt.Sprintf("Invalid registration ID: %d", regID)) return authz, err } @@ -162,7 +169,9 @@ func (ra *RegistrationAuthorityImpl) NewAuthorization(request core.Authorization // Get a pending Auth first so we can get our ID back, then update with challenges authz, err = ra.SA.NewPendingAuthorization(authz) if err != nil { - err = core.InternalServerError(err.Error()) + // InternalServerError since the user-data was validated before being + // passed to the SA. + err = core.InternalServerError(fmt.Sprintf("Invalid authorization request: %s", err)) return authz, err } @@ -173,6 +182,8 @@ func (ra *RegistrationAuthorityImpl) NewAuthorization(request core.Authorization challenges[i].URI = core.AcmeURL(*challengeURI) if !challenges[i].IsSane(false) { + // InternalServerError because we generated these challenges, they should + // be OK. err = core.InternalServerError(fmt.Sprintf("Challenge didn't pass sanity check: %+v", challenges[i])) return authz, err } @@ -184,6 +195,8 @@ func (ra *RegistrationAuthorityImpl) NewAuthorization(request core.Authorization // Store the authorization object, then return it err = ra.SA.UpdatePendingAuthorization(authz) if err != nil { + // InternalServerError because we created the authorization just above, + // and adding Sane challenges should not break it. err = core.InternalServerError(err.Error()) } return authz, err @@ -211,7 +224,7 @@ func (ra *RegistrationAuthorityImpl) NewCertificate(req core.CertificateRequest, }() if regID <= 0 { - err = core.InternalServerError("Invalid registration ID") + err = core.MalformedRequestError(fmt.Sprintf("Invalid registration ID: %d", regID)) return emptyCert, err } @@ -312,7 +325,10 @@ func (ra *RegistrationAuthorityImpl) NewCertificate(req core.CertificateRequest, // Create the certificate and log the result if cert, err = ra.CA.IssueCertificate(*csr, regID, earliestExpiry); err != nil { - err = core.InternalServerError(err.Error()) + // While this could be InternalServerError for certain conditions, most + // of the failure reasons (such as GoodKey failing) are caused by malformed + // requests. + err = core.MalformedRequestError(err.Error()) logEvent.Error = err.Error() return emptyCert, err } @@ -325,6 +341,8 @@ func (ra *RegistrationAuthorityImpl) NewCertificate(req core.CertificateRequest, parsedCertificate, err := x509.ParseCertificate([]byte(cert.DER)) if err != nil { + // InternalServerError because the certificate from the CA should be + // parseable. err = core.InternalServerError(err.Error()) logEvent.Error = err.Error() return emptyCert, err @@ -342,10 +360,18 @@ func (ra *RegistrationAuthorityImpl) NewCertificate(req core.CertificateRequest, func (ra *RegistrationAuthorityImpl) UpdateRegistration(base core.Registration, update core.Registration) (reg core.Registration, err error) { base.MergeUpdate(update) + + err = validateContacts(base.Contact) + if err != nil { + return + } + reg = base err = ra.SA.UpdateRegistration(base) if err != nil { - err = core.InternalServerError(err.Error()) + // InternalServerError since the user-data was validated before being + // passed to the SA. + err = core.InternalServerError(fmt.Sprintf("Could not update registration: %s", err)) } return } @@ -354,14 +380,16 @@ func (ra *RegistrationAuthorityImpl) UpdateAuthorization(base core.Authorization // Copy information over that the client is allowed to supply authz = base if challengeIndex >= len(authz.Challenges) { - err = core.MalformedRequestError("Invalid challenge index") + err = core.MalformedRequestError(fmt.Sprintf("Invalid challenge index: %d", challengeIndex)) return } authz.Challenges[challengeIndex] = authz.Challenges[challengeIndex].MergeResponse(response) // Store the updated version if err = ra.SA.UpdatePendingAuthorization(authz); err != nil { - err = core.InternalServerError(err.Error()) + // This can pretty much only happen when the client corrupts the Challenge + // data. + err = core.MalformedRequestError(err.Error()) return } diff --git a/ra/registration-authority_test.go b/ra/registration-authority_test.go index 183a7cb28..1933aa334 100644 --- a/ra/registration-authority_test.go +++ b/ra/registration-authority_test.go @@ -219,6 +219,48 @@ func assertAuthzEqual(t *testing.T, a1, a2 core.Authorization) { // Not testing: Challenges } +func TestValidateContacts(t *testing.T) { + tel, _ := url.Parse("tel:") + ansible, _ := url.Parse("ansible:earth.sol.milkyway.laniakea/letsencrypt") + validEmail, _ := url.Parse("mailto:admin@email.com") + invalidEmail, _ := url.Parse("mailto:admin@example.com") + malformedEmail, _ := url.Parse("mailto:admin.com") + + err := validateContacts([]core.AcmeURL{}) + test.AssertNotError(t, err, "No Contacts") + + err = validateContacts([]core.AcmeURL{core.AcmeURL(*tel)}) + test.AssertNotError(t, err, "Simple Telephone") + + err = validateContacts([]core.AcmeURL{core.AcmeURL(*validEmail)}) + test.AssertNotError(t, err, "Valid Email") + + err = validateContacts([]core.AcmeURL{core.AcmeURL(*invalidEmail)}) + test.AssertError(t, err, "Invalid Email") + + err = validateContacts([]core.AcmeURL{core.AcmeURL(*malformedEmail)}) + test.AssertError(t, err, "Malformed Email") + + err = validateContacts([]core.AcmeURL{core.AcmeURL(*ansible)}) + test.AssertError(t, err, "Unknown scehme") +} + +func TestValidateEmail(t *testing.T) { + err := validateEmail("an email`") + test.AssertError(t, err, "Malformed") + + err = validateEmail("a@not.a.domain") + test.AssertError(t, err, "Cannot resolve") + t.Logf("No Resolve: %s", err) + + err = validateEmail("a@example.com") + test.AssertError(t, err, "No MX Record") + t.Logf("No MX: %s", err) + + err = validateEmail("a@email.com") + test.AssertNotError(t, err, "Valid") +} + func TestNewRegistration(t *testing.T) { _, _, sa, ra := initAuthorities(t) mailto, _ := url.Parse("mailto:foo@letsencrypt.org") diff --git a/wfe/web-front-end.go b/wfe/web-front-end.go index fb9ac9c84..76d8963d2 100644 --- a/wfe/web-front-end.go +++ b/wfe/web-front-end.go @@ -71,8 +71,18 @@ func statusCodeFromError(err interface{}) int { switch err.(type) { case core.MalformedRequestError: return http.StatusBadRequest + case core.NotSupportedError: + return http.StatusNotImplemented + case core.SyntaxError: + return http.StatusBadRequest case core.UnauthorizedError: return http.StatusForbidden + case core.NotFoundError: + return http.StatusNotFound + case core.SignatureValidationError: + return http.StatusPreconditionFailed + case core.InternalServerError: + return http.StatusInternalServerError default: return http.StatusInternalServerError } @@ -327,6 +337,7 @@ func (wfe *WebFrontEndImpl) NewRegistration(response http.ResponseWriter, reques regURL := fmt.Sprintf("%s%d", wfe.RegBase, id) responseBody, err := json.Marshal(reg) if err != nil { + // StatusInternalServerError because we just created this registration, it should be OK. wfe.sendError(response, "Error marshaling registration", err, http.StatusInternalServerError) return } @@ -390,6 +401,7 @@ func (wfe *WebFrontEndImpl) NewAuthorization(response http.ResponseWriter, reque authz.RegistrationID = 0 responseBody, err := json.Marshal(authz) if err != nil { + // StatusInternalServerError because we generated the authz, it should be OK wfe.sendError(response, "Error marshaling authz", err, http.StatusInternalServerError) return } @@ -446,6 +458,7 @@ func (wfe *WebFrontEndImpl) RevokeCertificate(response http.ResponseWriter, requ } parsedCertificate, err := x509.ParseCertificate(cert.DER) if err != nil { + // InternalServerError because this is a failure to decode from our DB. wfe.sendError(response, "Invalid certificate", err, http.StatusInternalServerError) return } @@ -557,7 +570,7 @@ func (wfe *WebFrontEndImpl) NewCertificate(response http.ResponseWriter, request wfe.Stats.Inc("Certificates", 1, 1.0) } -func (wfe *WebFrontEndImpl) Challenge(authz core.Authorization, response http.ResponseWriter, request *http.Request) { +func (wfe *WebFrontEndImpl) challenge(authz core.Authorization, response http.ResponseWriter, request *http.Request) { wfe.sendStandardHeaders(response) if request.Method != "GET" && request.Method != "POST" { @@ -593,6 +606,8 @@ func (wfe *WebFrontEndImpl) Challenge(authz core.Authorization, response http.Re challenge := authz.Challenges[challengeIndex] jsonReply, err := json.Marshal(challenge) if err != nil { + // InternalServerError because this is a failure to decode data passed in + // by the caller, which got it from the DB. wfe.sendError(response, "Failed to marshal challenge", err, http.StatusInternalServerError) return } @@ -651,6 +666,7 @@ func (wfe *WebFrontEndImpl) Challenge(authz core.Authorization, response http.Re // assumption: UpdateAuthorization does not modify order of challenges jsonReply, err := json.Marshal(challenge) if err != nil { + // StatusInternalServerError because we made the challenges, they should be OK wfe.sendError(response, "Failed to marshal challenge", err, http.StatusInternalServerError) return } @@ -734,6 +750,7 @@ func (wfe *WebFrontEndImpl) Registration(response http.ResponseWriter, request * jsonReply, err := json.Marshal(updatedReg) if err != nil { + // StatusInternalServerError because we just generated the reg, it should be OK wfe.sendError(response, "Failed to marshal registration", err, http.StatusInternalServerError) return } @@ -763,7 +780,7 @@ func (wfe *WebFrontEndImpl) Authorization(response http.ResponseWriter, request // If there is a fragment, then this is actually a request to a challenge URI if len(request.URL.RawQuery) != 0 { - wfe.Challenge(authz, response, request) + wfe.challenge(authz, response, request) return } @@ -780,6 +797,7 @@ func (wfe *WebFrontEndImpl) Authorization(response http.ResponseWriter, request jsonReply, err := json.Marshal(authz) if err != nil { + // InternalServerError because this is a failure to decode from our DB. wfe.sendError(response, "Failed to marshal authz", err, http.StatusInternalServerError) return } @@ -827,7 +845,7 @@ func (wfe *WebFrontEndImpl) Certificate(response http.ResponseWriter, request *h cert, err := wfe.SA.GetCertificateByShortSerial(serial) if err != nil { if strings.HasPrefix(err.Error(), "gorp: multiple rows returned") { - wfe.sendError(response, "Multiple certificates with same short serial", err, http.StatusInternalServerError) + wfe.sendError(response, "Multiple certificates with same short serial", err, http.StatusConflict) } else { wfe.sendError(response, "Not found", err, http.StatusNotFound) } diff --git a/wfe/web-front-end_test.go b/wfe/web-front-end_test.go index 68d80ff82..ef8169c68 100644 --- a/wfe/web-front-end_test.go +++ b/wfe/web-front-end_test.go @@ -550,7 +550,7 @@ func TestChallenge(t *testing.T) { RegistrationID: 1, } - wfe.Challenge(authz, responseWriter, &http.Request{ + wfe.challenge(authz, responseWriter, &http.Request{ Method: "POST", URL: challengeURL, Body: makeBody(signRequest(t, "{}", &wfe.nonceService)),