Release 2021.12.3

CHANGES.md

@@ -1,8 +1,16 @@
 **Experimental**: This is a new format for release notes. The format and availability is subject to change.
 
+## 2021.12.2
+### Bug Fixes
+- Fix logging when `quic` transport is used and UDP traffic is proxied.
+- FIPS compliant cloudflared binaries will now be released as separate artifacts. Recall that these are only for linux
+and amd64.
+
 ## 2021.12.1
-### Bug Fixe
- - Fixes Github issue #530 where cloudflared 2021.12.0 could not reach origins that were HTTPS and using certain encryption methods forbidden by FIPS compliance (such as Let's Encrypt certificates). To address this fix we have temporarily reverted FIPS compliance from amd64 linux binaries that was recently introduced (or fixed actually as it was never working before).
+### Bug Fixes
+ - Fixes Github issue #530 where cloudflared 2021.12.0 could not reach origins that were HTTPS and using certain encryption
+methods forbidden by FIPS compliance (such as Let's Encrypt certificates). To address this fix we have temporarily reverted
+FIPS compliance from amd64 linux binaries that was recently introduced (or fixed actually as it was never working before).
 
 ## 2021.12.0
 ### New Features

RELEASE_NOTES

@@ -1,3 +1,9 @@
+2021.12.3
+- 2021-12-22 TUN-5584: Changes for release 2021.12.2
+- 2021-12-22 TUN-5590: QUIC datagram max user payload is 1217 bytes
+- 2021-12-22 TUN-5593: Read full packet from UDP connection, even if it exceeds MTU of the transport. When packet length is greater than the MTU of the transport, we will silently drop packets (for now).
+- 2021-12-23 TUN-5597: Log session ID when session is terminated by edge
+
 2021.12.2
 - 2021-12-20 TUN-5571: Remove redundant session manager log, it's already logged in origin/tunnel.ServeQUIC
 - 2021-12-20 TUN-5570: Only log RPC server events at error level to reduce noise

connection/quic.go

@@ -29,8 +29,7 @@ const (
 	// HTTPMethodKey is used to get or set http method in QUIC ALPN if the underlying proxy connection type is HTTP.
 	HTTPMethodKey = "HttpMethod"
 	// HTTPHostKey is used to get or set http Method in QUIC ALPN if the underlying proxy connection type is HTTP.
-	HTTPHostKey          = "HttpHost"
-	MaxDatagramFrameSize = 1220
+	HTTPHostKey = "HttpHost"
 )
 
 // QUICConnection represents the type that facilitates Proxying via QUIC streams.
@@ -199,10 +198,8 @@ func (q *QUICConnection) serveUDPSession(session *datagramsession.Session, close
 		} else {
 			q.closeUDPSession(ctx, session.ID, "terminated without error")
 		}
-		q.logger.Debug().Err(err).Str("sessionID", session.ID.String()).Msg("session terminated")
-		return
 	}
-	q.logger.Debug().Err(err).Msg("Session terminated by edge")
+	q.logger.Debug().Err(err).Str("sessionID", session.ID.String()).Msg("Session terminated")
 }
 
 // closeUDPSession first unregisters the session from session manager, then it tries to unregister from edge

datagramsession/manager.go

@@ -95,7 +95,7 @@ func (m *manager) RegisterSession(ctx context.Context, sessionID uuid.UUID, orig
 }
 
 func (m *manager) registerSession(ctx context.Context, registration *registerSessionEvent) {
-	session := newSession(registration.sessionID, m.transport, registration.originProxy)
+	session := newSession(registration.sessionID, m.transport, registration.originProxy, m.log)
 	m.sessions[registration.sessionID] = session
 	registration.resultChan <- session
 }

datagramsession/session.go

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/google/uuid"
+	"github.com/rs/zerolog"
 )
 
 const (
@@ -17,7 +18,7 @@ func SessionIdleErr(timeout time.Duration) error {
 	return fmt.Errorf("session idle for %v", timeout)
 }
 
-// Each Session is a bidirectional pipe of datagrams between transport and dstConn
+// Session is a bidirectional pipe of datagrams between transport and dstConn
 // Currently the only implementation of transport is quic DatagramMuxer
 // Destination can be a connection with origin or with eyeball
 // When the destination is origin:
@@ -35,9 +36,10 @@ type Session struct {
 	// activeAtChan is used to communicate the last read/write time
 	activeAtChan chan time.Time
 	closeChan    chan error
+	log          *zerolog.Logger
 }
 
-func newSession(id uuid.UUID, transport transport, dstConn io.ReadWriteCloser) *Session {
+func newSession(id uuid.UUID, transport transport, dstConn io.ReadWriteCloser, log *zerolog.Logger) *Session {
 	return &Session{
 		ID:        id,
 		transport: transport,
@@ -47,6 +49,7 @@ func newSession(id uuid.UUID, transport transport, dstConn io.ReadWriteCloser) *
 		activeAtChan: make(chan time.Time, 2),
 		// capacity is 2 because close() and dstToTransport routine in Serve() can write to this channel
 		closeChan: make(chan error, 2),
+		log:       log,
 	}
 }
 
@@ -54,7 +57,8 @@ func (s *Session) Serve(ctx context.Context, closeAfterIdle time.Duration) (clos
 	go func() {
 		// QUIC implementation copies data to another buffer before returning https://github.com/lucas-clemente/quic-go/blob/v0.24.0/session.go#L1967-L1975
 		// This makes it safe to share readBuffer between iterations
-		readBuffer := make([]byte, s.transport.MTU())
+		const maxPacketSize = 1500
+		readBuffer := make([]byte, maxPacketSize)
 		for {
 			if err := s.dstToTransport(readBuffer); err != nil {
 				s.closeChan <- err
@@ -103,8 +107,15 @@ func (s *Session) dstToTransport(buffer []byte) error {
 	n, err := s.dstConn.Read(buffer)
 	s.markActive()
 	if n > 0 {
-		if err := s.transport.SendTo(s.ID, buffer[:n]); err != nil {
-			return err
+		if n <= int(s.transport.MTU()) {
+			err = s.transport.SendTo(s.ID, buffer[:n])
+		} else {
+			// drop packet for now, eventually reply with ICMP for PMTUD
+			s.log.Debug().
+				Str("session", s.ID.String()).
+				Int("len", n).
+				Uint("mtu", s.transport.MTU()).
+				Msg("dropped packet exceeding MTU")
 		}
 	}
 	return err

datagramsession/session_test.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"github.com/google/uuid"
+	"github.com/rs/zerolog"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/sync/errgroup"
 )
@@ -44,7 +45,8 @@ func testSessionReturns(t *testing.T, closeBy closeMethod, closeAfterIdle time.D
 		reqChan:  newDatagramChannel(1),
 		respChan: newDatagramChannel(1),
 	}
-	session := newSession(sessionID, transport, cfdConn)
+	log := zerolog.Nop()
+	session := newSession(sessionID, transport, cfdConn, &log)
 
 	ctx, cancel := context.WithCancel(context.Background())
 	sessionDone := make(chan struct{})
@@ -119,7 +121,8 @@ func testActiveSessionNotClosed(t *testing.T, readFromDst bool, writeToDst bool)
 		reqChan:  newDatagramChannel(100),
 		respChan: newDatagramChannel(100),
 	}
-	session := newSession(sessionID, transport, cfdConn)
+	log := zerolog.Nop()
+	session := newSession(sessionID, transport, cfdConn, &log)
 
 	startTime := time.Now()
 	activeUntil := startTime.Add(activeTime)
@@ -181,7 +184,7 @@ func testActiveSessionNotClosed(t *testing.T, readFromDst bool, writeToDst bool)
 
 func TestMarkActiveNotBlocking(t *testing.T) {
 	const concurrentCalls = 50
-	session := newSession(uuid.New(), nil, nil)
+	session := newSession(uuid.New(), nil, nil, nil)
 	var wg sync.WaitGroup
 	wg.Add(concurrentCalls)
 	for i := 0; i < concurrentCalls; i++ {

datagramsession/transport.go

@@ -8,6 +8,6 @@ type transport interface {
 	SendTo(sessionID uuid.UUID, payload []byte) error
 	// ReceiveFrom reads the next datagram from the transport
 	ReceiveFrom() (uuid.UUID, []byte, error)
-	// Max transmission unit of the transport
+	// Max transmission unit to receive from the transport
 	MTU() uint
 }

datagramsession/transport_test.go

@@ -23,7 +23,7 @@ func (mt *mockQUICTransport) ReceiveFrom() (uuid.UUID, []byte, error) {
 }
 
 func (mt *mockQUICTransport) MTU() uint {
-	return 1220
+	return 1217
 }
 
 func (mt *mockQUICTransport) newRequest(ctx context.Context, sessionID uuid.UUID, payload []byte) error {

quic/datagram.go

@@ -9,7 +9,9 @@ import (
 )
 
 const (
-	MaxDatagramFrameSize = 1220
+	// Max datagram frame size is limited to 1220 https://github.com/lucas-clemente/quic-go/blob/v0.24.0/internal/protocol/params.go#L138
+	// However, 3 more bytes are reserved https://github.com/lucas-clemente/quic-go/blob/v0.24.0/internal/wire/datagram_frame.go#L61
+	MaxDatagramFrameSize = 1217
 	sessionIDLen         = len(uuid.UUID{})
 )
 
@@ -34,7 +36,7 @@ func NewDatagramMuxer(quicSession quic.Session) (*DatagramMuxer, error) {
 func (dm *DatagramMuxer) SendTo(sessionID uuid.UUID, payload []byte) error {
 	if len(payload) > MaxDatagramFrameSize-sessionIDLen {
 		// TODO: TUN-5302 return ICMP packet too big message
-		return fmt.Errorf("origin UDP payload has %d bytes, which exceeds transport MTU %d", len(payload), MaxDatagramFrameSize-sessionIDLen)
+		return fmt.Errorf("origin UDP payload has %d bytes, which exceeds transport MTU %d", len(payload), dm.MTU())
 	}
 	msgWithID, err := SuffixSessionID(sessionID, payload)
 	if err != nil {
@@ -57,8 +59,9 @@ func (dm *DatagramMuxer) ReceiveFrom() (uuid.UUID, []byte, error) {
 	return ExtractSessionID(msg)
 }
 
+// Maximum application payload to send to / receive from QUIC datagram frame
 func (dm *DatagramMuxer) MTU() uint {
-	return MaxDatagramFrameSize
+	return uint(MaxDatagramFrameSize - sessionIDLen)
 }
 
 // Each QUIC datagram should be suffixed with session ID.

quic/datagram_test.go

@@ -1,10 +1,20 @@
 package quic
 
 import (
+	"bytes"
+	"context"
+	"crypto/rand"
+	"crypto/rsa"
+	"crypto/tls"
+	"crypto/x509"
+	"encoding/pem"
+	"math/big"
 	"testing"
 
 	"github.com/google/uuid"
+	"github.com/lucas-clemente/quic-go"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/sync/errgroup"
 )
 
 var (
@@ -39,3 +49,91 @@ func TestSuffixSessionIDError(t *testing.T) {
 	_, err = SuffixSessionID(testSessionID, msg)
 	require.Error(t, err)
 }
+
+func TestMaxDatagramPayload(t *testing.T) {
+	payload := make([]byte, MaxDatagramFrameSize-sessionIDLen)
+
+	quicConfig := &quic.Config{
+		KeepAlive:       true,
+		EnableDatagrams: true,
+	}
+	quicListener := newQUICListener(t, quicConfig)
+	defer quicListener.Close()
+
+	errGroup, ctx := errgroup.WithContext(context.Background())
+	// Run edge side of datagram muxer
+	errGroup.Go(func() error {
+		// Accept quic connection
+		quicSession, err := quicListener.Accept(ctx)
+		require.NoError(t, err)
+
+		muxer, err := NewDatagramMuxer(quicSession)
+		require.NoError(t, err)
+
+		sessionID, receivedPayload, err := muxer.ReceiveFrom()
+		require.NoError(t, err)
+		require.Equal(t, testSessionID, sessionID)
+		require.True(t, bytes.Equal(payload, receivedPayload))
+
+		return nil
+	})
+
+	// Run cloudflared side of datagram muxer
+	errGroup.Go(func() error {
+		tlsClientConfig := &tls.Config{
+			InsecureSkipVerify: true,
+			NextProtos:         []string{"argotunnel"},
+		}
+		// Establish quic connection
+		quicSession, err := quic.DialAddrEarly(quicListener.Addr().String(), tlsClientConfig, quicConfig)
+		require.NoError(t, err)
+
+		muxer, err := NewDatagramMuxer(quicSession)
+		require.NoError(t, err)
+
+		err = muxer.SendTo(testSessionID, payload)
+		require.NoError(t, err)
+
+		// Payload larger than transport MTU, should return an error
+		largePayload := append(payload, byte(1))
+		err = muxer.SendTo(testSessionID, largePayload)
+		require.Error(t, err)
+
+		return nil
+	})
+
+	require.NoError(t, errGroup.Wait())
+}
+
+func newQUICListener(t *testing.T, config *quic.Config) quic.Listener {
+	// Create a simple tls config.
+	tlsConfig := generateTLSConfig()
+
+	listener, err := quic.ListenAddr("127.0.0.1:0", tlsConfig, config)
+	require.NoError(t, err)
+
+	return listener
+}
+
+func generateTLSConfig() *tls.Config {
+	key, err := rsa.GenerateKey(rand.Reader, 1024)
+	if err != nil {
+		panic(err)
+	}
+	template := x509.Certificate{SerialNumber: big.NewInt(1)}
+	certDER, err := x509.CreateCertificate(rand.Reader, &template, &template, &key.PublicKey, key)
+	if err != nil {
+		panic(err)
+	}
+	keyPEM := pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key)})
+	certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER})
+
+	tlsCert, err := tls.X509KeyPair(certPEM, keyPEM)
+	if err != nil {
+		panic(err)
+	}
+	return &tls.Config{
+		Certificates: []tls.Certificate{tlsCert},
+		NextProtos:   []string{"argotunnel"},
+	}
+}