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.

Sign up for GitHub

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

Jump to bottom

Collect connection address in connectivity test #223

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Collect connection address in connectivity test #223

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
60e4afb
Collect connection address
fortuna Apr 19, 2024
ebecc1f
Try intercepting the transport dialer
fortuna Apr 19, 2024
21e0e1b
Pass wrapper instead
fortuna Apr 19, 2024
577829f
Clean up
fortuna Apr 19, 2024
85269c2
Comment
fortuna Apr 19, 2024
8541f46
Merge branch 'main' into fortuna-address
fortuna Apr 24, 2024
db2f334
Collect connections
fortuna Apr 24, 2024
96ae7fb
Fix
fortuna Apr 24, 2024
7542c5a
Timeout
fortuna Apr 24, 2024
7d8d602
To fix
fortuna Apr 24, 2024
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
10 changes: 8 additions & 2 deletions x/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func NewStreamDialer(transportConfig string) (transport.StreamDialer, error) {
return WrapStreamDialer(&transport.TCPDialer{}, transportConfig)
}

// WrapStreamDialer created a [transport.StreamDialer] according to transportConfig, using dialer as the
// WrapStreamDialer creates a [transport.StreamDialer] according to transportConfig, using dialer as the
// base [transport.StreamDialer]. The given dialer must not be nil.
func WrapStreamDialer(dialer transport.StreamDialer, transportConfig string) (transport.StreamDialer, error) {
if dialer == nil {
Expand Down Expand Up @@ -110,11 +110,17 @@ func newStreamDialerFromPart(innerDialer transport.StreamDialer, oneDialerConfig

// NewPacketDialer creates a new [transport.PacketDialer] according to the given config.
func NewPacketDialer(transportConfig string) (dialer transport.PacketDialer, err error) {
dialer = &transport.UDPDialer{}
return WrapPacketDialer(&transport.UDPDialer{}, transportConfig)
}

// WrapPacketDialer creates a [transport.PacketDialer] according to transportConfig, using dialer as the
// base [transport.PacketDialer]. The given dialer must not be nil.
func WrapPacketDialer(dialer transport.PacketDialer, transportConfig string) (transport.PacketDialer, error) {
transportConfig = strings.TrimSpace(transportConfig)
if transportConfig == "" {
return dialer, nil
}
var err error
for _, part := range strings.Split(transportConfig, "|") {
dialer, err = newPacketDialerFromPart(dialer, part)
if err != nil {
Expand Down
43 changes: 40 additions & 3 deletions x/examples/test-connectivity/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"time"

"github.com/Jigsaw-Code/outline-sdk/dns"
"github.com/Jigsaw-Code/outline-sdk/transport"
"github.com/Jigsaw-Code/outline-sdk/x/config"
"github.com/Jigsaw-Code/outline-sdk/x/connectivity"
"github.com/Jigsaw-Code/outline-sdk/x/report"
Expand All @@ -47,6 +48,9 @@ type connectivityReport struct {
// TODO(fortuna): add sanitized transport config.
Transport string `json:"transport"`

// The address of the selected connection to the proxy server.
ConnectionAddress addressJSON `json:"connection_address"`

// Observations
Time time.Time `json:"time"`
DurationMs int64 `json:"duration_ms"`
Expand All @@ -62,6 +66,19 @@ type errorJSON struct {
Msg string `json:"msg,omitempty"`
}

type addressJSON struct {
Host string `json:"host"`
Port string `json:"port"`
}

func newAddressJSON(address string) (addressJSON, error) {
host, port, err := net.SplitHostPort(address)
if err != nil {
return addressJSON{}, err
}
return addressJSON{host, port}, nil
}

func makeErrorRecord(result *connectivity.ConnectivityError) *errorJSON {
if result == nil {
return nil
Expand Down Expand Up @@ -167,15 +184,30 @@ func main() {
for _, proto := range strings.Split(*protoFlag, ",") {
proto = strings.TrimSpace(proto)
var resolver dns.Resolver
var connectionAddress string
switch proto {
case "tcp":
streamDialer, err := config.NewStreamDialer(*transportFlag)
baseDialer := transport.FuncStreamDialer(func(ctx context.Context, addr string) (transport.StreamConn, error) {
conn, err := (&transport.TCPDialer{}).DialStream(ctx, addr)
if conn != nil {
connectionAddress = conn.RemoteAddr().String()
}
return conn, err
})
streamDialer, err := config.WrapStreamDialer(baseDialer, *transportFlag)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to intercept the streamDialer instead of the baseDialer, but it didn't work because the RemoteAddress is for the target in the PacketListenerDialer. Code: https://github.com/Jigsaw-Code/outline-sdk/compare/fortuna-address2?expand=1

It's not clear what the RemoteTarget should be, so I'm not sure that is a god approach. I think I prefer to keep this base dialer interception. But we could think about some sort of event listener instead, to reduce wrapping.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fortuna I like the idea of intercept the streamDialer. btw in the suggested implementation, if connect fails and conn is nil, connectionAddress will be left empty and we don't know what endpoint was attempted. I can address this by extracting attempted endpoint from err by assessing error type with err.(*net.OpError).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have the dialed address in the transport specification.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the PR. We now have functions that can test dialer wrappers, since the one that takes the resolver was not enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see your point. We won't have the address for errors, so it's hard to tell if it's a bad resolution, for instance.

I think we can extend the code to do Happy Eyeballs or Dialer.ControlContext. But perhaps in another PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If multiple endpoints are attempted, which one do we select? The first one? The last one? All of them? Perhaps we need to collect all of them, then specify which one we picked for the transport part.

I don't think we are able to capture all of them by looking at the error. We need to do the resolution ourselves.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is still a bit clunky, but I think the data model is better aligned. Example:

% go run . -transport $KEY -proto tcp | jq
{
  "resolver": "8.8.8.8:53",
  "proto": "tcp",
  "transport": "ss://[email protected]:80?prefix=",
  "connections": [
    {
      "address": {
        "host": "159.203.76.146",
        "port": "80"
      },
      "error": null
    }
  ],
  "selected_address": {
    "host": "159.203.76.146",
    "port": "80"
  },
  "time": "2024-04-24T02:22:09Z",
  "duration_ms": 30,
  "error": {
    "op": "receive",
    "msg": "chacha20poly1305: message authentication failed"
  }
}

if err != nil {
log.Fatalf("Failed to create StreamDialer: %v", err)
}
resolver = dns.NewTCPResolver(streamDialer, resolverAddress)
case "udp":
packetDialer, err := config.NewPacketDialer(*transportFlag)
baseDialer := transport.FuncPacketDialer(func(ctx context.Context, addr string) (net.Conn, error) {
conn, err := (&transport.UDPDialer{}).DialPacket(ctx, addr)
if conn != nil {
connectionAddress = conn.RemoteAddr().String()
}
return conn, err
})
packetDialer, err := config.WrapPacketDialer(baseDialer, *transportFlag)
if err != nil {
log.Fatalf("Failed to create PacketDialer: %v", err)
}
Expand All @@ -197,7 +229,7 @@ func main() {
if err != nil {
log.Fatalf("Failed to sanitize config: %v", err)
}
var r report.Report = connectivityReport{
r := connectivityReport{
Resolver: resolverAddress,
Proto: proto,
Time: startTime.UTC().Truncate(time.Second),
Expand All @@ -206,6 +238,11 @@ func main() {
DurationMs: testDuration.Milliseconds(),
Error: makeErrorRecord(result),
}
connectionAddressJSON, err := newAddressJSON(connectionAddress)
if err == nil {
r.ConnectionAddress = connectionAddressJSON
}

if reportCollector != nil {
err = reportCollector.Collect(context.Background(), r)
if err != nil {
Expand Down
Loading