diff --git a/.github/workflows/boulder-ci.yml b/.github/workflows/boulder-ci.yml index 15121b1ea99..86e9b703a9b 100644 --- a/.github/workflows/boulder-ci.yml +++ b/.github/workflows/boulder-ci.yml @@ -36,7 +36,8 @@ jobs: matrix: # Add additional docker image tags here and all tests will be run with the additional image. BOULDER_TOOLS_TAG: - - go1.22.5_2024-07-03 + - go1.22.5_2024-08-13 + - go1.23.0_2024-08-13 # Tests command definitions. Use the entire "docker compose" command you want to run. tests: # Run ./test.sh --help for a description of each of the flags. diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 88d46d28e08..d3d4a341eef 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -16,6 +16,7 @@ jobs: matrix: GO_VERSION: - "1.22.5" + - "1.23.0" runs-on: ubuntu-20.04 permissions: contents: write diff --git a/.github/workflows/try-release.yml b/.github/workflows/try-release.yml index f2cb5700e10..76879f0df7f 100644 --- a/.github/workflows/try-release.yml +++ b/.github/workflows/try-release.yml @@ -16,6 +16,7 @@ jobs: matrix: GO_VERSION: - "1.22.5" + - "1.23.0" runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v4 diff --git a/cmd/cert-checker/main_test.go b/cmd/cert-checker/main_test.go index 2a7f7419bcb..1dc56093654 100644 --- a/cmd/cert-checker/main_test.go +++ b/cmd/cert-checker/main_test.go @@ -134,7 +134,7 @@ func TestCheckWildcardCert(t *testing.T) { } _, problems := checker.checkCert(context.Background(), cert, nil) for _, p := range problems { - t.Errorf(p) + t.Error(p) } } diff --git a/core/objects.go b/core/objects.go index 80743085ce9..9a8f25bb4a9 100644 --- a/core/objects.go +++ b/core/objects.go @@ -200,7 +200,7 @@ func (ch Challenge) ExpectedKeyAuthorization(key *jose.JSONWebKey) (string, erro // RecordsSane checks the sanity of a ValidationRecord object before sending it // back to the RA to be stored. func (ch Challenge) RecordsSane() bool { - if ch.ValidationRecord == nil || len(ch.ValidationRecord) == 0 { + if len(ch.ValidationRecord) == 0 { return false } diff --git a/docker-compose.yml b/docker-compose.yml index 5f2aa45232f..76fc8a80c23 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -8,7 +8,7 @@ services: context: test/boulder-tools/ # Should match one of the GO_CI_VERSIONS in test/boulder-tools/tag_and_upload.sh. args: - GO_VERSION: 1.22.5 + GO_VERSION: 1.23.0 environment: # To solve HTTP-01 and TLS-ALPN-01 challenges, change the IP in FAKE_DNS # to the IP address where your ACME client's solver is listening. diff --git a/grpc/errors.go b/grpc/errors.go index 7f9aabbb6cf..0dd0d843311 100644 --- a/grpc/errors.go +++ b/grpc/errors.go @@ -91,7 +91,7 @@ func unwrapError(err error, md metadata.MD) error { inErrMsg, ) } - inErr := berrors.New(berrors.ErrorType(inErrType), inErrMsg) + inErr := berrors.New(berrors.ErrorType(inErrType), inErrMsg) //nolint:govet var outErr *berrors.BoulderError if !errors.As(inErr, &outErr) { return fmt.Errorf( diff --git a/grpc/internal/resolver/dns/dns_resolver_test.go b/grpc/internal/resolver/dns/dns_resolver_test.go index 891fb970ede..3ec58417900 100644 --- a/grpc/internal/resolver/dns/dns_resolver_test.go +++ b/grpc/internal/resolver/dns/dns_resolver_test.go @@ -600,7 +600,7 @@ func TestCustomAuthority(t *testing.T) { err = <-errChan if err != nil { - t.Errorf(err.Error()) + t.Error(err.Error()) } if a.expectError { diff --git a/probs/probs.go b/probs/probs.go index 90ba0b78505..24c4c677e71 100644 --- a/probs/probs.go +++ b/probs/probs.go @@ -92,19 +92,19 @@ func AccountDoesNotExist(detail string) *ProblemDetails { // AlreadyRevoked returns a ProblemDetails with a AlreadyRevokedProblem and a 400 Bad // Request status code. -func AlreadyRevoked(detail string, a ...any) *ProblemDetails { +func AlreadyRevoked(detail string) *ProblemDetails { return &ProblemDetails{ Type: AlreadyRevokedProblem, - Detail: fmt.Sprintf(detail, a...), + Detail: detail, HTTPStatus: http.StatusBadRequest, } } // BadCSR returns a ProblemDetails representing a BadCSRProblem. -func BadCSR(detail string, a ...any) *ProblemDetails { +func BadCSR(detail string) *ProblemDetails { return &ProblemDetails{ Type: BadCSRProblem, - Detail: fmt.Sprintf(detail, a...), + Detail: detail, HTTPStatus: http.StatusBadRequest, } } @@ -121,30 +121,30 @@ func BadNonce(detail string) *ProblemDetails { // BadPublicKey returns a ProblemDetails with a BadPublicKeyProblem and a 400 Bad // Request status code. -func BadPublicKey(detail string, a ...any) *ProblemDetails { +func BadPublicKey(detail string) *ProblemDetails { return &ProblemDetails{ Type: BadPublicKeyProblem, - Detail: fmt.Sprintf(detail, a...), + Detail: detail, HTTPStatus: http.StatusBadRequest, } } // BadRevocationReason returns a ProblemDetails representing // a BadRevocationReasonProblem -func BadRevocationReason(detail string, a ...any) *ProblemDetails { +func BadRevocationReason(detail string) *ProblemDetails { return &ProblemDetails{ Type: BadRevocationReasonProblem, - Detail: fmt.Sprintf(detail, a...), + Detail: detail, HTTPStatus: http.StatusBadRequest, } } // BadSignatureAlgorithm returns a ProblemDetails with a BadSignatureAlgorithmProblem // and a 400 Bad Request status code. -func BadSignatureAlgorithm(detail string, a ...any) *ProblemDetails { +func BadSignatureAlgorithm(detail string) *ProblemDetails { return &ProblemDetails{ Type: BadSignatureAlgorithmProblem, - Detail: fmt.Sprintf(detail, a...), + Detail: detail, HTTPStatus: http.StatusBadRequest, } } @@ -200,10 +200,10 @@ func Malformed(detail string, a ...any) *ProblemDetails { } // OrderNotReady returns a ProblemDetails representing a OrderNotReadyProblem -func OrderNotReady(detail string, a ...any) *ProblemDetails { +func OrderNotReady(detail string) *ProblemDetails { return &ProblemDetails{ Type: OrderNotReadyProblem, - Detail: fmt.Sprintf(detail, a...), + Detail: detail, HTTPStatus: http.StatusForbidden, } } diff --git a/ra/ra.go b/ra/ra.go index 13b636447ab..513f2d4444f 100644 --- a/ra/ra.go +++ b/ra/ra.go @@ -874,10 +874,10 @@ func (ra *RegistrationAuthorityImpl) checkOrderAuthorizations( func validatedBefore(authz *core.Authorization, caaRecheckTime time.Time) (bool, error) { numChallenges := len(authz.Challenges) if numChallenges != 1 { - return false, fmt.Errorf("authorization has incorrect number of challenges. 1 expected, %d found for: id %s", numChallenges, authz.ID) + return false, berrors.InternalServerError("authorization has incorrect number of challenges. 1 expected, %d found for: id %s", numChallenges, authz.ID) } if authz.Challenges[0].Validated == nil { - return false, fmt.Errorf("authorization's challenge has no validated timestamp for: id %s", authz.ID) + return false, berrors.InternalServerError("authorization's challenge has no validated timestamp for: id %s", authz.ID) } return authz.Challenges[0].Validated.Before(caaRecheckTime), nil } @@ -905,7 +905,7 @@ func (ra *RegistrationAuthorityImpl) checkAuthorizationsCAA( for _, authz := range authzs { if staleCAA, err := validatedBefore(authz, caaRecheckAfter); err != nil { - return berrors.InternalServerError(err.Error()) + return err } else if staleCAA { // Ensure that CAA is rechecked for this name recheckAuthzs = append(recheckAuthzs, authz) @@ -977,7 +977,7 @@ func (ra *RegistrationAuthorityImpl) recheckCAA(ctx context.Context, authzs []*c authz.ID, name, ) } else if resp.Problem != nil { - err = berrors.CAAError(resp.Problem.Detail) + err = berrors.CAAError("rechecking caa: %s", resp.Problem.Detail) } ch <- authzCAAResult{ authz: authz, @@ -1460,16 +1460,13 @@ func (ra *RegistrationAuthorityImpl) getSCTs(ctx context.Context, cert []byte, e started := ra.clk.Now() scts, err := ra.ctpolicy.GetSCTs(ctx, cert, expiration) took := ra.clk.Since(started) - // The final cert has already been issued so actually return it to the - // user even if this fails since we aren't actually doing anything with - // the SCTs yet. if err != nil { state := "failure" if err == context.DeadlineExceeded { state = "deadlineExceeded" // Convert the error to a missingSCTsError to communicate the timeout, // otherwise it will be a generic serverInternalError - err = berrors.MissingSCTsError(err.Error()) + err = berrors.MissingSCTsError("failed to get SCTs: %s", err.Error()) } ra.log.Warningf("ctpolicy.GetSCTs failed: %s", err) ra.ctpolicyResults.With(prometheus.Labels{"result": state}).Observe(took.Seconds()) @@ -1942,11 +1939,11 @@ func (ra *RegistrationAuthorityImpl) PerformValidation( // Look up the account key for this authorization regPB, err := ra.SA.GetRegistration(ctx, &sapb.RegistrationID{Id: authz.RegistrationID}) if err != nil { - return nil, berrors.InternalServerError(err.Error()) + return nil, berrors.InternalServerError("getting acct for authorization: %s", err.Error()) } reg, err := bgrpc.PbToRegistration(regPB) if err != nil { - return nil, berrors.InternalServerError(err.Error()) + return nil, berrors.InternalServerError("getting acct for authorization: %s", err.Error()) } // Compute the key authorization field based on the registration key @@ -1957,7 +1954,7 @@ func (ra *RegistrationAuthorityImpl) PerformValidation( // Double check before sending to VA if cErr := ch.CheckPending(); cErr != nil { - return nil, berrors.MalformedError(cErr.Error()) + return nil, berrors.MalformedError("cannot validate challenge: %s", cErr.Error()) } // Dispatch to the VA for service @@ -2463,7 +2460,7 @@ func (ra *RegistrationAuthorityImpl) DeactivateRegistration(ctx context.Context, } _, err := ra.SA.DeactivateRegistration(ctx, &sapb.RegistrationID{Id: reg.Id}) if err != nil { - return nil, berrors.InternalServerError(err.Error()) + return nil, err } return &emptypb.Empty{}, nil } diff --git a/sfe/sfe_test.go b/sfe/sfe_test.go index 3f79440feae..556f0e9d5c0 100644 --- a/sfe/sfe_test.go +++ b/sfe/sfe_test.go @@ -123,7 +123,7 @@ func TestUnpausePaths(t *testing.T) { fc.Add(time.Hour * 337) sfe.UnpauseForm(responseWriter, &http.Request{ Method: "GET", - URL: mustParseURL(fmt.Sprintf(unpause.GetForm + "?jwt=" + expiredJWT)), + URL: mustParseURL(unpause.GetForm + "?jwt=" + expiredJWT), }) test.AssertEquals(t, responseWriter.Code, http.StatusOK) test.AssertContains(t, responseWriter.Body.String(), "Expired unpause URL") @@ -134,7 +134,7 @@ func TestUnpausePaths(t *testing.T) { responseWriter = httptest.NewRecorder() sfe.UnpauseForm(responseWriter, &http.Request{ Method: "GET", - URL: mustParseURL(fmt.Sprintf(unpause.GetForm + "?jwt=" + validJWT)), + URL: mustParseURL(unpause.GetForm + "?jwt=" + validJWT), }) test.AssertEquals(t, responseWriter.Code, http.StatusOK) test.AssertContains(t, responseWriter.Body.String(), "Action required to unpause your account") @@ -143,7 +143,7 @@ func TestUnpausePaths(t *testing.T) { responseWriter = httptest.NewRecorder() sfe.UnpauseSubmit(responseWriter, &http.Request{ Method: "POST", - URL: mustParseURL(fmt.Sprintf(unpausePostForm + "?jwt=" + expiredJWT)), + URL: mustParseURL(unpausePostForm + "?jwt=" + expiredJWT), }) test.AssertEquals(t, responseWriter.Code, http.StatusOK) test.AssertContains(t, responseWriter.Body.String(), "Expired unpause URL") @@ -161,7 +161,7 @@ func TestUnpausePaths(t *testing.T) { responseWriter = httptest.NewRecorder() sfe.UnpauseSubmit(responseWriter, &http.Request{ Method: "POST", - URL: mustParseURL(fmt.Sprintf(unpausePostForm + "?jwt=x.x")), + URL: mustParseURL(unpausePostForm + "?jwt=x.x"), }) test.AssertEquals(t, responseWriter.Code, http.StatusOK) test.AssertContains(t, responseWriter.Body.String(), "Invalid unpause URL") @@ -170,7 +170,7 @@ func TestUnpausePaths(t *testing.T) { responseWriter = httptest.NewRecorder() sfe.UnpauseSubmit(responseWriter, &http.Request{ Method: "POST", - URL: mustParseURL(fmt.Sprintf(unpausePostForm + "?jwt=x.x.x")), + URL: mustParseURL(unpausePostForm + "?jwt=x.x.x"), }) test.AssertEquals(t, responseWriter.Code, http.StatusOK) test.AssertContains(t, responseWriter.Body.String(), "Invalid unpause URL") @@ -179,7 +179,7 @@ func TestUnpausePaths(t *testing.T) { responseWriter = httptest.NewRecorder() sfe.UnpauseSubmit(responseWriter, &http.Request{ Method: "POST", - URL: mustParseURL(fmt.Sprintf(unpausePostForm + "?jwt=" + validJWT)), + URL: mustParseURL(unpausePostForm + "?jwt=" + validJWT), }) test.AssertEquals(t, responseWriter.Code, http.StatusFound) test.AssertEquals(t, unpauseStatus+"?count=0", responseWriter.Result().Header.Get("Location")) diff --git a/test/boulder-tools/Dockerfile b/test/boulder-tools/Dockerfile index 3e3680b5522..1dbbbf88b4a 100644 --- a/test/boulder-tools/Dockerfile +++ b/test/boulder-tools/Dockerfile @@ -12,9 +12,9 @@ RUN curl "https://dl.google.com/go/go${GO_VERSION}.$(echo $TARGETPLATFORM | sed RUN go install github.com/rubenv/sql-migrate/sql-migrate@v1.1.2 RUN go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.34.1 RUN go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@bb9882e6ae58f0a80a6390b50a5ec3bd63e46a3c -RUN go install github.com/letsencrypt/pebble/v2/cmd/pebble-challtestsrv@66511d8 -RUN go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.57.2 -RUN go install honnef.co/go/tools/cmd/staticcheck@2023.1.7 +RUN go install github.com/letsencrypt/pebble/v2/cmd/pebble-challtestsrv@17d64a3 +RUN go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.60.0 +RUN go install honnef.co/go/tools/cmd/staticcheck@2024.1 RUN go install github.com/jsha/minica@v1.1.0 FROM rust:bullseye as rustdeps diff --git a/test/boulder-tools/tag_and_upload.sh b/test/boulder-tools/tag_and_upload.sh index 330c7651380..ea9cdd2704f 100755 --- a/test/boulder-tools/tag_and_upload.sh +++ b/test/boulder-tools/tag_and_upload.sh @@ -12,7 +12,7 @@ DOCKER_REPO="letsencrypt/boulder-tools" # .github/workflows/release.yml, # .github/workflows/try-release.yml if appropriate, # and .github/workflows/boulder-ci.yml with the new container tag. -GO_CI_VERSIONS=( "1.22.5" ) +GO_CI_VERSIONS=( "1.22.5" "1.23.0" ) echo "Please login to allow push to DockerHub" docker login diff --git a/va/http_test.go b/va/http_test.go index 82dfcfcf627..a9edd8a4c00 100644 --- a/va/http_test.go +++ b/va/http_test.go @@ -53,10 +53,23 @@ func TestDialerMismatchError(t *testing.T) { test.AssertEquals(t, err.Error(), expectedErr.Error()) } -// TestPreresolvedDialerTimeout tests that the preresolvedDialer's DialContext +// dnsMockReturnsUnroutable is a DNSClient mock that always returns an +// unroutable address for LookupHost. This is useful in testing connect +// timeouts. +type dnsMockReturnsUnroutable struct { + *bdns.MockClient +} + +func (mock dnsMockReturnsUnroutable) LookupHost(_ context.Context, hostname string) ([]net.IP, bdns.ResolverAddrs, error) { + return []net.IP{net.ParseIP("198.51.100.1")}, bdns.ResolverAddrs{"dnsMockReturnsUnroutable"}, nil +} + +// TestDialerTimeout tests that the preresolvedDialer's DialContext // will timeout after the expected singleDialTimeout. This ensures timeouts at -// the TCP level are handled correctly. -func TestPreresolvedDialerTimeout(t *testing.T) { +// the TCP level are handled correctly. It also ensures that we show the client +// the appropriate "Timeout during connect" error message, which helps clients +// distinguish between firewall problems and server problems. +func TestDialerTimeout(t *testing.T) { va, _ := setup(nil, 0, "", nil, nil) // Timeouts below 50ms tend to be flaky. va.singleDialTimeout = 50 * time.Millisecond @@ -77,7 +90,7 @@ func TestPreresolvedDialerTimeout(t *testing.T) { started := time.Now() _, _, err = va.fetchHTTP(ctx, "unroutable.invalid", "/.well-known/acme-challenge/whatever") took = time.Since(started) - if err != nil && strings.Contains(err.Error(), "Network unreachable") { + if err != nil && strings.Contains(err.Error(), "network is unreachable") { continue } else { break @@ -97,13 +110,7 @@ func TestPreresolvedDialerTimeout(t *testing.T) { } prob := detailedError(err) test.AssertEquals(t, prob.Type, probs.ConnectionProblem) - - expectMatch := regexp.MustCompile( - "Fetching http://unroutable.invalid/.well-known/acme-challenge/.*: Timeout during connect") - if !expectMatch.MatchString(prob.Detail) { - t.Errorf("Problem details incorrect. Got %q, expected to match %q", - prob.Detail, expectMatch) - } + test.AssertContains(t, prob.Detail, "Timeout during connect (likely firewall problem)") } func TestHTTPTransport(t *testing.T) { @@ -1348,64 +1355,6 @@ func TestHTTPTimeout(t *testing.T) { test.AssertEquals(t, prob.Detail, "127.0.0.1: Fetching http://localhost/.well-known/acme-challenge/wait-long: Timeout after connect (your server may be slow or overloaded)") } -// dnsMockReturnsUnroutable is a DNSClient mock that always returns an -// unroutable address for LookupHost. This is useful in testing connect -// timeouts. -type dnsMockReturnsUnroutable struct { - *bdns.MockClient -} - -func (mock dnsMockReturnsUnroutable) LookupHost(_ context.Context, hostname string) ([]net.IP, bdns.ResolverAddrs, error) { - return []net.IP{net.ParseIP("198.51.100.1")}, bdns.ResolverAddrs{"dnsMockReturnsUnroutable"}, nil -} - -// TestHTTPDialTimeout tests that we give the proper "Timeout during connect" -// error when dial fails. We do this by using a mock DNS client that resolves -// everything to an unroutable IP address. -func TestHTTPDialTimeout(t *testing.T) { - va, _ := setup(nil, 0, "", nil, nil) - - started := time.Now() - timeout := 250 * time.Millisecond - ctx, cancel := context.WithTimeout(context.Background(), timeout) - defer cancel() - - va.dnsClient = dnsMockReturnsUnroutable{&bdns.MockClient{}} - // The only method I've found so far to trigger a connect timeout is to - // connect to an unrouteable IP address. This usually generates a connection - // timeout, but will rarely return "Network unreachable" instead. If we get - // that, just retry until we get something other than "Network unreachable". - var err error - for range 20 { - _, err = va.validateHTTP01(ctx, dnsi("unroutable.invalid"), expectedToken, expectedKeyAuthorization) - if err != nil && strings.Contains(err.Error(), "network is unreachable") { - continue - } else { - break - } - } - if err == nil { - t.Fatalf("Connection should've timed out") - } - took := time.Since(started) - // Check that the HTTP connection doesn't return too fast, and times - // out after the expected time - if took < (timeout-200*time.Millisecond)/2 { - t.Fatalf("HTTP returned before %s (%s) with %q", timeout, took, err.Error()) - } - if took > 2*timeout { - t.Fatalf("HTTP connection didn't timeout after %s seconds", timeout) - } - prob := detailedError(err) - test.AssertEquals(t, prob.Type, probs.ConnectionProblem) - expectMatch := regexp.MustCompile( - "Fetching http://unroutable.invalid/.well-known/acme-challenge/.*: Timeout during connect") - if !expectMatch.MatchString(prob.Detail) { - t.Errorf("Problem details incorrect. Got %q, expected to match %q", - prob.Detail, expectMatch) - } -} - func TestHTTPRedirectLookup(t *testing.T) { hs := httpSrv(t, expectedToken) defer hs.Close() diff --git a/web/send_error.go b/web/send_error.go index c0e68d70731..f071b1db33f 100644 --- a/web/send_error.go +++ b/web/send_error.go @@ -47,7 +47,7 @@ func SendError( logEvent.Error += fmt.Sprintf(" [%s]", strings.Join(subDetails, ", ")) } if ierr != nil { - logEvent.AddError(fmt.Sprintf("%s", ierr)) + logEvent.AddError("%s", ierr) } // Set the proper namespace for the problem and any sub-problems. diff --git a/wfe2/wfe.go b/wfe2/wfe.go index 6083080d972..7620954e749 100644 --- a/wfe2/wfe.go +++ b/wfe2/wfe.go @@ -802,7 +802,7 @@ func (wfe *WebFrontEndImpl) NewAccount( return } } - wfe.log.Warningf(err.Error()) + wfe.log.Warning(err.Error()) } var newRegistrationSuccessful bool @@ -932,11 +932,11 @@ func (wfe *WebFrontEndImpl) parseRevocation( if !ok { reasonStr = "unknown" } - return nil, 0, probs.BadRevocationReason( + return nil, 0, probs.BadRevocationReason(fmt.Sprintf( "unsupported revocation reason code provided: %s (%d). Supported reasons: %s", reasonStr, *revokeRequest.Reason, - revocation.UserAllowedReasonsMessage) + revocation.UserAllowedReasonsMessage)) } reason = *revokeRequest.Reason } @@ -2384,7 +2384,7 @@ func (wfe *WebFrontEndImpl) NewOrder( return } } - wfe.log.Warningf(err.Error()) + wfe.log.Warning(err.Error()) } var newOrderSuccessful bool @@ -2582,11 +2582,7 @@ func (wfe *WebFrontEndImpl) FinalizeOrder(ctx context.Context, logEvent *web.Req // Only ready orders can be finalized. if order.Status != string(core.StatusReady) { - wfe.sendError(response, logEvent, - probs.OrderNotReady( - "Order's status (%q) is not acceptable for finalization", - order.Status), - nil) + wfe.sendError(response, logEvent, probs.OrderNotReady(fmt.Sprintf("Order's status (%q) is not acceptable for finalization", order.Status)), nil) return }