Skip to content

Commit

Permalink
cleanup(enginenetx): remove previous policies (#1593)
Browse files Browse the repository at this point in the history 
In #1592 and previous pull
requests, I replaced the policies that embedded mixing logic with
neutral policies and external mixing logic, which enabled me to
implement what was requested in the
#1552 pull request review. Now,
with this pull request, I am cleaning up, by removing the policies that
we were previously using. Work part of
ooni/probe#2704.

---------

Co-authored-by: Arturo Filastò <arturo@filasto.net>
  • Loading branch information
@bassosimone @hellais
bassosimone and hellais committed May 9, 2024
1 parent 17e74f4 commit 9f77da7
Show file tree
Hide file tree
Showing 8 changed files with 0 additions and 1,313 deletions.
61 changes: 0 additions & 61 deletions internal/enginenetx/bridgespolicy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package enginenetx
import (
"context"
"math/rand"
"slices"
"time"
)

Expand All @@ -32,66 +31,6 @@ func (p *bridgesPolicyV2) LookupTactics(ctx context.Context, domain, port string
return bridgesTacticsForDomain(domain, port)
}

// bridgesPolicy is a policy where we use bridges for communicating
// with the OONI backend, i.e., api.ooni.io.
//
// A bridge is an IP address that can route traffic from and to
// the OONI backend and accepts any SNI.
//
// The zero value is invalid; please, init MANDATORY fields.
type bridgesPolicy struct {
// Fallback is the MANDATORY fallback policy.
Fallback httpsDialerPolicy
}

var _ httpsDialerPolicy = &bridgesPolicy{}

// LookupTactics implements httpsDialerPolicy.
func (p *bridgesPolicy) LookupTactics(ctx context.Context, domain, port string) <-chan *httpsDialerTactic {
return mixSequentially(
// emit bridges related tactics first which are empty if there are
// no bridges for the givend domain and port
bridgesTacticsForDomain(domain, port),

// now fallback to get more tactics (typically here the fallback
// uses the DNS and obtains some extra tactics)
//
// we wrap whatever the underlying policy returns us with some
// extra logic for better communicating with test helpers
p.maybeRewriteTestHelpersTactics(p.Fallback.LookupTactics(ctx, domain, port)),
)
}

func (p *bridgesPolicy) maybeRewriteTestHelpersTactics(input <-chan *httpsDialerTactic) <-chan *httpsDialerTactic {
out := make(chan *httpsDialerTactic)

go func() {
defer close(out) // tell the parent when we're done

for tactic := range input {
// When we're not connecting to a TH, pass the policy down the chain unmodified
if !slices.Contains(testHelpersDomains, tactic.VerifyHostname) {
out <- tactic
continue
}

// This is the case where we're connecting to a test helper. Let's try
// to produce policies hiding the SNI to censoring middleboxes.
for _, sni := range bridgesDomainsInRandomOrder() {
out <- &httpsDialerTactic{
Address: tactic.Address,
InitialDelay: 0, // set when dialing
Port: tactic.Port,
SNI: sni,
VerifyHostname: tactic.VerifyHostname,
}
}
}
}()

return out
}

func bridgesTacticsForDomain(domain, port string) <-chan *httpsDialerTactic {
out := make(chan *httpsDialerTactic)

Expand Down
240 changes: 0 additions & 240 deletions internal/enginenetx/bridgespolicy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,7 @@ package enginenetx

import (
"context"
"errors"
"testing"

"github.com/ooni/probe-cli/v3/internal/mocks"
"github.com/ooni/probe-cli/v3/internal/model"
)

func TestBridgesPolicyV2(t *testing.T) {
Expand Down Expand Up @@ -63,239 +59,3 @@ func TestBridgesPolicyV2(t *testing.T) {
}
})
}

func TestBridgesPolicy(t *testing.T) {
t.Run("for domains for which we don't have bridges and DNS failure", func(t *testing.T) {
expected := errors.New("mocked error")
p := &bridgesPolicy{
Fallback: &dnsPolicy{
Logger: model.DiscardLogger,
Resolver: &mocks.Resolver{
MockLookupHost: func(ctx context.Context, domain string) ([]string, error) {
return nil, expected
},
},
},
}

ctx := context.Background()
tactics := p.LookupTactics(ctx, "www.example.com", "443")

var count int
for range tactics {
count++
}

if count != 0 {
t.Fatal("expected to see zero tactics")
}
})

t.Run("for domains for which we don't have bridges and DNS success", func(t *testing.T) {
p := &bridgesPolicy{
Fallback: &dnsPolicy{
Logger: model.DiscardLogger,
Resolver: &mocks.Resolver{
MockLookupHost: func(ctx context.Context, domain string) ([]string, error) {
return []string{"93.184.216.34"}, nil
},
},
},
}

ctx := context.Background()
tactics := p.LookupTactics(ctx, "www.example.com", "443")

var count int
for tactic := range tactics {
count++

if tactic.Port != "443" {
t.Fatal("the port should always be 443")
}
if tactic.Address != "93.184.216.34" {
t.Fatal("the host should always be 93.184.216.34")
}

if tactic.InitialDelay != 0 {
t.Fatal("unexpected InitialDelay")
}

if tactic.SNI != "www.example.com" {
t.Fatal("the SNI field should always be like `www.example.com`")
}

if tactic.VerifyHostname != "www.example.com" {
t.Fatal("the VerifyHostname field should always be like `www.example.com`")
}
}

if count != 1 {
t.Fatal("expected to see one tactic")
}
})

t.Run("for the api.ooni.io domain with DNS failure", func(t *testing.T) {
expected := errors.New("mocked error")
p := &bridgesPolicy{
Fallback: &dnsPolicy{
Logger: model.DiscardLogger,
Resolver: &mocks.Resolver{
MockLookupHost: func(ctx context.Context, domain string) ([]string, error) {
return nil, expected
},
},
},
}

ctx := context.Background()
tactics := p.LookupTactics(ctx, "api.ooni.io", "443")

// since the DNS fails, we should only see tactics generated by bridges
var count int
for tactic := range tactics {
count++

if tactic.Port != "443" {
t.Fatal("the port should always be 443")
}
if tactic.Address != "162.55.247.208" {
t.Fatal("the host should always be 162.55.247.208")
}

if tactic.InitialDelay != 0 {
t.Fatal("unexpected InitialDelay")
}

if tactic.SNI == "api.ooni.io" {
t.Fatal("we should not see the `api.ooni.io` SNI on the wire")
}

if tactic.VerifyHostname != "api.ooni.io" {
t.Fatal("the VerifyHostname field should always be like `api.ooni.io`")
}
}

if count <= 0 {
t.Fatal("expected to see at least one tactic")
}
})

t.Run("for the api.ooni.io domain with DNS success", func(t *testing.T) {
p := &bridgesPolicy{
Fallback: &dnsPolicy{
Logger: model.DiscardLogger,
Resolver: &mocks.Resolver{
MockLookupHost: func(ctx context.Context, domain string) ([]string, error) {
return []string{"130.192.91.211"}, nil
},
},
},
}

ctx := context.Background()
tactics := p.LookupTactics(ctx, "api.ooni.io", "443")

// since the DNS succeeds we should see bridge tactics mixed with DNS tactics
var (
bridgesCount int
dnsCount int
overallCount int
)
const expectedDNSEntryCount = 153 // yikes!
for tactic := range tactics {
overallCount++

t.Log(overallCount, tactic)

if tactic.Port != "443" {
t.Fatal("the port should always be 443")
}

switch {
case overallCount == expectedDNSEntryCount:
if tactic.Address != "130.192.91.211" {
t.Fatal("the host should be 130.192.91.211 for count ==", expectedDNSEntryCount)
}

if tactic.SNI != "api.ooni.io" {
t.Fatal("we should see the `api.ooni.io` SNI on the wire for count ==", expectedDNSEntryCount)
}

dnsCount++

default:
if tactic.Address != "162.55.247.208" {
t.Fatal("the host should be 162.55.247.208 for count !=", expectedDNSEntryCount)
}

if tactic.SNI == "api.ooni.io" {
t.Fatal("we should not see the `api.ooni.io` SNI on the wire for count !=", expectedDNSEntryCount)
}

bridgesCount++
}

if tactic.InitialDelay != 0 {
t.Fatal("unexpected InitialDelay")
}

if tactic.VerifyHostname != "api.ooni.io" {
t.Fatal("the VerifyHostname field should always be like `api.ooni.io`")
}
}

if overallCount <= 0 {
t.Fatal("expected to see at least one tactic")
}
if dnsCount != 1 {
t.Fatal("expected to see exactly one DNS based tactic")
}
if bridgesCount <= 0 {
t.Fatal("expected to see at least one bridge tactic")
}
})

t.Run("for test helper domains", func(t *testing.T) {
for _, domain := range testHelpersDomains {
t.Run(domain, func(t *testing.T) {
expectedAddrs := []string{"164.92.180.7"}

p := &bridgesPolicy{
Fallback: &dnsPolicy{
Logger: model.DiscardLogger,
Resolver: &mocks.Resolver{
MockLookupHost: func(ctx context.Context, domain string) ([]string, error) {
return expectedAddrs, nil
},
},
},
}

ctx := context.Background()
for tactic := range p.LookupTactics(ctx, domain, "443") {

if tactic.Address != "164.92.180.7" {
t.Fatal("unexpected .Address")
}

if tactic.InitialDelay != 0 {
t.Fatal("unexpected .InitialDelay")
}

if tactic.Port != "443" {
t.Fatal("unexpected .Port")
}

if tactic.SNI == domain {
t.Fatal("unexpected .Domain")
}

if tactic.VerifyHostname != domain {
t.Fatal("unexpected .VerifyHostname")
}
}
})
}
})
}

0 comments on commit 9f77da7

Please sign in to comment.