Skip to content

Commit

Permalink
refactor(enginenetx): make static/loadable policy easier to use (ooni…
Browse files Browse the repository at this point in the history
…#1297)

This diff makes the policy previously known as loadable and now know as
the static policy easier to use, by having a constructor that reads from
a key-value store and by passing it a fallback policy to use.

With this design, it should be possible to have code that uses the
static policy if applied, falling back to whatever policy we are
otherwise constructing into the NewNetwork constructor.

In a subsequent commit, I will hook this code into NewNetwork, so that
we can override the policy by changing the filesystem.

While there, notice several failures in the test suite and apply
workarounds (see ooni/probe#2539,
ooni/probe#2540,
ooni/probe#2541).

Part of ooni/probe#2531.
  • Loading branch information
bassosimone authored and Murphy-OrangeMud committed Feb 13, 2024
1 parent 2492145 commit d9f1366
Show file tree
Hide file tree
Showing 10 changed files with 342 additions and 150 deletions.
1 change: 1 addition & 0 deletions internal/engine/experiment_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ func TestRunTelegram(t *testing.T) {
}

func TestRunTor(t *testing.T) {
t.Skip("https://github.com/ooni/probe/issues/2539")
if testing.Short() {
t.Skip("skip test in short mode")
}
Expand Down
126 changes: 0 additions & 126 deletions internal/enginenetx/httpsdialer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package enginenetx_test
import (
"context"
"crypto/x509"
"encoding/json"
"net/url"
"testing"
"time"
Expand Down Expand Up @@ -442,131 +441,6 @@ func TestHTTPSDialerNetemQA(t *testing.T) {
}
}

func TestLoadHTTPSDialerPolicy(t *testing.T) {
// testcase is a test case implemented by this function
type testcase struct {
// name is the test case name
name string

// input contains the serialized input bytes
input []byte

// expectErr contains the expected error string or the empty string on success
expectErr string

// expectPolicy contains the expected policy we loaded or nil
expectedPolicy *enginenetx.HTTPSDialerLoadablePolicy
}

cases := []testcase{{
name: "with nil input",
input: nil,
expectErr: "unexpected end of JSON input",
expectedPolicy: nil,
}, {
name: "with invalid serialized JSON",
input: []byte(`{`),
expectErr: "unexpected end of JSON input",
expectedPolicy: nil,
}, {
name: "with empty serialized JSON",
input: []byte(`{}`),
expectErr: "",
expectedPolicy: &enginenetx.HTTPSDialerLoadablePolicy{},
}, {
name: "with real serialized policy",
input: (func() []byte {
return runtimex.Try1(json.Marshal(&enginenetx.HTTPSDialerLoadablePolicy{
Domains: map[string][]*enginenetx.HTTPSDialerTactic{
"api.ooni.io": {{
Endpoint: "162.55.247.208:443",
InitialDelay: 0,
SNI: "api.ooni.io",
VerifyHostname: "api.ooni.io",
}, {
Endpoint: "46.101.82.151:443",
InitialDelay: 300 * time.Millisecond,
SNI: "api.ooni.io",
VerifyHostname: "api.ooni.io",
}, {
Endpoint: "[2a03:b0c0:1:d0::ec4:9001]:443",
InitialDelay: 600 * time.Millisecond,
SNI: "api.ooni.io",
VerifyHostname: "api.ooni.io",
}, {
Endpoint: "46.101.82.151:443",
InitialDelay: 3000 * time.Millisecond,
SNI: "www.example.com",
VerifyHostname: "api.ooni.io",
}, {
Endpoint: "[2a03:b0c0:1:d0::ec4:9001]:443",
InitialDelay: 3300 * time.Millisecond,
SNI: "www.example.com",
VerifyHostname: "api.ooni.io",
}},
},
}))
})(),
expectErr: "",
expectedPolicy: &enginenetx.HTTPSDialerLoadablePolicy{
Domains: map[string][]*enginenetx.HTTPSDialerTactic{
"api.ooni.io": {{
Endpoint: "162.55.247.208:443",
InitialDelay: 0,
SNI: "api.ooni.io",
VerifyHostname: "api.ooni.io",
}, {
Endpoint: "46.101.82.151:443",
InitialDelay: 300 * time.Millisecond,
SNI: "api.ooni.io",
VerifyHostname: "api.ooni.io",
}, {
Endpoint: "[2a03:b0c0:1:d0::ec4:9001]:443",
InitialDelay: 600 * time.Millisecond,
SNI: "api.ooni.io",
VerifyHostname: "api.ooni.io",
}, {
Endpoint: "46.101.82.151:443",
InitialDelay: 3000 * time.Millisecond,
SNI: "www.example.com",
VerifyHostname: "api.ooni.io",
}, {
Endpoint: "[2a03:b0c0:1:d0::ec4:9001]:443",
InitialDelay: 3300 * time.Millisecond,
SNI: "www.example.com",
VerifyHostname: "api.ooni.io",
}},
},
},
}}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
policy, err := enginenetx.LoadHTTPSDialerPolicy(tc.input)

switch {
case err != nil && tc.expectErr == "":
t.Fatal("expected", tc.expectErr, "got", err)

case err == nil && tc.expectErr != "":
t.Fatal("expected", tc.expectErr, "got", err)

case err != nil && tc.expectErr != "":
if diff := cmp.Diff(tc.expectErr, err.Error()); diff != "" {
t.Fatal(diff)
}

case err == nil && tc.expectErr == "":
// all good
}

if diff := cmp.Diff(tc.expectedPolicy, policy); diff != "" {
t.Fatal(diff)
}
})
}
}

func TestHTTPSDialerTactic(t *testing.T) {
t.Run("String", func(t *testing.T) {
expected := `{"Endpoint":"162.55.247.208:443","InitialDelay":150000000,"SNI":"www.example.com","VerifyHostname":"api.ooni.io"}`
Expand Down
24 changes: 0 additions & 24 deletions internal/enginenetx/httpsdialerloadable.go

This file was deleted.

94 changes: 94 additions & 0 deletions internal/enginenetx/httpsdialerstatic.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package enginenetx

import (
"context"
"errors"
"fmt"

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

// HTTPSDialerStaticPolicy is an [HTTPSDialerPolicy] incorporating verbatim
// a static policy loaded from the engine's key-value store.
//
// This policy is very useful for exploration and experimentation.
type HTTPSDialerStaticPolicy struct {
// Fallback is the fallback policy in case the static one does not
// contain a rule for a specific domain.
Fallback HTTPSDialerPolicy

// Root is the root of the statically loaded policy.
Root *HTTPSDialerStaticPolicyRoot
}

// httpsDialerStaticPolicyKey is the kvstore key used to retrieve the static policy.
const httpsDialerStaticPolicyKey = "httpsdialer.conf"

// errDialerStaticPolicyWrongVersion means that the static policy document has the wrong version number.
var errDialerStaticPolicyWrongVersion = errors.New("wrong static policy version")

// NewHTTPSDialerStaticPolicy attempts to constructs a static policy using a given fallback
// policy and either returns a good policy or an error. The typical error case is the one
// in which there's no httpsDialerStaticPolicyKey in the key-value store.
func NewHTTPSDialerStaticPolicy(
kvStore model.KeyValueStore, fallback HTTPSDialerPolicy) (*HTTPSDialerStaticPolicy, error) {
// attempt to read the static policy bytes from the kvstore
data, err := kvStore.Get(httpsDialerStaticPolicyKey)
if err != nil {
return nil, err
}

// attempt to parse the static policy using human-readable JSON
var root HTTPSDialerStaticPolicyRoot
if err := hujsonx.Unmarshal(data, &root); err != nil {
return nil, err
}

// make sure the version is OK
if root.Version != httpsDialerStaticPolicyVersion {
err := fmt.Errorf(
"%s: %w: expected=%d got=%d",
httpsDialerStaticPolicyKey,
errDialerStaticPolicyWrongVersion,
httpsDialerStaticPolicyVersion,
root.Version,
)
return nil, err
}

out := &HTTPSDialerStaticPolicy{
Fallback: fallback,
Root: &root,
}
return out, nil
}

// httpsDialerStaticPolicyVersion is the current version of the static policy file.
const httpsDialerStaticPolicyVersion = 1

// HTTPSDialerStaticPolicyRoot is the root of a statically loaded policy.
type HTTPSDialerStaticPolicyRoot struct {
// Domains maps each domain to its policy.
Domains map[string][]*HTTPSDialerTactic

// Version is the data structure version.
Version int
}

var _ HTTPSDialerPolicy = &HTTPSDialerStaticPolicy{}

// LookupTactics implements HTTPSDialerPolicy.
func (ldp *HTTPSDialerStaticPolicy) LookupTactics(
ctx context.Context, domain string, port string, reso model.Resolver) ([]*HTTPSDialerTactic, error) {
tactics, found := ldp.Root.Domains[domain]
if !found {
return ldp.Fallback.LookupTactics(ctx, domain, port, reso)
}
return tactics, nil
}

// Parallelism implements HTTPSDialerPolicy.
func (ldp *HTTPSDialerStaticPolicy) Parallelism() int {
return 16
}
Loading

0 comments on commit d9f1366

Please sign in to comment.