Skip to content

Commit

Permalink
Port Randomization follow-up fixes (#244)
Browse files Browse the repository at this point in the history
revisiting port randomization and selection to ensure client and server match
  • Loading branch information
jmwample authored Oct 9, 2023
1 parent 8999840 commit 14ab64d
Show file tree
Hide file tree
Showing 11 changed files with 219 additions and 58 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ require (
github.com/pion/stun v0.6.1
github.com/pion/transport/v2 v2.2.3
github.com/refraction-networking/ed25519 v0.1.2
github.com/refraction-networking/gotapdance v1.7.5-0.20231007192233-6c0352155207
github.com/refraction-networking/gotapdance v1.7.5-0.20231008035356-980b28fc1555
github.com/refraction-networking/obfs4 v0.1.2
github.com/refraction-networking/utls v1.3.3
github.com/sirupsen/logrus v1.9.3
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/refraction-networking/ed25519 v0.1.2 h1:08kJZUkAlY7a7cZGosl1teGytV+QEoNxPO7NnRvAB+g=
github.com/refraction-networking/ed25519 v0.1.2/go.mod h1:nxYLUAYt/hmNpAh64PNSQ/tQ9gTIB89wCaGKJlRtZ9I=
github.com/refraction-networking/gotapdance v1.7.5-0.20231007192233-6c0352155207 h1:j13eRL4998NZSroZyz5E795cCQj2E/USmHLot+nr+G8=
github.com/refraction-networking/gotapdance v1.7.5-0.20231007192233-6c0352155207/go.mod h1:w40UUCQH18Z2pWVLAGqj6L9d3t3YpAhW4w5r0YgGptY=
github.com/refraction-networking/gotapdance v1.7.5-0.20231008035356-980b28fc1555 h1:k1s1Uz+GiAn/6qUdQt8NmrZLe9O2QxZ8CdqC/d71qdk=
github.com/refraction-networking/gotapdance v1.7.5-0.20231008035356-980b28fc1555/go.mod h1:ReFEvrTygB7w64WU380mcKB9+IDekqepWdfAA5yIbng=
github.com/refraction-networking/obfs4 v0.1.2 h1:J842O4fGSkd2W8ogYj0KN6gqVVY+Cpqodw9qFGL7wVU=
github.com/refraction-networking/obfs4 v0.1.2/go.mod h1:wAl/+gWiLsrcykJA3nKJHx89f5/gXGM8UKvty7+mvbM=
github.com/refraction-networking/utls v1.3.3 h1:f/TBLX7KBciRyFH3bwupp+CE4fzoYKCirhdRcC490sw=
Expand Down
134 changes: 134 additions & 0 deletions internal/port_integration_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
package integration_test

import (
"context"
"crypto/rand"
"os"
"testing"

"github.com/refraction-networking/conjure/internal/conjurepath"
"github.com/refraction-networking/conjure/internal/testutils"
"github.com/refraction-networking/conjure/pkg/core"
"github.com/refraction-networking/conjure/pkg/core/interfaces"
"github.com/refraction-networking/conjure/pkg/station/log"
pb "github.com/refraction-networking/conjure/proto"
"github.com/refraction-networking/ed25519"
"github.com/refraction-networking/ed25519/extra25519"
"github.com/stretchr/testify/require"
"golang.org/x/crypto/curve25519"
"google.golang.org/protobuf/types/known/anypb"
)

func TestBase(T *testing.T) {
T.Run("TestBase", func(T *testing.T) {
require.Equal(T, 1, 1)
})
}

func TestTransportPortSelection(t *testing.T) {
if *debug {
log.SetLevel(log.DebugLevel)
}

var testCases = getTestCases(t)
for _, testCase := range testCases {
t.Run(testCase.clientTransport.Name(), func(t *testing.T) {
testTransportPortSelection(t, testCase.stationTransportBuilder, testCase.clientTransport, testCase.clientParamPermuteGenerator)
})
}
}

func testTransportPortSelection(t *testing.T, builder stationBuilder, clientTransport interfaces.WrappingTransport, clientParamPermuteGenerator func() []TestParams) {
testSubnetPath := conjurepath.Root + "/pkg/station/lib/test/phantom_subnets.toml"
os.Setenv("PHANTOM_SUBNET_LOCATION", testSubnetPath)
libver := uint(core.CurrentClientLibraryVersion())

_, private, _ := ed25519.GenerateKey(rand.Reader)

var curve25519Public, curve25519Private [32]byte
extra25519.PrivateKeyToCurve25519(&curve25519Private, private)
curve25519.ScalarBaseMult(&curve25519Public, &curve25519Private)

transport := builder(curve25519Private)
transportType := clientTransport.ID()

// Ensure that we test all given parameter permutations as well as nil params
paramSet := append([]TestParams{nil}, clientParamPermuteGenerator()...)

for _, testParams := range paramSet {
if testParams == nil {
testParams = &nilParams{}
}
params := testParams.GetParams()
clientKeys, err := core.GenerateClientSharedKeys(curve25519Public)
require.Nil(t, err)

err = clientTransport.SetParams(params)
require.Nil(t, err)

// ensure that if the registration w/ transport config is re-used then the selected port is
// still consistent between the client and server.
for i := 0; i < 10; i++ {
gen := uint32(i%2 + 1) // the subnet set n gen1 doesn't support phantom randomization, the set in gen2 does

err = clientTransport.Prepare(context.Background(), nil)
require.Nil(t, err)

protoParams, err := clientTransport.GetParams()
require.Nil(t, err)
log.Debugf("running %s w/ %s", clientTransport.Name(), testParams.String())

manager := testutils.SetupRegistrationManager(testutils.Transport{Index: transportType, Transport: transport})
require.NotNil(t, manager)

v := uint32(libver)
covert := "1.2.3.4:56789"
regType := pb.RegistrationSource_API
c2s := &pb.ClientToStation{
ClientLibVersion: &v,
Transport: &transportType,
CovertAddress: &covert,
DecoyListGeneration: &gen,
}
if params != nil {
p, err := anypb.New(protoParams)
if err != nil {
log.Fatalln("failed to make params", err)
}
c2s.TransportParams = p
}

keys, err := core.GenSharedKeys(libver, clientKeys.SharedSecret, transportType)
if err != nil {
log.Fatalln("failed to generate shared keys:", err)
}

for _, v6Reg := range []bool{false, true} {
phantom, err := manager.PhantomSelector.Select(
clientKeys.ConjureSeed, uint(gen), libver, v6Reg)
require.Nil(t, err)

reg, err := manager.NewRegistration(c2s, &keys, v6Reg, &regType)
require.Nil(t, err, "failed to create new Registration")

reg.Transport = transportType

// Get the port that the client will connect to
clientPort, err := clientTransport.GetDstPort(clientKeys.ConjureSeed)
require.Nil(t, err, "failed to get client port")

serverPort := reg.PhantomPort

if phantom.SupportRandomPort() {
// If the phantom supports random ports, then the client and server should pick
// the same randomized destination port
require.Equal(t, clientPort, serverPort, "c:%d != s:%d - %s %s", clientPort, serverPort, transport.Name(), testParams.String())
} else {
// If the phantom does not support random ports, then the server should pick 443
// the client applies this policy at a higher level so we don't check it here.
require.Equal(t, uint16(443), serverPort, "443 != s:%d - %s %s", serverPort, transport.Name(), testParams.String())
}
}
}
}
}
2 changes: 2 additions & 0 deletions internal/test_assets/phantom_subnets.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@
Generation = 1
[[Networks.1.WeightedSubnets]]
Weight = 9
RandomizeDstPort = false
Subnets = ["192.122.190.0/24", "2001:48a8:687f:1::/64"]

[Networks.2]
Generation = 2
[[Networks.2.WeightedSubnets]]
Weight = 1
RandomizeDstPort = true
Subnets = ["192.122.190.0/28", "2001:48a8:687f:1::/96"]

[Networks.957]
Expand Down
40 changes: 23 additions & 17 deletions internal/transport_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,24 +45,16 @@ type TestParams interface {

type stationBuilder = func([32]byte) lib.WrappingTransport

func TestTransportsEndToEnd(t *testing.T) {
if *debug {
log.SetLevel(log.DebugLevel)
}
testCases := []struct {
func getTestCases(t *testing.T) []struct {
stationTransportBuilder stationBuilder
clientTransport interfaces.WrappingTransport
clientParamPermuteGenerator func() []TestParams
} {
return []struct {
stationTransportBuilder stationBuilder
clientTransport interfaces.WrappingTransport
clientParamPermuteGenerator func() []TestParams
}{
{
func(privKey [32]byte) lib.WrappingTransport {
tr, err := prefix.Default(privKey)
require.Nil(t, err)
return tr
},
&prefix.ClientTransport{},
prefixClientParamPermutations,
},
{
func(privKey [32]byte) lib.WrappingTransport {
return &obfs4.Transport{}
Expand All @@ -77,7 +69,24 @@ func TestTransportsEndToEnd(t *testing.T) {
&min.ClientTransport{},
genericParamPermutations,
},
{
func(privKey [32]byte) lib.WrappingTransport {
tr, err := prefix.Default(privKey)
require.Nil(t, err)
return tr
},
&prefix.ClientTransport{},
prefixClientParamPermutations,
},
}
}

func TestTransportsEndToEnd(t *testing.T) {
if *debug {
log.SetLevel(log.DebugLevel)
}

var testCases = getTestCases(t)
for _, testCase := range testCases {
t.Run(testCase.clientTransport.Name(), func(t *testing.T) {
testTransportsEndToEnd(t, testCase.stationTransportBuilder, testCase.clientTransport, testCase.clientParamPermuteGenerator)
Expand All @@ -101,11 +110,8 @@ func testTransportsEndToEnd(t *testing.T, builder stationBuilder, clientTranspor
transport := builder(curve25519Private)

// Ensure that we test all given parameter permutations as well as nil params
// paramSet := append([]any{nil}, clientParamPermuteGenerator())
paramSet := append([]TestParams{nil}, clientParamPermuteGenerator()...)

// for _, flushPolicy := range []int32{DefaultFlush, NoAddedFlush, FlushAfterPrefix} {
// for idx := range defaultPrefixes {
for _, testParams := range paramSet {

if testParams == nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/core/interfaces/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ type Transport interface {
Prepare(ctx context.Context, dialer func(ctx context.Context, network, laddr, raddr string) (net.Conn, error)) error

// GetDstPort returns the destination port that the client should open the phantom connection with.
GetDstPort(seed []byte, randomizeDstPorSupported bool) (uint16, error)
GetDstPort(seed []byte) (uint16, error)

// PrepareKeys provides an opportunity for the transport to integrate the station public key
// as well as bytes from the deterministic random generator associated with the registration
Expand Down
4 changes: 2 additions & 2 deletions pkg/transports/connecting/dtls/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,8 @@ func (*ClientTransport) DisableRegDelay() bool {
}

// GetDstPort returns the destination port that the client should open the phantom connection to
func (t *ClientTransport) GetDstPort(seed []byte, phantomSubnetSupportsRandPort bool) (uint16, error) {
if t.Parameters == nil || !t.Parameters.GetRandomizeDstPort() || !phantomSubnetSupportsRandPort {
func (t *ClientTransport) GetDstPort(seed []byte) (uint16, error) {
if t.Parameters == nil || !t.Parameters.GetRandomizeDstPort() {
return defaultPort, nil
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/transports/wrapping/min/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ func (t *ClientTransport) SetParams(p any) error {
}

// GetDstPort returns the destination port that the client should open the phantom connection to
func (t *ClientTransport) GetDstPort(seed []byte, phantomSubnetSupportsRandPort bool) (uint16, error) {
if t.sessionParams == nil || !t.sessionParams.GetRandomizeDstPort() || !phantomSubnetSupportsRandPort {
func (t *ClientTransport) GetDstPort(seed []byte) (uint16, error) {
if t.sessionParams == nil || !t.sessionParams.GetRandomizeDstPort() {
return 443, nil
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/transports/wrapping/obfs4/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ func (t ClientTransport) ParseParams(data *anypb.Any) (any, error) {
}

// GetDstPort returns the destination port that the client should open the phantom connection to
func (t *ClientTransport) GetDstPort(seed []byte, phantomSubnetSupportsRandPort bool) (uint16, error) {
if t.sessionParams == nil || !t.sessionParams.GetRandomizeDstPort() || !phantomSubnetSupportsRandPort {
func (t *ClientTransport) GetDstPort(seed []byte) (uint16, error) {
if t.sessionParams == nil || !t.sessionParams.GetRandomizeDstPort() {
return 443, nil
}

Expand Down
Loading

0 comments on commit 14ab64d

Please sign in to comment.