feat: add ability to trace handshake (#64)

In order to allo tracing of events, we refactor the configuration handling. We move
options to openvpn options, and we create a config object that is
broader in scope.

The tracer implementation lives in a public package, while the interface lives for the moment in `internal/model`.

This commit also partially includes the fix for the bug described in #65, since it made sense to test the integration altogether.

---------

Co-authored-by: Simone Basso <[email protected]>

.github/workflows/build.yml

@@ -42,15 +42,3 @@ jobs:
         go-version: '1.20'
     - name: Ensure coverage threshold
       run: make test-coverage-threshold
-
-  integration:
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v2
-      - name: setup go
-        uses: actions/setup-go@v2
-        with:
-          go-version: '1.20'
-      - name: run integration tests
-        run: go test -v ./tests/integration
-

internal/controlchannel/controlchannel.go

@@ -1,3 +1,5 @@
+// Package controlchannel implements the control channel logic. The control channel sits
+// above the reliable transport and below the TLS layer.
 package controlchannel
 
 import (
@@ -36,12 +38,12 @@ type Service struct {
 //
 // [ARCHITECTURE]: https://github.com/ooni/minivpn/blob/main/ARCHITECTURE.md
 func (svc *Service) StartWorkers(
-	logger model.Logger,
+	config *model.Config,
 	workersManager *workers.Manager,
 	sessionManager *session.Manager,
 ) {
 	ws := &workersState{
-		logger:               logger,
+		logger:               config.Logger(),
 		notifyTLS:            *svc.NotifyTLS,
 		controlToReliable:    *svc.ControlToReliable,
 		reliableToControl:    svc.ReliableToControl,
@@ -90,10 +92,10 @@ func (ws *workersState) moveUpWorker() {
 				// even if after the first key generation we receive two SOFT_RESET requests
 				// back to back.
 
-				if ws.sessionManager.NegotiationState() < session.S_GENERATED_KEYS {
+				if ws.sessionManager.NegotiationState() < model.S_GENERATED_KEYS {
 					continue
 				}
-				ws.sessionManager.SetNegotiationState(session.S_INITIAL)
+				ws.sessionManager.SetNegotiationState(model.S_INITIAL)
 				// TODO(ainghazal): revisit this step.
 				// when we implement key rotation.  OpenVPN has
 				// the concept of a "lame duck", i.e., the

internal/datachannel/controller.go

@@ -25,7 +25,7 @@ type dataChannelHandler interface {
 // DataChannel represents the data "channel", that will encrypt and decrypt the tunnel payloads.
 // data implements the dataHandler interface.
 type DataChannel struct {
-	options         *model.Options
+	options         *model.OpenVPNOptions
 	sessionManager  *session.Manager
 	state           *dataChannelState
 	decodeFn        func(model.Logger, []byte, *session.Manager, *dataChannelState) (*encryptedData, error)
@@ -39,7 +39,7 @@ var _ dataChannelHandler = &DataChannel{} // Ensure that we implement dataChanne
 // NewDataChannelFromOptions returns a new data object, initialized with the
 // options given. it also returns any error raised.
 func NewDataChannelFromOptions(log model.Logger,
-	opt *model.Options,
+	opt *model.OpenVPNOptions,
 	sessionManager *session.Manager) (*DataChannel, error) {
 	runtimex.Assert(opt != nil, "openvpn datachannel: opts cannot be nil")
 	runtimex.Assert(opt != nil, "openvpn datachannel: opts cannot be nil")

internal/datachannel/read.go

@@ -97,7 +97,7 @@ func decodeEncryptedPayloadNonAEAD(log model.Logger, buf []byte, session *sessio
 // modes are supported at the moment, so no real decompression is done. It
 // returns a byte array, and an error if the operation could not be completed
 // successfully.
-func maybeDecompress(b []byte, st *dataChannelState, opt *model.Options) ([]byte, error) {
+func maybeDecompress(b []byte, st *dataChannelState, opt *model.OpenVPNOptions) ([]byte, error) {
 	if st == nil || st.dataCipher == nil {
 		return []byte{}, fmt.Errorf("%w:%s", errBadInput, "bad state")
 	}

internal/datachannel/service.go

@@ -50,22 +50,21 @@ type Service struct {
 // 3. keyWorker BLOCKS on keyUp to read a dataChannelKey and
 // initializes the internal state with the resulting key;
 func (s *Service) StartWorkers(
-	logger model.Logger,
+	config *model.Config,
 	workersManager *workers.Manager,
 	sessionManager *session.Manager,
-	options *model.Options,
 ) {
-	dc, err := NewDataChannelFromOptions(logger, options, sessionManager)
+	dc, err := NewDataChannelFromOptions(config.Logger(), config.OpenVPNOptions(), sessionManager)
 	if err != nil {
-		logger.Warnf("cannot initialize channel %v", err)
+		config.Logger().Warnf("cannot initialize channel %v", err)
 		return
 	}
 	ws := &workersState{
 		dataChannel:          dc,
 		dataOrControlToMuxer: *s.DataOrControlToMuxer,
 		dataToTUN:            s.DataToTUN,
 		keyReady:             s.KeyReady,
-		logger:               logger,
+		logger:               config.Logger(),
 		muxerToData:          s.MuxerToData,
 		sessionManager:       sessionManager,
 		tunToData:            s.TUNToData,
@@ -193,7 +192,7 @@ func (ws *workersState) keyWorker(firstKeyReady chan<- any) {
 				ws.logger.Warnf("error on key derivation: %v", err)
 				continue
 			}
-			ws.sessionManager.SetNegotiationState(session.S_GENERATED_KEYS)
+			ws.sessionManager.SetNegotiationState(model.S_GENERATED_KEYS)
 			once.Do(func() {
 				close(firstKeyReady)
 			})

internal/model/config.go

@@ -0,0 +1,96 @@
+package model
+
+import (
+	"net"
+
+	"github.com/apex/log"
+	"github.com/ooni/minivpn/internal/runtimex"
+)
+
+// Config contains options to initialize the OpenVPN tunnel.
+type Config struct {
+	// openVPNOptions contains options related to openvpn.
+	openvpnOptions *OpenVPNOptions
+
+	// logger will be used to log events.
+	logger Logger
+
+	// if a tracer is provided, it will be used to trace the openvpn handshake.
+	tracer HandshakeTracer
+}
+
+// NewConfig returns a Config ready to intialize a vpn tunnel.
+func NewConfig(options ...Option) *Config {
+	cfg := &Config{
+		openvpnOptions: &OpenVPNOptions{},
+		logger:         log.Log,
+		tracer:         &dummyTracer{},
+	}
+	for _, opt := range options {
+		opt(cfg)
+	}
+	return cfg
+}
+
+// Option is an option you can pass to initialize minivpn.
+type Option func(config *Config)
+
+// WithConfigFile configures OpenVPNOptions parsed from the given file.
+func WithConfigFile(configPath string) Option {
+	return func(config *Config) {
+		openvpnOpts, err := ReadConfigFile(configPath)
+		runtimex.PanicOnError(err, "cannot parse config file")
+		runtimex.PanicIfFalse(openvpnOpts.HasAuthInfo(), "missing auth info")
+		config.openvpnOptions = openvpnOpts
+	}
+}
+
+// WithLogger configures the passed [Logger].
+func WithLogger(logger Logger) Option {
+	return func(config *Config) {
+		config.logger = logger
+	}
+}
+
+// WithHandshakeTracer configures the passed [HandshakeTracer].
+func WithHandshakeTracer(tracer HandshakeTracer) Option {
+	return func(config *Config) {
+		config.tracer = tracer
+	}
+}
+
+// Logger returns the configured logger.
+func (c *Config) Logger() Logger {
+	return c.logger
+}
+
+// Tracer returns the handshake tracer.
+func (c *Config) Tracer() HandshakeTracer {
+	return c.tracer
+}
+
+// OpenVPNOptions returns the configured openvpn options.
+func (c *Config) OpenVPNOptions() *OpenVPNOptions {
+	return c.openvpnOptions
+}
+
+// Remote returns the OpenVPN remote.
+func (c *Config) Remote() *Remote {
+	return &Remote{
+		IPAddr:   c.openvpnOptions.Remote,
+		Endpoint: net.JoinHostPort(c.openvpnOptions.Remote, c.openvpnOptions.Port),
+		Protocol: c.openvpnOptions.Proto.String(),
+	}
+}
+
+// Remote has info about the OpenVPN remote.
+type Remote struct {
+	// IPAddr is the IP Address for the remote.
+	IPAddr string
+
+	// Endpoint is in the form ip:port.
+	Endpoint string
+
+	// Protocol is either "tcp" or "udp"
+	Protocol string
+}

internal/model/packet.go

@@ -334,13 +334,8 @@ func (p *Packet) IsData() bool {
 	return p.Opcode.IsData()
 }
 
-const (
-	DirectionIncoming = iota
-	DirectionOutgoing
-)
-
 // Log writes an entry in the passed logger with a representation of this packet.
-func (p *Packet) Log(logger Logger, direction int) {
+func (p *Packet) Log(logger Logger, direction Direction) {
 	var dir string
 	switch direction {
 	case DirectionIncoming:

internal/model/session.go

@@ -0,0 +1,59 @@
+package model
+
+// NegotiationState is the state of the session negotiation.
+type NegotiationState int
+
+const (
+	// S_ERROR means there was some form of protocol error.
+	S_ERROR = NegotiationState(iota) - 1
+
+	// S_UNDER is the undefined state.
+	S_UNDEF
+
+	// S_INITIAL means we're ready to begin the three-way handshake.
+	S_INITIAL
+
+	// S_PRE_START means we're waiting for acknowledgment from the remote.
+	S_PRE_START
+
+	// S_START means we've done the three-way handshake.
+	S_START
+
+	// S_SENT_KEY means we have sent the local part of the key_source2 random material.
+	S_SENT_KEY
+
+	// S_GOT_KEY means we have got the remote part of key_source2.
+	S_GOT_KEY
+
+	// S_ACTIVE means the control channel was established.
+	S_ACTIVE
+
+	// S_GENERATED_KEYS means the data channel keys have been generated.
+	S_GENERATED_KEYS
+)
+
+// String maps a [SessionNegotiationState] to a string.
+func (sns NegotiationState) String() string {
+	switch sns {
+	case S_UNDEF:
+		return "S_UNDEF"
+	case S_INITIAL:
+		return "S_INITIAL"
+	case S_PRE_START:
+		return "S_PRE_START"
+	case S_START:
+		return "S_START"
+	case S_SENT_KEY:
+		return "S_SENT_KEY"
+	case S_GOT_KEY:
+		return "S_GOT_KEY"
+	case S_ACTIVE:
+		return "S_ACTIVE"
+	case S_GENERATED_KEYS:
+		return "S_GENERATED_KEYS"
+	case S_ERROR:
+		return "S_ERROR"
+	default:
+		return "S_INVALID"
+	}
+}

internal/model/trace.go

@@ -0,0 +1,72 @@
+package model
+
+import (
+	"fmt"
+	"time"
+)
+
+// HandshakeTracer allows to collect traces for a given OpenVPN handshake. A HandshakeTracer can be optionally
+// added to the top-level TUN constructor, and it will be propagated to any layer that needs to register an event.
+type HandshakeTracer interface {
+	// TimeNow allows to inject time for deterministic tests.
+	TimeNow() time.Time
+
+	// OnStateChange is called for each transition in the state machine.
+	OnStateChange(state NegotiationState)
+
+	// OnIncomingPacket is called when a packet is received.
+	OnIncomingPacket(packet *Packet, stage NegotiationState)
+
+	// OnOutgoingPacket is called when a packet is about to be sent.
+	OnOutgoingPacket(packet *Packet, stage NegotiationState, retries int)
+
+	// OnDroppedPacket is called whenever a packet is dropped (in/out)
+	OnDroppedPacket(direction Direction, stage NegotiationState, packet *Packet)
+}
+
+// Direction is one of two directions on a packet.
+type Direction int
+
+const (
+	// DirectionIncoming marks received packets.
+	DirectionIncoming = Direction(iota)
+
+	// DirectionOutgoing marks packets to be sent.
+	DirectionOutgoing
+)
+
+var _ fmt.Stringer = Direction(0)
+
+// String implements fmt.Stringer
+func (d Direction) String() string {
+	switch d {
+	case DirectionIncoming:
+		return "read"
+	case DirectionOutgoing:
+		return "write"
+	default:
+		return "undefined"
+	}
+}
+
+// dummyTracer is a no-op implementation of [model.HandshakeTracer] that does nothing
+// but can be safely passed as a default implementation.
+type dummyTracer struct{}
+
+// TimeNow allows to manipulate time for deterministic tests.
+func (dt *dummyTracer) TimeNow() time.Time { return time.Now() }
+
+// OnStateChange is called for each transition in the state machine.
+func (dt *dummyTracer) OnStateChange(NegotiationState) {}
+
+// OnIncomingPacket is called when a packet is received.
+func (dt *dummyTracer) OnIncomingPacket(*Packet, NegotiationState) {}
+
+// OnOutgoingPacket is called when a packet is about to be sent.
+func (dt *dummyTracer) OnOutgoingPacket(*Packet, NegotiationState, int) {}
+
+// OnDroppedPacket is called whenever a packet is dropped (in/out)
+func (dt *dummyTracer) OnDroppedPacket(Direction, NegotiationState, *Packet) {}
+
+// Assert that dummyTracer implements [model.HandshakeTracer].
+var _ HandshakeTracer = &dummyTracer{}