Skip to content

Commit

Permalink
Merge branch 'main' into newnewtxnbuilder
Browse files Browse the repository at this point in the history
  • Loading branch information
jprenken authored Dec 7, 2024
2 parents aa6a651 + 071b8c5 commit 5c9ac63
Show file tree
Hide file tree
Showing 23 changed files with 251 additions and 225 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/boulder-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
# tags and 5 tests there would be 10 jobs run.
b:
# The type of runner that the job will run on
runs-on: ubuntu-20.04
runs-on: ubuntu-24.04

strategy:
# When set to true, GitHub cancels all in-progress jobs if any matrix job fails. Default: true
Expand Down Expand Up @@ -95,7 +95,7 @@ jobs:
run: ${{ matrix.tests }}

govulncheck:
runs-on: ubuntu-22.04
runs-on: ubuntu-24.04
strategy:
fail-fast: false

Expand All @@ -117,7 +117,7 @@ jobs:
run: go run golang.org/x/vuln/cmd/govulncheck@latest ./...

vendorcheck:
runs-on: ubuntu-20.04
runs-on: ubuntu-24.04
strategy:
# When set to true, GitHub cancels all in-progress jobs if any matrix job fails. Default: true
fail-fast: false
Expand Down Expand Up @@ -153,7 +153,7 @@ jobs:
permissions:
contents: none
if: ${{ always() }}
runs-on: ubuntu-latest
runs-on: ubuntu-24.04
name: Boulder CI Test Matrix
needs:
- b
Expand Down
20 changes: 20 additions & 0 deletions cmd/admin/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"context"
"errors"
"fmt"

"github.com/jmhodges/clock"
Expand Down Expand Up @@ -94,3 +95,22 @@ func newAdmin(configFile string, dryRun bool) (*admin, error) {
log: logger,
}, nil
}

// findActiveInputMethodFlag returns a single key from setInputs with a value of `true`,
// if exactly one exists. Otherwise it returns an error.
func findActiveInputMethodFlag(setInputs map[string]bool) (string, error) {
var activeFlags []string
for flag, isSet := range setInputs {
if isSet {
activeFlags = append(activeFlags, flag)
}
}

if len(activeFlags) == 0 {
return "", errors.New("at least one input method flag must be specified")
} else if len(activeFlags) > 1 {
return "", fmt.Errorf("more than one input method flag specified: %v", activeFlags)
}

return activeFlags[0], nil
}
59 changes: 59 additions & 0 deletions cmd/admin/admin_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package main

import (
"testing"

"github.com/letsencrypt/boulder/test"
)

func Test_findActiveInputMethodFlag(t *testing.T) {
tests := []struct {
name string
setInputs map[string]bool
expected string
wantErr bool
}{
{
name: "No active flags",
setInputs: map[string]bool{
"-private-key": false,
"-spki-file": false,
"-cert-file": false,
},
expected: "",
wantErr: true,
},
{
name: "Multiple active flags",
setInputs: map[string]bool{
"-private-key": true,
"-spki-file": true,
"-cert-file": false,
},
expected: "",
wantErr: true,
},
{
name: "Single active flag",
setInputs: map[string]bool{
"-private-key": true,
"-spki-file": false,
"-cert-file": false,
},
expected: "-private-key",
wantErr: false,
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
result, err := findActiveInputMethodFlag(tc.setInputs)
if tc.wantErr {
test.AssertError(t, err, "findActiveInputMethodFlag() should have errored")
} else {
test.AssertNotError(t, err, "findActiveInputMethodFlag() should not have errored")
test.AssertEquals(t, result, tc.expected)
}
})
}
}
12 changes: 4 additions & 8 deletions cmd/admin/cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"unicode"

"golang.org/x/crypto/ocsp"
"golang.org/x/exp/maps"

core "github.com/letsencrypt/boulder/core"
berrors "github.com/letsencrypt/boulder/errors"
Expand Down Expand Up @@ -109,16 +108,13 @@ func (s *subcommandRevokeCert) Run(ctx context.Context, a *admin) error {
"-reg-id": s.regID != 0,
"-cert-file": s.certFile != "",
}
maps.DeleteFunc(setInputs, func(_ string, v bool) bool { return !v })
if len(setInputs) == 0 {
return errors.New("at least one input method flag must be specified")
} else if len(setInputs) > 1 {
return fmt.Errorf("more than one input method flag specified: %v", maps.Keys(setInputs))
activeFlag, err := findActiveInputMethodFlag(setInputs)
if err != nil {
return err
}

var serials []string
var err error
switch maps.Keys(setInputs)[0] {
switch activeFlag {
case "-serial":
serials, err = []string{s.serial}, nil
case "-incident-table":
Expand Down
12 changes: 4 additions & 8 deletions cmd/admin/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"sync"
"sync/atomic"

"golang.org/x/exp/maps"
"google.golang.org/protobuf/types/known/timestamppb"

"github.com/letsencrypt/boulder/core"
Expand Down Expand Up @@ -69,16 +68,13 @@ func (s *subcommandBlockKey) Run(ctx context.Context, a *admin) error {
"-cert-file": s.certFile != "",
"-csr-file": s.csrFile != "",
}
maps.DeleteFunc(setInputs, func(_ string, v bool) bool { return !v })
if len(setInputs) == 0 {
return errors.New("at least one input method flag must be specified")
} else if len(setInputs) > 1 {
return fmt.Errorf("more than one input method flag specified: %v", maps.Keys(setInputs))
activeFlag, err := findActiveInputMethodFlag(setInputs)
if err != nil {
return err
}

var spkiHashes [][]byte
var err error
switch maps.Keys(setInputs)[0] {
switch activeFlag {
case "-private-key":
var spkiHash []byte
spkiHash, err = a.spkiHashFromPrivateKey(s.privKey)
Expand Down
12 changes: 4 additions & 8 deletions cmd/admin/unpause_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (

sapb "github.com/letsencrypt/boulder/sa/proto"
"github.com/letsencrypt/boulder/unpause"
"golang.org/x/exp/maps"
)

// subcommandUnpauseAccount encapsulates the "admin unpause-account" command.
Expand Down Expand Up @@ -44,16 +43,13 @@ func (u *subcommandUnpauseAccount) Run(ctx context.Context, a *admin) error {
"-account": u.accountID != 0,
"-batch-file": u.batchFile != "",
}
maps.DeleteFunc(setInputs, func(_ string, v bool) bool { return !v })
if len(setInputs) == 0 {
return errors.New("at least one input method flag must be specified")
} else if len(setInputs) > 1 {
return fmt.Errorf("more than one input method flag specified: %v", maps.Keys(setInputs))
activeFlag, err := findActiveInputMethodFlag(setInputs)
if err != nil {
return err
}

var regIDs []int64
var err error
switch maps.Keys(setInputs)[0] {
switch activeFlag {
case "-account":
regIDs = []int64{u.accountID}
case "-batch-file":
Expand Down
4 changes: 2 additions & 2 deletions cmd/boulder-va/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ type RemoteVAGRPCClientConfig struct {
// - RIPE
// - APNIC
// - LACNIC
// - AfriNIC
// - AFRINIC
//
// TODO(#7615): Make mandatory.
RIR string `validate:"omitempty,oneof=ARIN RIPE APNIC LACNIC AfriNIC"`
RIR string `validate:"omitempty,oneof=ARIN RIPE APNIC LACNIC AFRINIC"`
}

type Config struct {
Expand Down
6 changes: 3 additions & 3 deletions cmd/remoteva/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type Config struct {
// of RVAs deployed in the same datacenter.
//
// TODO(#7615): Make mandatory.
Perspective string `validate:"omitempty"`
Perspective string `omitempty:"omitempty"`

// RIR indicates the Regional Internet Registry where this RVA is
// located. This field is used to identify the RIR region from which a
Expand All @@ -38,10 +38,10 @@ type Config struct {
// - RIPE
// - APNIC
// - LACNIC
// - AfriNIC
// - AFRINIC
//
// TODO(#7615): Make mandatory.
RIR string `validate:"omitempty,oneof=ARIN RIPE APNIC LACNIC AfriNIC"`
RIR string `validate:"omitempty,oneof=ARIN RIPE APNIC LACNIC AFRINIC"`

// SkipGRPCClientCertVerification, when disabled as it should typically
// be, will cause the remoteva server (which receives gRPCs from a
Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ require (
go.opentelemetry.io/otel/sdk v1.30.0
go.opentelemetry.io/otel/trace v1.30.0
golang.org/x/crypto v0.27.0
golang.org/x/exp v0.0.0-20240112132812-db7319d0e0e3
golang.org/x/net v0.29.0
golang.org/x/sync v0.8.0
golang.org/x/term v0.24.0
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,6 @@ golang.org/x/crypto v0.23.0/go.mod h1:CKFgDieR+mRhux2Lsu27y0fO304Db0wZe70UKqHu0v
golang.org/x/crypto v0.26.0/go.mod h1:GY7jblb9wI+FOo5y8/S2oY4zWP07AkOJ4+jxCqdqn54=
golang.org/x/crypto v0.27.0 h1:GXm2NjJrPaiv/h1tb2UH8QfgC/hOf/+z0p6PT8o1w7A=
golang.org/x/crypto v0.27.0/go.mod h1:1Xngt8kV6Dvbssa53Ziq6Eqn0HqbZi5Z6R0ZpwQzt70=
golang.org/x/exp v0.0.0-20240112132812-db7319d0e0e3 h1:hNQpMuAJe5CtcUqCXaWga3FHu+kQvCqcsoVaQgSV60o=
golang.org/x/exp v0.0.0-20240112132812-db7319d0e0e3/go.mod h1:idGWGoKP1toJGkd5/ig9ZLuPcZBC3ewk7SzmH0uou08=
golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=
golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
Expand Down
4 changes: 3 additions & 1 deletion test/config/remoteva-a.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@
"accountURIPrefixes": [
"http://boulder.service.consul:4000/acme/reg/",
"http://boulder.service.consul:4001/acme/acct/"
]
],
"perspective": "dadaist",
"rir": "ARIN"
},
"syslog": {
"stdoutlevel": 4,
Expand Down
4 changes: 3 additions & 1 deletion test/config/remoteva-b.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@
"accountURIPrefixes": [
"http://boulder.service.consul:4000/acme/reg/",
"http://boulder.service.consul:4001/acme/acct/"
]
],
"perspective": "surrealist",
"rir": "RIPE"
},
"syslog": {
"stdoutlevel": 4,
Expand Down
4 changes: 3 additions & 1 deletion test/config/remoteva-c.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@
"accountURIPrefixes": [
"http://boulder.service.consul:4000/acme/reg/",
"http://boulder.service.consul:4001/acme/acct/"
]
],
"perspective": "cubist",
"rir": "ARIN"
},
"syslog": {
"stdoutlevel": 4,
Expand Down
12 changes: 9 additions & 3 deletions test/config/va.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,23 @@
{
"serverAddress": "rva1.service.consul:9397",
"timeout": "15s",
"hostOverride": "rva1.boulder"
"hostOverride": "rva1.boulder",
"perspective": "dadaist",
"rir": "ARIN"
},
{
"serverAddress": "rva1.service.consul:9498",
"timeout": "15s",
"hostOverride": "rva1.boulder"
"hostOverride": "rva1.boulder",
"perspective": "surrealist",
"rir": "RIPE"
},
{
"serverAddress": "rva1.service.consul:9499",
"timeout": "15s",
"hostOverride": "rva1.boulder"
"hostOverride": "rva1.boulder",
"perspective": "cubist",
"rir": "ARIN"
}
],
"maxRemoteValidationFailures": 1,
Expand Down
17 changes: 12 additions & 5 deletions va/caa.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,19 @@ func (va *ValidationAuthorityImpl) IsCAAValid(ctx context.Context, req *vapb.IsC
// It will also later be serialized in JSON, which defaults to UTF-8. Make
// sure it is UTF-8 clean now.
prob = filterProblemDetails(prob)
return &vapb.IsCAAValidResponse{Problem: &corepb.ProblemDetails{
ProblemType: string(prob.Type),
Detail: replaceInvalidUTF8([]byte(prob.Detail)),
}}, nil
return &vapb.IsCAAValidResponse{
Problem: &corepb.ProblemDetails{
ProblemType: string(prob.Type),
Detail: replaceInvalidUTF8([]byte(prob.Detail)),
},
Perspective: va.perspective,
Rir: va.rir,
}, nil
} else {
return &vapb.IsCAAValidResponse{}, nil
return &vapb.IsCAAValidResponse{
Perspective: va.perspective,
Rir: va.rir,
}, nil
}
}

Expand Down
17 changes: 15 additions & 2 deletions va/va.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ func NewValidationAuthorityImpl(

for i, va1 := range remoteVAs {
for j, va2 := range remoteVAs {
// TODO(#7819): Remove the != "" check once perspective is required.
// TODO(#7615): Remove the != "" check once perspective is required.
if i != j && va1.Perspective == va2.Perspective && va1.Perspective != "" {
return nil, fmt.Errorf("duplicate remote VA perspective %q", va1.Perspective)
}
Expand Down Expand Up @@ -504,6 +504,19 @@ func (va *ValidationAuthorityImpl) performRemoteOperation(ctx context.Context, o
for _, i := range rand.Perm(remoteVACount) {
go func(rva RemoteVA) {
res, err := op(subCtx, rva, req)
if err != nil {
responses <- &response{rva.Address, rva.Perspective, rva.RIR, res, err}
return
}
// TODO(#7615): Remove the != "" checks once perspective and rir are required.
if (rva.Perspective != "" && res.GetPerspective() != "" && res.GetPerspective() != rva.Perspective) ||
(rva.RIR != "" && res.GetRir() != "" && res.GetRir() != rva.RIR) {
err = fmt.Errorf(
"Expected perspective %q (%q) but got reply from %q (%q) - misconfiguration likely", rva.Perspective, rva.RIR, res.GetPerspective(), res.GetRir(),
)
responses <- &response{rva.Address, rva.Perspective, rva.RIR, res, err}
return
}
responses <- &response{rva.Address, rva.Perspective, rva.RIR, res, err}
}(va.remoteVAs[i])
}
Expand Down Expand Up @@ -538,7 +551,7 @@ func (va *ValidationAuthorityImpl) performRemoteOperation(ctx context.Context, o
}
if isCAACheck {
// We're checking CAA, log the problem.
va.log.Errf("Operation on Remote VA (%s) returned a problem: %s", resp.addr, err)
va.log.Errf("Operation on Remote VA (%s) returned a problem: %s", resp.addr, currProb)
}
} else {
// The remote VA returned a successful result.
Expand Down
Loading

0 comments on commit 5c9ac63

Please sign in to comment.