resolve conflicts

.gitignore

@@ -1,10 +1,9 @@
-minivpn
-openvpn
-vpnping
-obfs4vpn
-geturl
+/minivpn
+/vpnping
+/obfs4vpn
+/geturl
+/ndt7
 .vscode
-ndt7
 *.swp
 *.swo
 *.pem

go.mod

@@ -1,8 +1,8 @@
 module github.com/ooni/minivpn
 
-go 1.21.1
+go 1.21
 
-//toolchain go1.21.5
+toolchain go1.21.5
 
 // pinning for backwards-incompatible change
 // replace gitlab.com/yawning/obfs4.git v0.0.0-20220204003609-77af0cba934d => gitlab.com/yawning/obfs4.git v0.0.0-20210511220700-e330d1b7024b

go.sum

@@ -328,7 +328,6 @@ github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UV
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
-github.com/stretchr/testify v1.7.1 h1:5TQK59W5E3v0r2duFAb7P95B6hEeOyEnHRa8MjYSMTY=
 github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
 github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
@@ -628,8 +627,8 @@ google.golang.org/protobuf v1.23.1-0.20200526195155-81db48ad09cc/go.mod h1:EGpAD
 google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c=
 google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw=
 google.golang.org/protobuf v1.27.1/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
-google.golang.org/protobuf v1.31.0 h1:g0LDEJHgrBl9N9r17Ru3sqWhkIx2NB67okBHPwC7hs8=
-google.golang.org/protobuf v1.31.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I=
+google.golang.org/protobuf v1.28.2-0.20230118093459-a9481185b34d h1:qp0AnQCvRCMlu9jBjtdbTaaEmThIgZOrbVyDEOcmKhQ=
+google.golang.org/protobuf v1.28.2-0.20230118093459-a9481185b34d/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I=
 gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
@@ -657,8 +656,6 @@ gotest.tools/v3 v3.4.0 h1:ZazjZUfuVeZGLAmlKKuyv3IKP5orXcwtOwDQH6YVr6o=
 gotest.tools/v3 v3.4.0/go.mod h1:CtbdzLSsqVhDgMtKsx03ird5YTGB3ar27v0u/yKBW5g=
 gvisor.dev/gvisor v0.0.0-20230927004350-cbd86285d259 h1:TbRPT0HtzFP3Cno1zZo7yPzEEnfu8EjLfl6IU9VfqkQ=
 gvisor.dev/gvisor v0.0.0-20230927004350-cbd86285d259/go.mod h1:AVgIgHMwK63XvmAzWG9vLQ41YnVHN0du0tEC46fI7yY=
-gvisor.dev/gvisor v0.0.0-20240104232245-1e61310ce61e h1:KNBb7yeP1HRByrH54W1Bw2LCrKualEWp8oQtCJarK00=
-gvisor.dev/gvisor v0.0.0-20240104232245-1e61310ce61e/go.mod h1:10sU+Uh5KKNv1+2x2A0Gvzt8FjD3ASIhorV3YsauXhk=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=

internal/bytesx/bytesx.go

@@ -1,14 +1,13 @@
-package bytesx
-
+// Package bytesx provides functions operating on bytes.
 //
-// Functions operating on bytes:
+// Specifically we implement these operations:
 //
 // 1. generating random bytes;
 //
 // 2. OpenVPN options encoding and decoding;
 //
 // 3. PKCS#7 padding and unpadding.
-//
+package bytesx
 
 import (
 	"bytes"

internal/model/doc.go

@@ -0,0 +1,2 @@
+// Package model implements common models for the vpn data structures.
+package model

internal/model/packet.go

@@ -1,5 +1,11 @@
 package model
 
+//
+// Packet
+//
+// Parsing and serializing OpenVPN packets.
+//
+
 import (
 	"bytes"
 	"errors"

internal/networkio/closeonce.go

@@ -5,29 +5,29 @@ import (
 	"sync"
 )
 
-// CloseOnceConn is a [net.Conn] where the Close method has once semantics.
+// closeOnceConn is a [net.Conn] where the Close method has once semantics.
 //
-// The zero value is invalid; use [NewCloseOnceConn].
-type CloseOnceConn struct {
+// The zero value is invalid; use [newCloseOnceConn].
+type closeOnceConn struct {
 	// once ensures we close just once.
 	once sync.Once
 
 	// Conn is the underlying conn.
 	net.Conn
 }
 
-var _ net.Conn = &CloseOnceConn{}
+var _ net.Conn = &closeOnceConn{}
 
-// NewCloseOnceConn creates a [CloseOnceConn].
-func NewCloseOnceConn(conn net.Conn) *CloseOnceConn {
-	return &CloseOnceConn{
+// newCloseOnceConn creates a [closeOnceConn].
+func newCloseOnceConn(conn net.Conn) *closeOnceConn {
+	return &closeOnceConn{
 		once: sync.Once{},
 		Conn: conn,
 	}
 }
 
 // Close implements net.Conn
-func (c *CloseOnceConn) Close() (err error) {
+func (c *closeOnceConn) Close() (err error) {
 	c.once.Do(func() {
 		err = c.Conn.Close()
 	})

internal/networkio/datagram.go

@@ -5,15 +5,15 @@ import (
 	"net"
 )
 
-// DatagramConn wraps a datagram socket and implements OpenVPN framing.
-type DatagramConn struct {
+// datagramConn wraps a datagram socket and implements OpenVPN framing.
+type datagramConn struct {
 	net.Conn
 }
 
-var _ FramingConn = &DatagramConn{}
+var _ FramingConn = &datagramConn{}
 
 // ReadRawPacket implements FramingConn
-func (c *DatagramConn) ReadRawPacket() ([]byte, error) {
+func (c *datagramConn) ReadRawPacket() ([]byte, error) {
 	buffer := make([]byte, math.MaxUint16) // maximum UDP datagram size
 	count, err := c.Read(buffer)
 	if err != nil {
@@ -24,7 +24,7 @@ func (c *DatagramConn) ReadRawPacket() ([]byte, error) {
 }
 
 // WriteRawPacket implements FramingConn
-func (c *DatagramConn) WriteRawPacket(pkt []byte) error {
+func (c *datagramConn) WriteRawPacket(pkt []byte) error {
 	if len(pkt) > math.MaxUint16 {
 		return ErrPacketTooLarge
 	}

internal/networkio/dialer.go

@@ -34,16 +34,16 @@ func (d *Dialer) DialContext(ctx context.Context, network, address string) (Fram
 		return nil, err
 	}
 
-	d.logger.Infof("connected to %s/%s", address, network)
+	d.logger.Debugf("networkio: connected to %s/%s", address, network)
 
 	// make sure the conn has close once semantics
-	conn = NewCloseOnceConn(conn)
+	conn = newCloseOnceConn(conn)
 
 	// wrap the conn and return
 	switch conn.LocalAddr().Network() {
 	case "udp", "udp4", "udp6":
-		return &DatagramConn{conn}, nil
+		return &datagramConn{conn}, nil
 	default:
-		return &StreamConn{conn}, nil
+		return &streamConn{conn}, nil
 	}
 }

internal/networkio/service.go

@@ -10,6 +10,7 @@ import (
 type Service struct {
 	// MuxerToNetwork moves bytes down from the muxer to the network IO layer
 	MuxerToNetwork chan []byte
+
 	// NetworkToMuxer moves bytes up from the network IO layer to the muxer
 	NetworkToMuxer *chan []byte
 }
@@ -30,7 +31,8 @@ func (svc *Service) StartWorkers(
 		muxerToNetwork: svc.MuxerToNetwork,
 		networkToMuxer: *svc.NetworkToMuxer,
 	}
-	manager.StartWorker(ws.moveUpWorker) // TAKES conn ownership
+
+	manager.StartWorker(ws.moveUpWorker)
 	manager.StartWorker(ws.moveDownWorker)
 }
 
@@ -73,16 +75,13 @@ func (ws *workersState) moveUpWorker() {
 		// POSSIBLY BLOCK on the connection to read a new packet
 		pkt, err := ws.conn.ReadRawPacket()
 		if err != nil {
-			ws.logger.Infof("networkio: moveUpWorker: ReadRawPacket: %s", err.Error())
+			ws.logger.Debugf("networkio: moveUpWorker: ReadRawPacket: %s", err.Error())
 			return
 		}
 
-		// ws.logger.Infof("DEBUG < read %v bytes, select", len(pkt))
-
 		// POSSIBLY BLOCK on the channel to deliver the packet
 		select {
 		case ws.networkToMuxer <- pkt:
-			// ws.logger.Infof("< incoming %v bytes", len(pkt))
 		case <-ws.manager.ShouldShutdown():
 			return
 		}
@@ -100,11 +99,7 @@ func (ws *workersState) moveDownWorker() {
 	ws.logger.Debug("networkio: moveDownWorker: started")
 
 	for {
-		// While this channel receive could possibly block, the [ARCHITECTURE] is
-		// such that (1) the channel is buffered and (2) the channel sender should
-		// avoid blocking when inserting data into the channel.
-		//
-		// [ARCHITECTURE]: https://github.com/ooni/minivpn/blob/main/ARCHITECTURE.md
+		// POSSIBLY BLOCK when receiving from channel.
 		select {
 		case pkt := <-ws.muxerToNetwork:
 			// POSSIBLY BLOCK on the connection to write the packet

internal/networkio/stream.go

@@ -8,15 +8,15 @@ import (
 	"net"
 )
 
-// StreamConn wraps a stream socket and implements OpenVPN framing.
-type StreamConn struct {
+// streamConn wraps a stream socket and implements OpenVPN framing.
+type streamConn struct {
 	net.Conn
 }
 
-var _ FramingConn = &StreamConn{}
+var _ FramingConn = &streamConn{}
 
 // ReadRawPacket implements FramingConn
-func (c *StreamConn) ReadRawPacket() ([]byte, error) {
+func (c *streamConn) ReadRawPacket() ([]byte, error) {
 	lenbuf := make([]byte, 2)
 	if _, err := io.ReadFull(c.Conn, lenbuf); err != nil {
 		return nil, err
@@ -33,7 +33,7 @@ func (c *StreamConn) ReadRawPacket() ([]byte, error) {
 var ErrPacketTooLarge = errors.New("openvpn: packet too large")
 
 // WriteRawPacket implements FramingConn
-func (c *StreamConn) WriteRawPacket(pkt []byte) error {
+func (c *streamConn) WriteRawPacket(pkt []byte) error {
 	if len(pkt) > math.MaxUint16 {
 		return ErrPacketTooLarge
 	}

internal/workers/workers.go

@@ -1,4 +1,7 @@
 // Package workers contains code to manage workers.
+//
+// A worker is a goroutine running in the background that performs some
+// activity related to implementing the OpenVPN protocol.
 package workers
 
 import (
@@ -22,7 +25,7 @@ type Manager struct {
 	wg *sync.WaitGroup
 }
 
-// NewManager creates a new manager.
+// NewManager creates a new [*Manager].
 func NewManager() *Manager {
 	return &Manager{
 		shouldShutdown: make(chan any),
@@ -37,7 +40,7 @@ func (m *Manager) StartWorker(fx func()) {
 	go fx()
 }
 
-// OnWorkerDone must be called when a worker goroutine terminates.
+// OnWorkerDone MUST be called when a worker goroutine terminates.
 func (m *Manager) OnWorkerDone() {
 	m.wg.Done()
 }