Merge branch 'main' into refactor-baseline

merging after refactor-packetmuxer has been merged

internal/packetmuxer/service.go

@@ -10,15 +10,25 @@ import (
 // Service is the packetmuxer service. Make sure you initialize
 // the channels before invoking [Service.StartWorkers].
 type Service struct {
-	HardReset       chan any
-	NotifyTLS       *chan *model.Notification
+	// HardReset receives requests to initiate a hard reset, that will start the openvpn handshake.
+	HardReset chan any
+
+	// NotifyTLS sends reset notifications to tlsstate.
+	NotifyTLS *chan *model.Notification
+
+	// MuxerToReliable moves packets up to reliabletransport.
 	MuxerToReliable *chan *model.Packet
-	MuxerToData     *chan *model.Packet
-	// DataOrControlToMuxer moves packets down from reliable or from dataChannel
+
+	// MuxerToData moves packets up to the datachannel.
+	MuxerToData *chan *model.Packet
+
+	// DataOrControlToMuxer moves packets down from the reliabletransport or datachannel.
 	DataOrControlToMuxer chan *model.Packet
-	// MuxerToNetwork moves bytes down
+
+	// MuxerToNetwork moves bytes down to the networkio layer below us.
 	MuxerToNetwork *chan []byte
-	// NetworkToMuxer moves bytes up
+
+	// NetworkToMuxer moves bytes up to us from the networkio layer below.
 	NetworkToMuxer chan []byte
 }
 
@@ -47,15 +57,15 @@ func (s *Service) StartWorkers(
 	workersManager.StartWorker(ws.moveDownWorker)
 }
 
-// workersState contains the reliable transport workers state.
+// workersState contains the reliabletransport workers state.
 type workersState struct {
 	// logger is the logger to use
 	logger model.Logger
 
 	// hardReset is the channel posted to force a hard reset.
 	hardReset <-chan any
 
-	// notifyTLS is used to send notifications to the TLS state service.
+	// notifyTLS is used to send notifications to the TLS service.
 	notifyTLS chan<- *model.Notification
 
 	// dataOrControlToMuxer is the channel for reading all the packets traveling down the stack.
@@ -212,7 +222,7 @@ func (ws *workersState) finishThreeWayHandshake(packet *model.Packet) error {
 	ws.sessionManager.SetRemoteSessionID(packet.LocalSessionID)
 
 	// we need to manually ACK because the reliable layer is above us
-	ws.logger.Infof(
+	ws.logger.Debugf(
 		"< %s localID=%x remoteID=%x [%d bytes]",
 		packet.Opcode,
 		packet.LocalSessionID,
@@ -266,7 +276,7 @@ func (ws *workersState) serializeAndEmit(packet *model.Packet) error {
 		return workers.ErrShutdown
 	}
 
-	ws.logger.Infof(
+	ws.logger.Debugf(
 		"> %s localID=%x remoteID=%x [%d bytes]",
 		packet.Opcode,
 		packet.LocalSessionID,

internal/session/datachannelkey.go

@@ -12,6 +12,7 @@ import (
 // the short key_id that is passed in the lower 3 bits if a packet header.
 // The setup of the keys for a given data channel (that is, for every key_id)
 // is made by expanding the keysources using the prf function.
+//
 // Do note that we are not yet implementing key renegotiation - but the index
 // is provided for convenience when/if we support that in the future.
 type DataChannelKey struct {

internal/session/doc.go

@@ -1 +1,4 @@
+// Package session keeps state for the application, including internal state
+// transitions for the OpenVPN protocol, data channel keys, and all the state
+// pertaining to the different packet counters.
 package session

internal/session/keysource.go

@@ -21,7 +21,7 @@ type KeySource struct {
 	PreMaster [48]byte
 }
 
-// Bytes returns the byte representation of a keySource.
+// Bytes returns the byte representation of a [KeySource].
 func (k *KeySource) Bytes() []byte {
 	buf := &bytes.Buffer{}
 	buf.Write(k.PreMaster[:])

internal/session/manager.go

@@ -85,24 +85,30 @@ type Manager struct {
 	remoteSessionID      optional.Value[model.SessionID]
 	tunnelInfo           model.TunnelInfo
 
-	// TODO(ainghazal): look for a better way of signaling when we're ready
-	Ready chan (any)
+	// Ready is a channel where we signal that we can start accepting data, because we've
+	// successfully generated key material for the data channel.
+	// TODO(ainghazal): find a better way?
+	Ready chan any
 }
 
 // NewManager returns a [Manager] ready to be used.
 func NewManager(logger model.Logger) (*Manager, error) {
 	key0 := &DataChannelKey{}
 	sessionManager := &Manager{
-		keyID:             0,
-		keys:              []*DataChannelKey{key0},
-		localSessionID:    [8]byte{},
-		logger:            logger,
-		mu:                sync.Mutex{},
-		negState:          0,
-		remoteSessionID:   optional.None[model.SessionID](),
-		tunnelInfo:        model.TunnelInfo{},
-		Ready:             make(chan any),
+		keyID:           0,
+		keys:            []*DataChannelKey{key0},
+		localSessionID:  [8]byte{},
+		logger:          logger,
+		mu:              sync.Mutex{},
+		negState:        0,
+		remoteSessionID: optional.None[model.SessionID](),
+		tunnelInfo:      model.TunnelInfo{},
+
+		// empirically, it seems that the reference OpenVPN server misbehaves if we initialize
+		// the data packet ID counter to zero.
 		localDataPacketID: 1,
+
+		Ready: make(chan any),
 	}
 
 	randomBytes, err := randomFn(8)