Merge pull request #154 from rolandshoemaker/sanity

Challenge sanity check

core/objects.go

@@ -7,9 +7,11 @@ package core
 
 import (
 	"crypto/x509"
+	"encoding/hex"
 	"encoding/json"
 	"github.com/letsencrypt/boulder/jose"
 	"time"
+	"strings"
 )
 
 type IdentifierType string
@@ -152,6 +154,78 @@ type Challenge struct {
 	Nonce string `json:"nonce,omitempty"`
 }
 
+// Check the sanity of a challenge object before issued to the client (completed = false)
+// and before validation (completed = true).
+func (ch Challenge) IsSane(completed bool) bool {
+	if ch.Status != StatusPending {
+		return false
+	}
+
+	switch ch.Type {
+	case ChallengeTypeSimpleHTTPS:
+		// check extra fields aren't used
+		if ch.R != "" || ch.S != "" || ch.Nonce != "" {
+			return false
+		}
+
+		if completed {
+			// see if ch.Path starts with /.well-known/acme-challenge/
+			if ch.Path == "" || !strings.HasPrefix(ch.Path, "/.well-known/acme-challenge/") {
+				return false
+			}
+		} else {
+			if ch.Path != "" {
+				return false
+			}
+		}
+
+		// check token is present, corrent length, and contains b64 encoded string
+		if ch.Token == "" || len(ch.Token) != 43 {
+			return false
+		}
+		if _, err := B64dec(ch.Token); err != nil {
+			return false
+		}
+	case ChallengeTypeDVSNI:
+		// check extra fields aren't used
+		if ch.Path != "" || ch.Token != "" {
+			return false
+		}
+
+		if ch.Nonce == "" || len(ch.Nonce) != 32 {
+			return false
+		}
+		if _, err := hex.DecodeString(ch.Nonce); err != nil {
+			return false
+		}
+
+		// Check R & S are sane
+		if ch.R == "" || len(ch.R) != 43 {
+			return false
+		}
+		if _, err := B64dec(ch.R); err != nil {
+			return false
+		}
+
+		if completed {
+			if ch.S == "" || len(ch.S) != 43 {
+				return false
+			}
+			if _, err := B64dec(ch.S); err != nil {
+				return false
+			}
+		} else {
+			if ch.S != "" {
+				return false
+			}
+		}
+	default:
+		return false
+	}
+
+	return true
+}
+
 // Merge a client-provide response to a challenge with the issued challenge
 // TODO: Remove return type from this method
 func (ch Challenge) MergeResponse(resp Challenge) Challenge {

core/objects_test.go

@@ -0,0 +1,61 @@
+// Copyright 2015 ISRG.  All rights reserved
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+package core
+
+import (
+	"testing"
+
+	"github.com/letsencrypt/boulder/test"
+)
+
+func TestSanityCheck(t *testing.T) {
+	chall := Challenge{Type: ChallengeTypeSimpleHTTPS, Status: StatusValid}
+	test.Assert(t, !chall.IsSane(false), "IsSane should be false")
+	chall.Status = StatusPending
+	test.Assert(t, !chall.IsSane(false), "IsSane should be false")
+	chall.R = "bad"
+	chall.S = "bad"
+	chall.Nonce = "bad"
+	test.Assert(t, !chall.IsSane(false), "IsSane should be false")
+	chall = Challenge{Type: ChallengeTypeSimpleHTTPS, Path: "bad", Status: StatusPending}
+	test.Assert(t, !chall.IsSane(false), "IsSane should be false")
+	chall.Path = ""
+	test.Assert(t, !chall.IsSane(true), "IsSane should be false")
+	chall.Token = ""
+	test.Assert(t, !chall.IsSane(false), "IsSane should be false")
+	chall.Token = "notlongenough"
+	test.Assert(t, !chall.IsSane(false), "IsSane should be false")
+	chall.Token = "evaGxfADs6pSRb2LAv9IZf17Dt3juxGJ+PCt92wr+o!"
+	test.Assert(t, !chall.IsSane(false), "IsSane should be false")
+	chall.Token = "KQqLsiS5j0CONR_eUXTUSUDNVaHODtc-0pD6ACif7U4"
+	test.Assert(t, chall.IsSane(false), "IsSane should be true")
+
+	chall = Challenge{Type: ChallengeTypeDVSNI, Status: StatusPending}
+	chall.Path = "bad"
+	chall.Token = "bad"
+	test.Assert(t, !chall.IsSane(false), "IsSane should be false")
+	chall = Challenge{Type: ChallengeTypeDVSNI, Status: StatusPending}
+	test.Assert(t, !chall.IsSane(false), "IsSane should be false")
+	chall.Nonce = "wutwut"
+	test.Assert(t, !chall.IsSane(false), "IsSane should be false")
+	chall.Nonce = "!2345678901234567890123456789012"
+	test.Assert(t, !chall.IsSane(false), "IsSane should be false")
+	chall.Nonce = "12345678901234567890123456789012"
+	test.Assert(t, !chall.IsSane(false), "IsSane should be false")
+	chall.R = "notlongenough"
+	test.Assert(t, !chall.IsSane(false), "IsSane should be false")
+	chall.R = "evaGxfADs6pSRb2LAv9IZf17Dt3juxGJ+PCt92wr+o!"
+	test.Assert(t, !chall.IsSane(false), "IsSane should be false")
+	chall.R = "KQqLsiS5j0CONR_eUXTUSUDNVaHODtc-0pD6ACif7U4"
+	test.Assert(t, chall.IsSane(false), "IsSane should be true")
+	chall.S = "anything"
+	test.Assert(t, !chall.IsSane(false), "IsSane should be false")
+	test.Assert(t, !chall.IsSane(true), "IsSane should be false")
+	chall.S = "evaGxfADs6pSRb2LAv9IZf17Dt3juxGJ+PCt92wr+o!"
+	test.Assert(t, !chall.IsSane(true), "IsSane should be false")
+	chall.S = "KQqLsiS5j0CONR_eUXTUSUDNVaHODtc-0pD6ACif7U4"
+	test.Assert(t, chall.IsSane(true), "IsSane should be true")
+}

ra/registration-authority.go

@@ -83,6 +83,11 @@ func (ra *RegistrationAuthorityImpl) NewAuthorization(request core.Authorization
 		// Ignoring these errors because we construct the URLs to be correct
 		challengeURI, _ := url.Parse(ra.AuthzBase + authID + "?challenge=" + strconv.Itoa(i))
 		challenges[i].URI = core.AcmeURL(*challengeURI)
+
+		if !challenges[i].IsSane(false) {
+			err = fmt.Errorf("Challenge didn't pass sanity check: %+v", challenges[i])
+			return
+		}
 	}
 
 	// Create a new authorization object

va/validation-authority.go

@@ -171,6 +171,11 @@ func (va ValidationAuthorityImpl) validate(authz core.Authorization) {
 	// Select the first supported validation method
 	// XXX: Remove the "break" lines to process all supported validations
 	for i, challenge := range authz.Challenges {
+		if !challenge.IsSane(true) {
+			challenge.Status = core.StatusInvalid
+			continue
+		}
+
 		switch challenge.Type {
 		case core.ChallengeTypeSimpleHTTPS:
 			authz.Challenges[i] = va.validateSimpleHTTPS(authz.Identifier, challenge)