Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VA/RVA: Add metadata necessary for the MPIC ballot #7732

Merged
merged 2 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
27 changes: 26 additions & 1 deletion cmd/remoteva/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,29 @@ type Config struct {
RVA struct {
vaConfig.Common

// 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.
Perspective string `validate:"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
//
// TODO(#7615): Make mandatory.
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
// boulder-va client) to use our default RequireAndVerifyClientCert
Expand Down Expand Up @@ -118,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
4 changes: 3 additions & 1 deletion test/config-next/remoteva-a.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@
"accountURIPrefixes": [
"http://boulder.service.consul:4000/acme/reg/",
"http://boulder.service.consul:4001/acme/acct/"
]
],
"perspective": "development",
"rir": "ARIN"
},
"syslog": {
"stdoutlevel": 4,
Expand Down
4 changes: 3 additions & 1 deletion test/config-next/remoteva-b.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@
"accountURIPrefixes": [
"http://boulder.service.consul:4000/acme/reg/",
"http://boulder.service.consul:4001/acme/acct/"
]
],
"perspective": "development",
beautifulentropy marked this conversation as resolved.
Show resolved Hide resolved
"rir": "RIPE"
},
"syslog": {
"stdoutlevel": 4,
Expand Down
8 changes: 4 additions & 4 deletions va/caa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ 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)

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
65 changes: 42 additions & 23 deletions va/proto/va.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions va/proto/va.proto
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,6 @@ message AuthzMeta {
message ValidationResult {
repeated core.ValidationRecord records = 1;
core.ProblemDetails problems = 2;
string perspective = 3;
string rir = 4;
}
22 changes: 22 additions & 0 deletions va/va.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import (
vapb "github.com/letsencrypt/boulder/va/proto"
)

const PrimaryPerspective = "Primary"

var (
// badTLSHeader contains the string 'HTTP /' which is returned when
// we try to talk TLS to a server that only talks HTTP
Expand Down Expand Up @@ -256,6 +258,8 @@ type ValidationAuthorityImpl struct {
maxRemoteFailures int
accountURIPrefixes []string
singleDialTimeout time.Duration
perspective string
rir string

metrics *vaMetrics
}
Expand All @@ -274,6 +278,8 @@ func NewValidationAuthorityImpl(
clk clock.Clock,
logger blog.Logger,
accountURIPrefixes []string,
perspective string,
rir string,
) (*ValidationAuthorityImpl, error) {

if len(accountURIPrefixes) == 0 {
Expand All @@ -300,6 +306,8 @@ func NewValidationAuthorityImpl(
// used for the DialContext operations that take place during an
// HTTP-01 challenge validation.
singleDialTimeout: 10 * time.Second,
perspective: perspective,
rir: rir,
}

return va, nil
Expand All @@ -314,6 +322,8 @@ type verificationRequestEvent struct {
ValidationLatency float64
Error string `json:",omitempty"`
InternalError string `json:",omitempty"`
Perspective string `json:",omitempty"`
RIR string `json:",omitempty"`
}

// ipError is an error type used to pass though the IP address of the remote
Expand Down Expand Up @@ -668,6 +678,18 @@ func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, req *v
logEvent.Challenge.Status = core.StatusValid
}

if va.perspective != "" && va.perspective != PrimaryPerspective {
// This validation was performed by a remote VA. According to the
// requirements in section 5.4.1 (2) vii of the BRs we need to log
// the perspective used. Additionally, we'll log the RIR where this
// RVA is located.
//
// TODO(#7615): Make these fields mandatory for non-Primary
// perspectives and remove the (va.perspective != "") check.
logEvent.Perspective = va.perspective
logEvent.RIR = va.rir
}
beautifulentropy marked this conversation as resolved.
Show resolved Hide resolved

va.metrics.localValidationTime.With(prometheus.Labels{
"type": string(logEvent.Challenge.Type),
"result": string(logEvent.Challenge.Status),
Expand Down
Loading