From 89f7fb1636d5ea024792cdee162f517182829a75 Mon Sep 17 00:00:00 2001 From: Aaron Gable Date: Fri, 21 Oct 2022 15:54:18 -0700 Subject: [PATCH] Clean up go1.19 TODOs (#6464) Clean up several spots where we were behaving differently on go1.18 and go1.19, now that we're using go1.19 everywhere. Also re-enable the lint and generate tests, and fix the various places where the two versions disagreed on how comments should be formatted. Also clean up the OldTLS codepaths, now that both go1.19 and our own feature flags have forbidden TLS < 1.2 everywhere. Fixes #6011 --- ca/proto/ca.pb.go | 1 + core/objects.go | 5 -- crl/storer/proto/storer.pb.go | 1 + docker-compose.yml | 9 ++- features/featureflag_string.go | 46 +++++++------- features/features.go | 10 +-- test.sh | 54 +++++++---------- va/caa_test.go | 16 ++--- va/http.go | 23 ++----- va/http_test.go | 108 --------------------------------- va/tlsalpn_test.go | 23 ++----- wfe2/wfe.go | 10 ++- wfe2/wfe_test.go | 15 +---- 13 files changed, 77 insertions(+), 244 deletions(-) diff --git a/ca/proto/ca.pb.go b/ca/proto/ca.pb.go index 349538bce..a0dc25c95 100644 --- a/ca/proto/ca.pb.go +++ b/ca/proto/ca.pb.go @@ -343,6 +343,7 @@ type GenerateCRLRequest struct { unknownFields protoimpl.UnknownFields // Types that are assignable to Payload: + // // *GenerateCRLRequest_Metadata // *GenerateCRLRequest_Entry Payload isGenerateCRLRequest_Payload `protobuf_oneof:"payload"` diff --git a/core/objects.go b/core/objects.go index db99a2e22..47ba4b70c 100644 --- a/core/objects.go +++ b/core/objects.go @@ -175,11 +175,6 @@ type ValidationRecord struct { // ... // } AddressesTried []net.IP `json:"addressesTried,omitempty"` - - // OldTLS is true if any request in the validation chain used HTTPS and negotiated - // a TLS version lower than 1.2. - // TODO(#6011): Remove once TLS 1.0 and 1.1 support is gone. - OldTLS bool `json:"oldTLS,omitempty"` } func looksLikeKeyAuthorization(str string) error { diff --git a/crl/storer/proto/storer.pb.go b/crl/storer/proto/storer.pb.go index 17ae73819..4e74a4f6b 100644 --- a/crl/storer/proto/storer.pb.go +++ b/crl/storer/proto/storer.pb.go @@ -27,6 +27,7 @@ type UploadCRLRequest struct { unknownFields protoimpl.UnknownFields // Types that are assignable to Payload: + // // *UploadCRLRequest_Metadata // *UploadCRLRequest_CrlChunk Payload isUploadCRLRequest_Payload `protobuf_oneof:"payload"` diff --git a/docker-compose.yml b/docker-compose.yml index 095e25e9e..02460285f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -10,11 +10,10 @@ services: FAKE_DNS: 10.77.77.77 BOULDER_CONFIG_DIR: test/config GOFLAGS: -mod=vendor - # Go 1.18 turns off SHA-1 validation on CSRs (and certs, but that doesn't - # affect us). It also turns off TLS 1.0 and TLS 1.1. Temporarily go back - # to allowing these so we can upgrade to Go 1.18 while doing a deprecation - # window. These overrides will stop working in Go 1.19. - GODEBUG: x509sha1=1,tls10default=1 + # Go 1.18 turned off SHA-1 validation on CSRs (and certs, but that doesn't + # affect us) by default, but it can be turned back on with the x509sha1 + # flag. This override will stop working in an unknown future release. + GODEBUG: x509sha1=1 volumes: - .:/boulder:cached - ./.gocache:/root/.cache/go-build:cached diff --git a/features/featureflag_string.go b/features/featureflag_string.go index 7ba1e5217..c518f02af 100644 --- a/features/featureflag_string.go +++ b/features/featureflag_string.go @@ -16,36 +16,36 @@ func _() { _ = x[StreamlineOrderAndAuthzs-5] _ = x[V1DisableNewValidations-6] _ = x[ExpirationMailerDontLookTwice-7] - _ = x[ROCSPStage1-8] - _ = x[ROCSPStage2-9] - _ = x[ROCSPStage3-10] - _ = x[CAAValidationMethods-11] - _ = x[CAAAccountURI-12] - _ = x[EnforceMultiVA-13] - _ = x[MultiVAFullResults-14] - _ = x[MandatoryPOSTAsGET-15] - _ = x[AllowV1Registration-16] - _ = x[StoreRevokerInfo-17] - _ = x[RestrictRSAKeySizes-18] - _ = x[FasterNewOrdersRateLimit-19] - _ = x[ECDSAForAll-20] - _ = x[ServeRenewalInfo-21] - _ = x[GetAuthzReadOnly-22] - _ = x[GetAuthzUseIndex-23] - _ = x[CheckFailedAuthorizationsFirst-24] - _ = x[AllowReRevocation-25] - _ = x[MozRevocationReasons-26] - _ = x[OldTLSOutbound-27] - _ = x[OldTLSInbound-28] + _ = x[OldTLSInbound-8] + _ = x[OldTLSOutbound-9] + _ = x[ROCSPStage1-10] + _ = x[ROCSPStage2-11] + _ = x[ROCSPStage3-12] + _ = x[CAAValidationMethods-13] + _ = x[CAAAccountURI-14] + _ = x[EnforceMultiVA-15] + _ = x[MultiVAFullResults-16] + _ = x[MandatoryPOSTAsGET-17] + _ = x[AllowV1Registration-18] + _ = x[StoreRevokerInfo-19] + _ = x[RestrictRSAKeySizes-20] + _ = x[FasterNewOrdersRateLimit-21] + _ = x[ECDSAForAll-22] + _ = x[ServeRenewalInfo-23] + _ = x[GetAuthzReadOnly-24] + _ = x[GetAuthzUseIndex-25] + _ = x[CheckFailedAuthorizationsFirst-26] + _ = x[AllowReRevocation-27] + _ = x[MozRevocationReasons-28] _ = x[SHA1CSRs-29] _ = x[AllowUnrecognizedFeatures-30] _ = x[RejectDuplicateCSRExtensions-31] _ = x[ROCSPStage6-32] } -const _FeatureFlag_name = "unusedPrecertificateRevocationStripDefaultSchemePortNonCFSSLSignerStoreIssuerInfoStreamlineOrderAndAuthzsV1DisableNewValidationsExpirationMailerDontLookTwiceROCSPStage1ROCSPStage2ROCSPStage3CAAValidationMethodsCAAAccountURIEnforceMultiVAMultiVAFullResultsMandatoryPOSTAsGETAllowV1RegistrationStoreRevokerInfoRestrictRSAKeySizesFasterNewOrdersRateLimitECDSAForAllServeRenewalInfoGetAuthzReadOnlyGetAuthzUseIndexCheckFailedAuthorizationsFirstAllowReRevocationMozRevocationReasonsOldTLSOutboundOldTLSInboundSHA1CSRsAllowUnrecognizedFeaturesRejectDuplicateCSRExtensionsROCSPStage6" +const _FeatureFlag_name = "unusedPrecertificateRevocationStripDefaultSchemePortNonCFSSLSignerStoreIssuerInfoStreamlineOrderAndAuthzsV1DisableNewValidationsExpirationMailerDontLookTwiceOldTLSInboundOldTLSOutboundROCSPStage1ROCSPStage2ROCSPStage3CAAValidationMethodsCAAAccountURIEnforceMultiVAMultiVAFullResultsMandatoryPOSTAsGETAllowV1RegistrationStoreRevokerInfoRestrictRSAKeySizesFasterNewOrdersRateLimitECDSAForAllServeRenewalInfoGetAuthzReadOnlyGetAuthzUseIndexCheckFailedAuthorizationsFirstAllowReRevocationMozRevocationReasonsSHA1CSRsAllowUnrecognizedFeaturesRejectDuplicateCSRExtensionsROCSPStage6" -var _FeatureFlag_index = [...]uint16{0, 6, 30, 52, 66, 81, 105, 128, 157, 168, 179, 190, 210, 223, 237, 255, 273, 292, 308, 327, 351, 362, 378, 394, 410, 440, 457, 477, 491, 504, 512, 537, 565, 576} +var _FeatureFlag_index = [...]uint16{0, 6, 30, 52, 66, 81, 105, 128, 157, 170, 184, 195, 206, 217, 237, 250, 264, 282, 300, 319, 335, 354, 378, 389, 405, 421, 437, 467, 484, 504, 512, 537, 565, 576} 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 8a97ae469..b1ef8fe14 100644 --- a/features/features.go +++ b/features/features.go @@ -20,6 +20,8 @@ const ( StreamlineOrderAndAuthzs V1DisableNewValidations ExpirationMailerDontLookTwice + OldTLSInbound + OldTLSOutbound ROCSPStage1 ROCSPStage2 ROCSPStage3 @@ -82,14 +84,6 @@ const ( // with the certificate's keypair, the cert will be revoked with reason // keyCompromise, regardless of what revocation reason they request. MozRevocationReasons - // OldTLSOutbound allows the VA to negotiate TLS 1.0 and TLS 1.1 during - // HTTPS redirects. When it is set to false, the VA will only connect to - // HTTPS servers that support TLS 1.2 or above. - OldTLSOutbound - // OldTLSInbound controls whether the WFE rejects inbound requests using - // TLS 1.0 and TLS 1.1. Because WFE does not terminate TLS in production, - // we rely on the TLS-Version header (set by our reverse proxy). - OldTLSInbound // SHA1CSRs controls whether the /acme/finalize endpoint rejects CSRs that // are self-signed using SHA1. SHA1CSRs diff --git a/test.sh b/test.sh index 9941d5dd2..6b30a4b54 100755 --- a/test.sh +++ b/test.sh @@ -211,20 +211,15 @@ print_heading "Starting..." # STAGE="lints" if [[ "${RUN[@]}" =~ "$STAGE" ]] ; then - # TODO(#6275): Remove this conditional and globally re-enable this test. - if [[ $(go version) == *go1.19* ]] ; then - print_heading "Skipping Lints" - else - print_heading "Running Lints" - golangci-lint run --timeout 9m ./... - python3 test/grafana/lint.py - # Check for common spelling errors using codespell. - # Update .codespell.ignore.txt if you find false positives (NOTE: ignored - # words should be all lowercase). - run_and_expect_silence codespell \ - --ignore-words=.codespell.ignore.txt \ - --skip=.git,.gocache,go.sum,go.mod,vendor,bin,*.pyc,*.pem,*.der,*.resp,*.req,*.csr,.codespell.ignore.txt,.*.swp - fi + print_heading "Running Lints" + golangci-lint run --timeout 9m ./... + python3 test/grafana/lint.py + # Check for common spelling errors using codespell. + # Update .codespell.ignore.txt if you find false positives (NOTE: ignored + # words should be all lowercase). + run_and_expect_silence codespell \ + --ignore-words=.codespell.ignore.txt \ + --skip=.git,.gocache,go.sum,go.mod,vendor,bin,*.pyc,*.pem,*.der,*.resp,*.req,*.csr,.codespell.ignore.txt,.*.swp fi # @@ -276,24 +271,19 @@ fi # so will fail if imports are not available in $GOPATH. STAGE="generate" if [[ "${RUN[@]}" =~ "$STAGE" ]] ; then - # TODO(#6275): Remove this conditional and globally re-enable this test. - if [[ $(go version) == *go1.19* ]] ; then - print_heading "Skipping Generate" - else - print_heading "Running Generate" - # Additionally, we need to run go install before go generate because the stringer command - # (using in ./grpc/) checks imports, and depends on the presence of a built .a - # file to determine an import really exists. See - # https://golang.org/src/go/internal/gcimporter/gcimporter.go#L30 - # Without this, we get error messages like: - # stringer: checking package: grpc/bcodes.go:6:2: could not import - # github.com/letsencrypt/boulder/probs (can't find import: - # github.com/letsencrypt/boulder/probs) - go install ./probs - go install ./vendor/google.golang.org/grpc/codes - run_and_expect_silence go generate ./... - run_and_expect_silence git diff --exit-code . - fi + print_heading "Running Generate" + # Additionally, we need to run go install before go generate because the stringer command + # (using in ./grpc/) checks imports, and depends on the presence of a built .a + # file to determine an import really exists. See + # https://golang.org/src/go/internal/gcimporter/gcimporter.go#L30 + # Without this, we get error messages like: + # stringer: checking package: grpc/bcodes.go:6:2: could not import + # github.com/letsencrypt/boulder/probs (can't find import: + # github.com/letsencrypt/boulder/probs) + go install ./probs + go install ./vendor/google.golang.org/grpc/codes + run_and_expect_silence go generate ./... + run_and_expect_silence git diff --exit-code . fi STAGE="make-artifacts" diff --git a/va/caa_test.go b/va/caa_test.go index 317e2f099..852c65b47 100644 --- a/va/caa_test.go +++ b/va/caa_test.go @@ -760,47 +760,47 @@ func TestExtractIssuerDomainAndParameters(t *testing.T) { wantValid: true, }, { - value: " letsencrypt.org ;foo=bar;baz=bar", + value: " letsencrypt.org ;foo=bar;baz=bar", wantDomain: "letsencrypt.org", wantParameters: map[string]string{"foo": "bar", "baz": "bar"}, wantValid: true, }, { - value: " letsencrypt.org ;foo=bar;baz=bar", + value: " letsencrypt.org ;foo=bar;baz=bar", wantDomain: "letsencrypt.org", wantParameters: map[string]string{"foo": "bar", "baz": "bar"}, wantValid: true, }, { - value: "letsencrypt.org; foo=; baz = bar", + value: "letsencrypt.org; foo=; baz = bar", wantDomain: "letsencrypt.org", wantParameters: map[string]string{"foo": "", "baz": "bar"}, wantValid: true, }, { - value: "letsencrypt.org; foo= ; baz = bar", + value: "letsencrypt.org; foo= ; baz = bar", wantDomain: "letsencrypt.org", wantParameters: map[string]string{"foo": "", "baz": "bar"}, wantValid: true, }, { - value: "letsencrypt.org; foo=b1,b2,b3 ; baz = a=b ", + value: "letsencrypt.org; foo=b1,b2,b3 ; baz = a=b ", wantDomain: "letsencrypt.org", wantParameters: map[string]string{"foo": "b1,b2,b3", "baz": "a=b"}, wantValid: true, }, { - value: "letsencrypt.org; foo=b1,b2,b3 ; baz = a = b ", + value: "letsencrypt.org; foo=b1,b2,b3 ; baz = a = b ", wantDomain: "letsencrypt.org", wantValid: false, }, { - value: "letsencrypt.org; foo=b1,b2,b3 ; baz=a= b", + value: "letsencrypt.org; foo=b1,b2,b3 ; baz=a= b", wantDomain: "letsencrypt.org", wantValid: false, }, { - value: "letsencrypt.org; foo=b1,b2,b3 ; baz = a;b ", + value: "letsencrypt.org; foo=b1,b2,b3 ; baz = a;b ", wantDomain: "letsencrypt.org", wantValid: false, }, diff --git a/va/http.go b/va/http.go index 3f50d9569..d8ce0b331 100644 --- a/va/http.go +++ b/va/http.go @@ -16,7 +16,6 @@ import ( "github.com/letsencrypt/boulder/core" berrors "github.com/letsencrypt/boulder/errors" - "github.com/letsencrypt/boulder/features" "github.com/letsencrypt/boulder/iana" "github.com/letsencrypt/boulder/identifier" "github.com/letsencrypt/boulder/probs" @@ -497,7 +496,6 @@ func (va *ValidationAuthorityImpl) processHTTPValidation( // addresses explicitly, not following redirects to ports != [80,443], etc) records := []core.ValidationRecord{baseRecord} numRedirects := 0 - var oldTLS bool processRedirect := func(req *http.Request, via []*http.Request) error { va.log.Debugf("processing a HTTP redirect from the server to %q", req.URL.String()) // Only process up to maxRedirect redirects @@ -507,9 +505,11 @@ func (va *ValidationAuthorityImpl) processHTTPValidation( numRedirects++ va.metrics.http01Redirects.Inc() - // TODO(#6011): Remove once TLS 1.0 and 1.1 support is gone. if req.Response.TLS != nil && req.Response.TLS.Version < tls.VersionTLS12 { - oldTLS = true + return berrors.ConnectionFailureError( + "validation attempt was redirected to an HTTPS server that doesn't " + + "support TLSv1.2 or better. See " + + "https://community.letsencrypt.org/t/rejecting-sha-1-csrs-and-validation-using-tls-1-0-1-1-urls/175144") } // If the response contains an HTTP 303 or any other forbidden redirect, @@ -628,21 +628,6 @@ func (va *ValidationAuthorityImpl) processHTTPValidation( records[len(records)-1].URL, httpResponse.StatusCode)) } - // TODO(#6011): Remove once TLS 1.0 and 1.1 support is gone. - if httpResponse.TLS != nil && httpResponse.TLS.Version < tls.VersionTLS12 { - oldTLS = true - if !features.Enabled(features.OldTLSOutbound) { - return nil, records, berrors.MalformedError( - "validation attempt was redirected to an HTTPS server that doesn't " + - "support TLSv1.2 or better. See " + - "https://community.letsencrypt.org/t/rejecting-sha-1-csrs-and-validation-using-tls-1-0-1-1-urls/175144") - } - } - - if oldTLS { - records[len(records)-1].OldTLS = true - } - // At this point we've made a successful request (be it from a retry or // otherwise) and can read and process the response body. body, err := io.ReadAll(&io.LimitedReader{R: httpResponse.Body, N: maxResponseSize}) diff --git a/va/http_test.go b/va/http_test.go index 8c5ea51b9..808b43ee6 100644 --- a/va/http_test.go +++ b/va/http_test.go @@ -3,18 +3,15 @@ package va import ( "bytes" "context" - "crypto/tls" "encoding/base64" "errors" "fmt" - "io" mrand "math/rand" "net" "net/http" "net/http/httptest" "net/url" "regexp" - "runtime" "strconv" "strings" "time" @@ -23,7 +20,6 @@ import ( "github.com/letsencrypt/boulder/bdns" "github.com/letsencrypt/boulder/core" berrors "github.com/letsencrypt/boulder/errors" - "github.com/letsencrypt/boulder/features" "github.com/letsencrypt/boulder/identifier" "github.com/letsencrypt/boulder/probs" "github.com/letsencrypt/boulder/test" @@ -1546,107 +1542,3 @@ func TestLimitedReader(t *testing.T) { t.Errorf("Problem Detail contained an invalid UTF-8 string") } } - -// oldTLSRedirectSrv returns a pair of *httptest.Servers. The first one is -// plaintext and redirects all URLs to the second one. The second one is HTTPS -// and supports a max TLS version of 1.1. -// TODO(#6011): Remove once TLS 1.0 and 1.1 support is gone. -func oldTLSRedirectSrv(t *testing.T, token string) (*httptest.Server, *httptest.Server) { - var port int - m := http.NewServeMux() - addrs, err := net.LookupHost("example.com") - if err != nil { - t.Fatal(err) - } - if len(addrs) < 1 || addrs[0] != "127.0.0.1" { - t.Fatalf("this test requires example.com to resolve to 127.0.0.1 but it resolved to %s", addrs[0]) - } - m.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - http.Redirect(w, r, fmt.Sprintf("https://example.com:%d%s", port, r.URL.Path), http.StatusMovedPermanently) - }) - - server := httptest.NewUnstartedServer(m) - server.Start() - - httpsMux := http.NewServeMux() - httpsMux.HandleFunc("/.well-known/acme-challenge/", func(w http.ResponseWriter, r *http.Request) { - pathComponents := strings.Split(r.URL.Path, "/") - if len(pathComponents) < 4 || pathComponents[3] != token { - http.NotFound(w, r) - return - } - - ch := core.Challenge{Token: token} - keyAuthz, _ := ch.ExpectedKeyAuthorization(accountKey) - fmt.Fprint(w, keyAuthz) - }) - httpsServer := httptest.NewUnstartedServer(httpsMux) - httpsServer.TLS = &tls.Config{ - MaxVersion: tls.VersionTLS11, - } - httpsServer.StartTLS() - - port = getPort(httpsServer) - - return server, httpsServer -} - -// TODO(#6011): Remove once TLS 1.0 and 1.1 support is gone. -func TestOldTLS(t *testing.T) { - // Don't run this test with go1.19, which removes support for TLS 1.0 and 1.1. - if strings.Contains(runtime.Version(), "go1.19") { - return - } - - features.Reset() - - chall := httpChallenge() - server, httpsServer := oldTLSRedirectSrv(t, chall.Token) - startURL := server.URL - - // Check that the HTTP servers are running as expected - c := http.Client{ - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: true, - }, - }, - } - resp, err := c.Get(startURL + "/.well-known/acme-challenge/" + chall.Token) - if err != nil { - t.Fatal(err) - } - body, err := io.ReadAll(resp.Body) - if err != nil { - t.Fatal(err) - } - if !strings.Contains(string(body), chall.Token) { - t.Fatal("response did not contain token") - } - if resp.TLS == nil { - t.Fatal("request did not redirect to HTTPS") - } - if resp.TLS.Version > tls.VersionTLS11 { - t.Fatalf("HTTPS request negotiated TLS version 0x%x (test expected 1.1). Try setting GODEBUG=tls10default=1", - resp.TLS.Version) - } - - // The real test - va, _ := setup(server, 0, "", nil) - va.httpsPort = getPort(httpsServer) - - _, prob := va.validateHTTP01(ctx, dnsi("localhost"), chall) - if prob != nil { - t.Errorf("(OldTLSOutbound == true) expected success, got %s", prob) - } - - err = features.Set(map[string]bool{"OldTLSOutbound": false}) - if err != nil { - t.Fatal(err) - } - - _, prob = va.validateHTTP01(ctx, dnsi("localhost"), chall) - if prob == nil { - t.Error("(OldTLSOutbound == false) expected fail, got success") - } -} diff --git a/va/tlsalpn_test.go b/va/tlsalpn_test.go index 2260a3ca0..0b45a3c53 100644 --- a/va/tlsalpn_test.go +++ b/va/tlsalpn_test.go @@ -15,7 +15,6 @@ import ( "net/http" "net/http/httptest" "net/url" - "runtime" "strconv" "strings" "testing" @@ -769,14 +768,9 @@ func TestTLSALPN01ExtraSANs(t *testing.T) { _, prob := va.validateChallenge(ctx, dnsi("expected"), chall) test.AssertError(t, prob, "validation should have failed") - if strings.Contains(runtime.Version(), "go1.19") { - // In go >= 1.19, the TLS client library detects that the certificate has - // a duplicate extension and terminates the connection itself. - // TODO: Make this assertion unconditional and remove the else clause. - test.AssertContains(t, prob.Error(), "Error getting validation data") - } else { - test.AssertContains(t, prob.Error(), "Extension OID 2.5.29.17 seen twice") - } + // In go >= 1.19, the TLS client library detects that the certificate has + // a duplicate extension and terminates the connection itself. + test.AssertContains(t, prob.Error(), "Error getting validation data") } func TestTLSALPN01ExtraAcmeExtensions(t *testing.T) { @@ -829,14 +823,9 @@ func TestTLSALPN01ExtraAcmeExtensions(t *testing.T) { _, prob := va.validateChallenge(ctx, dnsi("expected"), chall) test.AssertError(t, prob, "validation should have failed") - if strings.Contains(runtime.Version(), "go1.19") { - // In go >= 1.19, the TLS client library detects that the certificate has - // a duplicate extension and terminates the connection itself. - // TODO: Make this assertion unconditional and remove the else clause. - test.AssertContains(t, prob.Error(), "Error getting validation data") - } else { - test.AssertContains(t, prob.Error(), "Extension OID 1.3.6.1.5.5.7.1.31 seen twice") - } + // In go >= 1.19, the TLS client library detects that the certificate has + // a duplicate extension and terminates the connection itself. + test.AssertContains(t, prob.Error(), "Error getting validation data") } func TestAcceptableExtensions(t *testing.T) { diff --git a/wfe2/wfe.go b/wfe2/wfe.go index af29d9af8..873d5daec 100644 --- a/wfe2/wfe.go +++ b/wfe2/wfe.go @@ -238,12 +238,10 @@ func (wfe *WebFrontEndImpl) HandleFunc(mux *http.ServeMux, pattern string, h web logEvent.Slug = request.URL.Path beeline.AddFieldToTrace(ctx, "slug", request.URL.Path) } - if !features.Enabled(features.OldTLSInbound) { - tls := request.Header.Get("TLS-Version") - if tls == "TLSv1" || tls == "TLSv1.1" { - wfe.sendError(response, logEvent, probs.Malformed("upgrade your ACME client to support TLSv1.2 or better"), nil) - return - } + tls := request.Header.Get("TLS-Version") + if tls == "TLSv1" || tls == "TLSv1.1" { + wfe.sendError(response, logEvent, probs.Malformed("upgrade your ACME client to support TLSv1.2 or better"), nil) + return } if request.Method != "GET" || pattern == newNoncePath { // Historically we did not return a error to the client diff --git a/wfe2/wfe_test.go b/wfe2/wfe_test.go index f5c5eb8a6..e73753678 100644 --- a/wfe2/wfe_test.go +++ b/wfe2/wfe_test.go @@ -3228,7 +3228,7 @@ func TestPrepAuthzForDisplay(t *testing.T) { Identifier: identifier.DNSIdentifier("*.example.com"), Challenges: []core.Challenge{ { - Type: "dns", + Type: "dns", ProvidedKeyAuthorization: " 🔑", }, }, @@ -3676,10 +3676,7 @@ func TestIncidentARI(t *testing.T) { test.AssertEquals(t, ri.SuggestedWindow.End.Before(wfe.clk.Now()), true) } -// TODO(#6011): Remove once TLS 1.0 and 1.1 support is gone. func TestOldTLSInbound(t *testing.T) { - features.Reset() - wfe, _ := setupWFE(t) req := &http.Request{ URL: &url.URL{Path: "/directory"}, @@ -3688,17 +3685,9 @@ func TestOldTLSInbound(t *testing.T) { http.CanonicalHeaderKey("TLS-Version"): {"TLSv1"}, }), } + responseWriter := httptest.NewRecorder() wfe.Handler(metrics.NoopRegisterer).ServeHTTP(responseWriter, req) - test.AssertEquals(t, responseWriter.Code, http.StatusOK) - - err := features.Set(map[string]bool{"OldTLSInbound": false}) - if err != nil { - t.Fatal(err) - } - - responseWriter = httptest.NewRecorder() - wfe.Handler(metrics.NoopRegisterer).ServeHTTP(responseWriter, req) test.AssertEquals(t, responseWriter.Code, http.StatusBadRequest) }