Skip to content

Commit

Permalink
Log the Perspective and RIR when present, do not require at this time.
Browse files Browse the repository at this point in the history
  • Loading branch information
beautifulentropy committed Oct 9, 2024
1 parent dba1629 commit c3dd91f
Show file tree
Hide file tree
Showing 13 changed files with 227 additions and 118 deletions.
4 changes: 3 additions & 1 deletion cmd/boulder-va/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,9 @@ func main() {
scope,
clk,
logger,
c.VA.AccountURIPrefixes)
c.VA.AccountURIPrefixes,
va.PrimaryPerspective,
"")
cmd.FailOnError(err, "Unable to create VA server")

start, err := bgrpc.NewServer(c.VA.GRPC, logger).Add(
Expand Down
26 changes: 15 additions & 11 deletions cmd/remoteva/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,27 @@ type Config struct {
RVA struct {
vaConfig.Common

// Cluster specifies the cluster name that this RVA belongs to. The
// format is unrestricted, but it should uniquely identify a group of
// RVAs deployed in the same physical datacenter.
// Perspective uniquely identifies the Network Perspective used to
// perform the validation, as specified in BRs Section 5.4.1,
// Requirement 2.7 ("Multi-Perspective Issuance Corroboration attempts
// from each Network Perspective"). It should uniquely identify a group
// of RVAs deployed in the same datacenter.
//
// TODO(#7615): Make mandatory once referenced in audit logs. Update the
// comment above.
Cluster string `validate:"omitempty"`
// TODO(#7615): Make mandatory.
Perspective string `validate:"omitempty"`

// RIR indicates the Regional Internet Registry where this RVA is
// located. This will be used to to identify which RIR a given
// validation was performed from. Must be one of the following values:
// located. This field is used to identify the RIR region from which a
// given validation was performed, as specified in the "Phased
// Implementation Timeline" in BRs Section 3.2.2.9. It must be one of
// the following values:
// - ARIN
// - RIPE
// - APNIC
// - LACNIC
// - AfriNIC
//
// TODO(#7615): Make mandatory once referenced in audit logs. Update the
// comment above.
// TODO(#7615): Make mandatory.
RIR string `validate:"omitempty,oneof=ARIN RIPE APNIC LACNIC AfriNIC"`

// SkipGRPCClientCertVerification, when disabled as it should typically
Expand Down Expand Up @@ -139,7 +141,9 @@ func main() {
scope,
clk,
logger,
c.RVA.AccountURIPrefixes)
c.RVA.AccountURIPrefixes,
c.RVA.Perspective,
c.RVA.RIR)
cmd.FailOnError(err, "Unable to create Remote-VA server")

start, err := bgrpc.NewServer(c.RVA.GRPC, logger).Add(
Expand Down
20 changes: 20 additions & 0 deletions core/objects.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ type ValidationRecord struct {
Port string `json:"port,omitempty"`
AddressesResolved []net.IP `json:"addressesResolved,omitempty"`
AddressUsed net.IP `json:"addressUsed,omitempty"`

// AddressesTried contains a list of addresses tried before the `AddressUsed`.
// Presently this will only ever be one IP from `AddressesResolved` since the
// only retry is in the case of a v6 failure with one v4 fallback. E.g. if
Expand All @@ -144,10 +145,29 @@ type ValidationRecord struct {
// ...
// }
AddressesTried []net.IP `json:"addressesTried,omitempty"`

// ResolverAddrs is the host:port of the DNS resolver(s) that fulfilled the
// lookup for AddressUsed. During recursive A and AAAA lookups, a record may
// instead look like A:host:port or AAAA:host:port
ResolverAddrs []string `json:"resolverAddrs,omitempty"`

// Perspective uniquely identifies the Network Perspective used to perform
// the validation, as specified in BRs Section 5.4.1, Requirement 2.7
// ("Multi-Perspective Issuance Corroboration attempts from each Network
// Perspective"). It should uniquely identify either the Primary Perspective
// (VA) or a group of RVAs deployed in the same datacenter.
Perspective string `json:"perspective,omitempty"`

// RIR indicates the Regional Internet Registry where this RVA is located.
// This field is used to identify the RIR region from which a given
// validation was performed, as specified in the "Phased Implementation
// Timeline" in BRs Section 3.2.2.9. It must be one of the following values:
// - ARIN
// - RIPE
// - APNIC
// - LACNIC
// - AfriNIC
RIR string `json:"rir,omitempty"`
}

// Challenge is an aggregate of all data needed for any challenges.
Expand Down
2 changes: 1 addition & 1 deletion test/config-next/remoteva-a.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
"http://boulder.service.consul:4000/acme/reg/",
"http://boulder.service.consul:4001/acme/acct/"
],
"cluster": "development",
"perspective": "development",
"rir": "ARIN"
},
"syslog": {
Expand Down
4 changes: 2 additions & 2 deletions test/config-next/remoteva-b.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@
"http://boulder.service.consul:4000/acme/reg/",
"http://boulder.service.consul:4001/acme/acct/"
],
"cluster": "development",
"rir": "ARIN"
"perspective": "development",
"rir": "RIPE"
},
"syslog": {
"stdoutlevel": 4,
Expand Down
24 changes: 12 additions & 12 deletions va/caa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func (mock caaMockDNS) LookupCAA(_ context.Context, domain string) ([]*dns.CAA,
}

func TestCAATimeout(t *testing.T) {
va, _ := setup(nil, 0, "", nil, caaMockDNS{})
va, _ := setup(nil, 0, "", nil, caaMockDNS{}, "", "")

params := &caaParams{
accountURIID: 12345,
Expand Down Expand Up @@ -407,7 +407,7 @@ func TestCAAChecking(t *testing.T) {
method := core.ChallengeTypeHTTP01
params := &caaParams{accountURIID: accountURIID, validationMethod: method}

va, _ := setup(nil, 0, "", nil, caaMockDNS{})
va, _ := setup(nil, 0, "", nil, caaMockDNS{}, "", "")
va.accountURIPrefixes = []string{"https://letsencrypt.org/acct/reg/"}

for _, caaTest := range testCases {
Expand All @@ -430,7 +430,7 @@ func TestCAAChecking(t *testing.T) {
}

func TestCAALogging(t *testing.T) {
va, _ := setup(nil, 0, "", nil, caaMockDNS{})
va, _ := setup(nil, 0, "", nil, caaMockDNS{}, "", "")

testCases := []struct {
Name string
Expand Down Expand Up @@ -520,7 +520,7 @@ func TestCAALogging(t *testing.T) {
// TestIsCAAValidErrMessage tests that an error result from `va.IsCAAValid`
// includes the domain name that was being checked in the failure detail.
func TestIsCAAValidErrMessage(t *testing.T) {
va, _ := setup(nil, 0, "", nil, caaMockDNS{})
va, _ := setup(nil, 0, "", nil, caaMockDNS{}, "", "")

// Call IsCAAValid with a domain we know fails with a generic error from the
// caaMockDNS.
Expand All @@ -545,7 +545,7 @@ func TestIsCAAValidErrMessage(t *testing.T) {
// which do not have the necessary parameters to do CAA Account and Method
// Binding checks.
func TestIsCAAValidParams(t *testing.T) {
va, _ := setup(nil, 0, "", nil, caaMockDNS{})
va, _ := setup(nil, 0, "", nil, caaMockDNS{}, "", "")

// Calling IsCAAValid without a ValidationMethod should fail.
_, err := va.IsCAAValid(ctx, &vapb.IsCAAValidRequest{
Expand Down Expand Up @@ -589,9 +589,9 @@ func (b caaBrokenDNS) LookupCAA(_ context.Context, domain string) ([]*dns.CAA, s
}

func TestDisabledMultiCAARechecking(t *testing.T) {
brokenRVA := setupRemote(nil, "broken", caaBrokenDNS{})
brokenRVA := setupRemote(nil, "broken", caaBrokenDNS{}, "", "")
remoteVAs := []RemoteVA{{brokenRVA, "broken"}}
va, _ := setup(nil, 0, "local", remoteVAs, nil)
va, _ := setup(nil, 0, "local", remoteVAs, nil, "", "")

features.Set(features.Config{
EnforceMultiCAA: false,
Expand Down Expand Up @@ -663,10 +663,10 @@ func TestMultiCAARechecking(t *testing.T) {
brokenUA = "broken"
hijackedUA = "hijacked"
)
remoteVA := setupRemote(nil, remoteUA, nil)
brokenVA := setupRemote(nil, brokenUA, caaBrokenDNS{})
remoteVA := setupRemote(nil, remoteUA, nil, "", "")
brokenVA := setupRemote(nil, brokenUA, caaBrokenDNS{}, "", "")
// Returns incorrect results
hijackedVA := setupRemote(nil, hijackedUA, caaHijackedDNS{})
hijackedVA := setupRemote(nil, hijackedUA, caaHijackedDNS{}, "", "")

testCases := []struct {
name string
Expand Down Expand Up @@ -899,7 +899,7 @@ func TestMultiCAARechecking(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
va, mockLog := setup(nil, tc.maxLookupFailures, localUA, tc.remoteVAs, tc.localDNSClient)
va, mockLog := setup(nil, tc.maxLookupFailures, localUA, tc.remoteVAs, tc.localDNSClient, "", "")
defer mockLog.Clear()

// MultiCAAFullResults: false is inherently flaky because of the
Expand Down Expand Up @@ -962,7 +962,7 @@ func TestCAAFailure(t *testing.T) {
hs := httpSrv(t, expectedToken)
defer hs.Close()

va, _ := setup(hs, 0, "", nil, caaMockDNS{})
va, _ := setup(hs, 0, "", nil, caaMockDNS{}, "", "")

err := va.checkCAA(ctx, dnsi("reserved.com"), &caaParams{1, core.ChallengeTypeHTTP01})
if err == nil {
Expand Down
20 changes: 10 additions & 10 deletions va/dns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
)

func TestDNSValidationEmpty(t *testing.T) {
va, _ := setup(nil, 0, "", nil, nil)
va, _ := setup(nil, 0, "", nil, nil, "", "")

// This test calls PerformValidation directly, because that is where the
// metrics checked below are incremented.
Expand All @@ -36,7 +36,7 @@ func TestDNSValidationEmpty(t *testing.T) {
}

func TestDNSValidationWrong(t *testing.T) {
va, _ := setup(nil, 0, "", nil, nil)
va, _ := setup(nil, 0, "", nil, nil, "", "")
_, err := va.validateDNS01(context.Background(), dnsi("wrong-dns01.com"), expectedKeyAuthorization)
if err == nil {
t.Fatalf("Successful DNS validation with wrong TXT record")
Expand All @@ -46,7 +46,7 @@ func TestDNSValidationWrong(t *testing.T) {
}

func TestDNSValidationWrongMany(t *testing.T) {
va, _ := setup(nil, 0, "", nil, nil)
va, _ := setup(nil, 0, "", nil, nil, "", "")

_, err := va.validateDNS01(context.Background(), dnsi("wrong-many-dns01.com"), expectedKeyAuthorization)
if err == nil {
Expand All @@ -57,7 +57,7 @@ func TestDNSValidationWrongMany(t *testing.T) {
}

func TestDNSValidationWrongLong(t *testing.T) {
va, _ := setup(nil, 0, "", nil, nil)
va, _ := setup(nil, 0, "", nil, nil, "", "")

_, err := va.validateDNS01(context.Background(), dnsi("long-dns01.com"), expectedKeyAuthorization)
if err == nil {
Expand All @@ -68,7 +68,7 @@ func TestDNSValidationWrongLong(t *testing.T) {
}

func TestDNSValidationFailure(t *testing.T) {
va, _ := setup(nil, 0, "", nil, nil)
va, _ := setup(nil, 0, "", nil, nil, "", "")

_, err := va.validateDNS01(ctx, dnsi("localhost"), expectedKeyAuthorization)
prob := detailedError(err)
Expand All @@ -82,7 +82,7 @@ func TestDNSValidationInvalid(t *testing.T) {
Value: "790DB180-A274-47A4-855F-31C428CB1072",
}

va, _ := setup(nil, 0, "", nil, nil)
va, _ := setup(nil, 0, "", nil, nil, "", "")

_, err := va.validateDNS01(ctx, notDNS, expectedKeyAuthorization)
prob := detailedError(err)
Expand All @@ -91,7 +91,7 @@ func TestDNSValidationInvalid(t *testing.T) {
}

func TestDNSValidationServFail(t *testing.T) {
va, _ := setup(nil, 0, "", nil, nil)
va, _ := setup(nil, 0, "", nil, nil, "", "")

_, err := va.validateDNS01(ctx, dnsi("servfail.com"), expectedKeyAuthorization)

Expand All @@ -100,7 +100,7 @@ func TestDNSValidationServFail(t *testing.T) {
}

func TestDNSValidationNoServer(t *testing.T) {
va, log := setup(nil, 0, "", nil, nil)
va, log := setup(nil, 0, "", nil, nil, "", "")
staticProvider, err := bdns.NewStaticProvider([]string{})
test.AssertNotError(t, err, "Couldn't make new static provider")

Expand All @@ -119,15 +119,15 @@ func TestDNSValidationNoServer(t *testing.T) {
}

func TestDNSValidationOK(t *testing.T) {
va, _ := setup(nil, 0, "", nil, nil)
va, _ := setup(nil, 0, "", nil, nil, "", "")

_, prob := va.validateDNS01(ctx, dnsi("good-dns01.com"), expectedKeyAuthorization)

test.Assert(t, prob == nil, "Should be valid.")
}

func TestDNSValidationNoAuthorityOK(t *testing.T) {
va, _ := setup(nil, 0, "", nil, nil)
va, _ := setup(nil, 0, "", nil, nil, "", "")

_, prob := va.validateDNS01(ctx, dnsi("no-authority-dns01.com"), expectedKeyAuthorization)

Expand Down
Loading

0 comments on commit c3dd91f

Please sign in to comment.