Skip to content

Commit

Permalink
VA: Remove logging of RIR and Perspective (#7818)
Browse files Browse the repository at this point in the history
  • Loading branch information
@beautifulentropy
beautifulentropy authored Nov 15, 2024
1 parent 56f0ed6 commit 2502113
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 58 deletions.
8 changes: 4 additions & 4 deletions va/caa_test.go
View file
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
14 changes: 0 additions & 14 deletions va/va.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,6 @@ 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 @@ -678,18 +676,6 @@ 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
}

va.metrics.localValidationTime.With(prometheus.Labels{
"type": string(logEvent.Challenge.Type),
"result": string(logEvent.Challenge.Status),
Expand Down
55 changes: 15 additions & 40 deletions va/va_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,12 +148,12 @@ func setup(srv *httptest.Server, maxRemoteFailures int, userAgent string, remote
return va, logger
}

func setupRemote(srv *httptest.Server, userAgent string, mockDNSClientOverride bdns.Client, perspective, rir string) (RemoteClients, *blog.Mock) {
rva, log := setup(srv, 0, userAgent, nil, mockDNSClientOverride)
func setupRemote(srv *httptest.Server, userAgent string, mockDNSClientOverride bdns.Client, perspective, rir string) RemoteClients {
rva, _ := setup(srv, 0, userAgent, nil, mockDNSClientOverride)
rva.perspective = perspective
rva.rir = rir

return RemoteClients{VAClient: &inMemVA{*rva}, CAAClient: &inMemVA{*rva}}, log
return RemoteClients{VAClient: &inMemVA{*rva}, CAAClient: &inMemVA{*rva}}
}

type multiSrv struct {
Expand Down Expand Up @@ -373,8 +373,8 @@ func TestMultiVA(t *testing.T) {
ms := httpMultiSrv(t, expectedToken, allowedUAs)
defer ms.Close()

remoteVA1, _ := setupRemote(ms.Server, remoteUA1, nil, "", "")
remoteVA2, _ := setupRemote(ms.Server, remoteUA2, nil, "", "")
remoteVA1 := setupRemote(ms.Server, remoteUA1, nil, "", "")
remoteVA2 := setupRemote(ms.Server, remoteUA2, nil, "", "")
remoteVAs := []RemoteVA{
{remoteVA1, remoteUA1},
{remoteVA2, remoteUA2},
Expand Down Expand Up @@ -511,8 +511,8 @@ func TestMultiVAEarlyReturn(t *testing.T) {
ms := httpMultiSrv(t, expectedToken, allowedUAs)
defer ms.Close()

remoteVA1, _ := setupRemote(ms.Server, remoteUA1, nil, "", "")
remoteVA2, _ := setupRemote(ms.Server, remoteUA2, nil, "", "")
remoteVA1 := setupRemote(ms.Server, remoteUA1, nil, "", "")
remoteVA2 := setupRemote(ms.Server, remoteUA2, nil, "", "")

remoteVAs := []RemoteVA{
{remoteVA1, remoteUA1},
Expand Down Expand Up @@ -561,8 +561,8 @@ func TestMultiVAPolicy(t *testing.T) {
ms := httpMultiSrv(t, expectedToken, allowedUAs)
defer ms.Close()

remoteVA1, _ := setupRemote(ms.Server, remoteUA1, nil, "", "")
remoteVA2, _ := setupRemote(ms.Server, remoteUA2, nil, "", "")
remoteVA1 := setupRemote(ms.Server, remoteUA1, nil, "", "")
remoteVA2 := setupRemote(ms.Server, remoteUA2, nil, "", "")

remoteVAs := []RemoteVA{
{remoteVA1, remoteUA1},
Expand Down Expand Up @@ -591,43 +591,18 @@ func TestMultiVALogging(t *testing.T) {
ms := httpMultiSrv(t, expectedToken, map[string]bool{localUA: true, rva1UA: true, rva2UA: true})
defer ms.Close()

rva1, rva1Log := setupRemote(ms.Server, rva1UA, nil, "dev-arin", "ARIN")
rva2, rva2Log := setupRemote(ms.Server, rva2UA, nil, "dev-ripe", "RIPE")
rva1 := setupRemote(ms.Server, rva1UA, nil, "dev-arin", "ARIN")
rva2 := setupRemote(ms.Server, rva2UA, nil, "dev-ripe", "RIPE")

remoteVAs := []RemoteVA{
{rva1, rva1UA},
{rva2, rva2UA},
}
va, vaLog := setup(ms.Server, 0, localUA, remoteVAs, nil)
va, _ := setup(ms.Server, 0, localUA, remoteVAs, nil)
req := createValidationRequest("letsencrypt.org", core.ChallengeTypeHTTP01)
res, err := va.PerformValidation(ctx, req)
test.Assert(t, res.Problems == nil, fmt.Sprintf("validation failed with: %#v", res.Problems))
test.AssertNotError(t, err, "performing validation")

// We do not log perspective or RIR for the local VAs.
// We expect these log lines to be available immediately.
test.Assert(t, len(vaLog.GetAllMatching(`"Perspective"`)) == 0, "expected no logged perspective for primary")
test.Assert(t, len(vaLog.GetAllMatching(`"RIR"`)) == 0, "expected no logged RIR for primary")

// We do log perspective and RIR for the remote VAs.
//
// Because the remote VAs are operating on different goroutines, we aren't guaranteed their
// log lines have arrived yet. Give it a few tries.
for i := 0; i < 10; i++ {
if len(rva1Log.GetAllMatching(`"Perspective":"dev-arin"`)) >= 1 &&
len(rva1Log.GetAllMatching(`"RIR":"ARIN"`)) >= 1 &&
len(rva2Log.GetAllMatching(`"Perspective":"dev-ripe"`)) >= 1 &&
len(rva2Log.GetAllMatching(`"RIR":"RIPE"`)) >= 1 {
break
}
if i == 9 {
t.Logf("VA:\n%s\n", strings.Join(vaLog.GetAll(), "\n"))
t.Logf("RVA 1:\n%s\n", strings.Join(rva1Log.GetAll(), "\n"))
t.Logf("RVA 2:\n%s\n", strings.Join(rva2Log.GetAll(), "\n"))
t.Errorf("expected perspective and RIR logs for remote VAs, but they never arrived")
}
time.Sleep(100 * time.Millisecond)
}
}

func TestDetailedError(t *testing.T) {
Expand Down Expand Up @@ -684,9 +659,9 @@ func TestDetailedError(t *testing.T) {

func TestLogRemoteDifferentials(t *testing.T) {
// Create some remote VAs
remoteVA1, _ := setupRemote(nil, "remote 1", nil, "", "")
remoteVA2, _ := setupRemote(nil, "remote 2", nil, "", "")
remoteVA3, _ := setupRemote(nil, "remote 3", nil, "", "")
remoteVA1 := setupRemote(nil, "remote 1", nil, "", "")
remoteVA2 := setupRemote(nil, "remote 2", nil, "", "")
remoteVA3 := setupRemote(nil, "remote 3", nil, "", "")
remoteVAs := []RemoteVA{
{remoteVA1, "remote 1"},
{remoteVA2, "remote 2"},
Expand Down

0 comments on commit 2502113

Please sign in to comment.