From 8c547473b89c7d86f32a1c6529b3025d753608bb Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 9 May 2017 15:43:16 -0400 Subject: [PATCH 1/2] Adds "meta" entry w/ ToS to /directory. This commit adds the acme draft-02+ optional "meta" element for the /directory response. Presently we only include the optional "terms-of-service" URL. Whether the meta entry is included is controlled by two factors: 1. The state of the "DirectoryMeta" feature flag, which defaults to off 2. Whether the client advertises the UA we know to be intolerant of new directory entries. The TestDirectory unit test is updated to test both states of the flag and the UA detection. --- features/featureflag_string.go | 4 ++-- features/features.go | 2 ++ test/config-next/wfe.json | 3 ++- wfe/wfe.go | 38 ++++++++++++++++++++++++---------- wfe/wfe_test.go | 16 ++++++++++++++ 5 files changed, 49 insertions(+), 14 deletions(-) diff --git a/features/featureflag_string.go b/features/featureflag_string.go index 2d62143ce..3a0afefa2 100644 --- a/features/featureflag_string.go +++ b/features/featureflag_string.go @@ -4,9 +4,9 @@ package features import "fmt" -const _FeatureFlag_name = "unusedIDNASupportAllowAccountDeactivationAllowKeyRolloverResubmitMissingSCTsOnlyGoogleSafeBrowsingV4UseAIAIssuerURLAllowTLS02ChallengesGenerateOCSPEarlyCountCertificatesExactRandomDirectoryEntryIPv6First" +const _FeatureFlag_name = "unusedIDNASupportAllowAccountDeactivationAllowKeyRolloverResubmitMissingSCTsOnlyGoogleSafeBrowsingV4UseAIAIssuerURLAllowTLS02ChallengesGenerateOCSPEarlyCountCertificatesExactRandomDirectoryEntryIPv6FirstDirectoryMeta" -var _FeatureFlag_index = [...]uint8{0, 6, 17, 41, 57, 80, 100, 115, 135, 152, 174, 194, 203} +var _FeatureFlag_index = [...]uint8{0, 6, 17, 41, 57, 80, 100, 115, 135, 152, 174, 194, 203, 216} func (i FeatureFlag) String() string { if i < 0 || i >= FeatureFlag(len(_FeatureFlag_index)-1) { diff --git a/features/features.go b/features/features.go index ec67bc72c..72f366d2d 100644 --- a/features/features.go +++ b/features/features.go @@ -23,6 +23,7 @@ const ( CountCertificatesExact RandomDirectoryEntry IPv6First + DirectoryMeta ) // List of features and their default value, protected by fMu @@ -39,6 +40,7 @@ var features = map[FeatureFlag]bool{ CountCertificatesExact: false, RandomDirectoryEntry: false, IPv6First: false, + DirectoryMeta: false, } var fMu = new(sync.RWMutex) diff --git a/test/config-next/wfe.json b/test/config-next/wfe.json index 1f349db48..5132164bb 100644 --- a/test/config-next/wfe.json +++ b/test/config-next/wfe.json @@ -33,7 +33,8 @@ "AllowAccountDeactivation": true, "AllowKeyRollover": true, "UseAIAIssuerURL": true, - "RandomDirectoryEntry": true + "RandomDirectoryEntry": true, + "DirectoryMeta": true } }, diff --git a/wfe/wfe.go b/wfe/wfe.go index b3edf4282..95120a2f9 100644 --- a/wfe/wfe.go +++ b/wfe/wfe.go @@ -264,10 +264,10 @@ func (wfe *WebFrontEndImpl) relativeEndpoint(request *http.Request, endpoint str const randomDirKeyExplanationLink = "https://community.letsencrypt.org/t/adding-random-entries-to-the-directory/33417" -func (wfe *WebFrontEndImpl) relativeDirectory(request *http.Request, directory map[string]string) ([]byte, error) { +func (wfe *WebFrontEndImpl) relativeDirectory(request *http.Request, directory map[string]interface{}) ([]byte, error) { // Create an empty map sized equal to the provided directory to store the // relative-ized result - relativeDir := make(map[string]string, len(directory)) + relativeDir := make(map[string]interface{}, len(directory)) // Copy each entry of the provided directory into the new relative map. If // `wfe.BaseURL` != "", use the old behaviour and prefix each endpoint with @@ -277,7 +277,14 @@ func (wfe *WebFrontEndImpl) relativeDirectory(request *http.Request, directory m if features.Enabled(features.RandomDirectoryEntry) && v == randomDirKeyExplanationLink { continue } - relativeDir[k] = wfe.relativeEndpoint(request, v) + switch v.(type) { + case string: + // Only relative-ize top level string values, e.g. not the "meta" element + relativeDir[k] = wfe.relativeEndpoint(request, v.(string)) + default: + // If it isn't a string, put it into the results unmodified + relativeDir[k] = v + } } directoryJSON, err := marshalIndent(relativeDir) @@ -366,25 +373,34 @@ func addRequesterHeader(w http.ResponseWriter, requester int64) { // object stored in the WFE's DirectoryEndpoints member with paths prefixed // using the `request.Host` of the HTTP request. func (wfe *WebFrontEndImpl) Directory(ctx context.Context, logEvent *requestEvent, response http.ResponseWriter, request *http.Request) { - directoryEndpoints := map[string]string{ + directoryEndpoints := map[string]interface{}{ "new-reg": newRegPath, "new-authz": newAuthzPath, "new-cert": newCertPath, "revoke-cert": revokeCertPath, } - if features.Enabled(features.AllowKeyRollover) && !strings.HasPrefix(request.UserAgent(), "LetsEncryptPythonClient") { - // Versions of Certbot pre-0.6.0 (named LetsEncryptPythonClient at the time) break when they - // encounter a directory containing elements they don't expect so we gate adding the key-change - // field on a User-Agent header that doesn't start with 'LetsEncryptPythonClient' + + // Versions of Certbot pre-0.6.0 (named LetsEncryptPythonClient at the time) break when they + // encounter a directory containing elements they don't expect so we gate + // adding new directory fields for clients matching this UA. + clientDirChangeIntolerant := strings.HasPrefix(request.UserAgent(), "LetsEncryptPythonClient") + if features.Enabled(features.AllowKeyRollover) && !clientDirChangeIntolerant { directoryEndpoints["key-change"] = rolloverPath } - if features.Enabled(features.RandomDirectoryEntry) && !strings.HasPrefix(request.UserAgent(), "LetsEncryptPythonClient") { + if features.Enabled(features.RandomDirectoryEntry) && !clientDirChangeIntolerant { // Add a random key to the directory in order to make sure that clients don't hardcode an // expected set of keys. This ensures that we can properly extend the directory when we - // need to add a new endpoint or meta element. Gate on UA not being one of the pre-0.6.0 - // Certbot clients that we know will be broken by this change. + // need to add a new endpoint or meta element. directoryEndpoints[core.RandomString(8)] = randomDirKeyExplanationLink } + if features.Enabled(features.DirectoryMeta) && !clientDirChangeIntolerant { + // ACME since draft-02 describes an optional "meta" directory entry. The + // meta entry may optionally contain a "terms-of-service" URI for the + // current ToS. + directoryEndpoints["meta"] = map[string]string{ + "terms-of-service": wfe.SubscriberAgreementURL, + } + } response.Header().Set("Content-Type", "application/json") diff --git a/wfe/wfe_test.go b/wfe/wfe_test.go index 69e339995..34ae0ad5f 100644 --- a/wfe/wfe_test.go +++ b/wfe/wfe_test.go @@ -626,6 +626,22 @@ func TestDirectory(t *testing.T) { test.AssertEquals(t, responseWriter.Code, http.StatusOK) assertJSONEquals(t, responseWriter.Body.String(), `{"key-change":"http://localhost:4300/acme/key-change","new-authz":"http://localhost:4300/acme/new-authz","new-cert":"http://localhost:4300/acme/new-cert","new-reg":"http://localhost:4300/acme/new-reg","revoke-cert":"http://localhost:4300/acme/revoke-cert"}`) + // With the DirectoryMeta flag enabled we expect to see a "meta" entry with + // the correct terms-of-service URL. + _ = features.Set(map[string]bool{"DirectoryMeta": true}) + responseWriter.Body.Reset() + url, _ = url.Parse("/directory") + mux.ServeHTTP(responseWriter, &http.Request{ + Method: "GET", + URL: url, + Host: "127.0.0.1:4300", + }) + test.AssertEquals(t, responseWriter.Header().Get("Content-Type"), "application/json") + test.AssertEquals(t, responseWriter.Code, http.StatusOK) + assertJSONEquals(t, responseWriter.Body.String(), `{"key-change":"http://localhost:4300/acme/key-change","meta":{"terms-of-service":"http://example.invalid/terms"},"new-authz":"http://localhost:4300/acme/new-authz","new-cert":"http://localhost:4300/acme/new-cert","new-reg":"http://localhost:4300/acme/new-reg","revoke-cert":"http://localhost:4300/acme/revoke-cert"}`) + + // Even with the DirectoryMeta flag enabled, if the UA is + // LetsEncryptPythonClient we expect to *not* see the meta entry. responseWriter.Body.Reset() url, _ = url.Parse("/directory") headers := map[string][]string{ From 45837b0f71902ff64355363523d5234d8a544cdd Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 12 May 2017 14:50:27 -0400 Subject: [PATCH 2/2] Avoids double type assert --- wfe/wfe.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wfe/wfe.go b/wfe/wfe.go index 95120a2f9..3b4b134cc 100644 --- a/wfe/wfe.go +++ b/wfe/wfe.go @@ -277,10 +277,10 @@ func (wfe *WebFrontEndImpl) relativeDirectory(request *http.Request, directory m if features.Enabled(features.RandomDirectoryEntry) && v == randomDirKeyExplanationLink { continue } - switch v.(type) { + switch v := v.(type) { case string: // Only relative-ize top level string values, e.g. not the "meta" element - relativeDir[k] = wfe.relativeEndpoint(request, v.(string)) + relativeDir[k] = wfe.relativeEndpoint(request, v) default: // If it isn't a string, put it into the results unmodified relativeDir[k] = v