From 944d48e39c8fc28e2b4e7b90b3c6a0aa58591b91 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Wed, 15 Jul 2026 17:11:37 +0200 Subject: [PATCH] Remove deprecated relay Hello handshake and gob token decode --- combined/cmd/root.go | 2 +- relay/cmd/root.go | 2 +- relay/server/handshake.go | 84 +++------------------------ shared/relay/auth/allow/allow_all.go | 4 -- shared/relay/auth/hmac/token.go | 10 ---- shared/relay/auth/hmac/validator.go | 33 ----------- shared/relay/auth/validator.go | 11 +--- shared/relay/messages/auth/auth.go | 43 -------------- shared/relay/messages/message.go | 66 --------------------- shared/relay/messages/message_test.go | 28 ++------- 10 files changed, 18 insertions(+), 265 deletions(-) delete mode 100644 shared/relay/auth/hmac/validator.go delete mode 100644 shared/relay/messages/auth/auth.go diff --git a/combined/cmd/root.go b/combined/cmd/root.go index 31e0580fb..2b7956f11 100644 --- a/combined/cmd/root.go +++ b/combined/cmd/root.go @@ -226,7 +226,7 @@ func (s *serverInstances) createRelayServer(cfg *CombinedConfig, tlsSupport bool } hashedSecret := sha256.Sum256([]byte(cfg.Relay.AuthSecret)) - authenticator := auth.NewTimedHMACValidator(hashedSecret[:], 24*time.Hour) + authenticator := auth.NewTimedHMACValidator(hashedSecret[:]) relayCfg := relayServer.Config{ Meter: s.metricsServer.Meter, diff --git a/relay/cmd/root.go b/relay/cmd/root.go index b1949ca11..4dd1e6236 100644 --- a/relay/cmd/root.go +++ b/relay/cmd/root.go @@ -173,7 +173,7 @@ func execute(cmd *cobra.Command, args []string) error { } hashedSecret := sha256.Sum256([]byte(cobraConfig.AuthSecret)) - authenticator := auth.NewTimedHMACValidator(hashedSecret[:], 24*time.Hour) + authenticator := auth.NewTimedHMACValidator(hashedSecret[:]) cfg := server.Config{ Meter: metricsServer.Meter, diff --git a/relay/server/handshake.go b/relay/server/handshake.go index 067888406..f064b3501 100644 --- a/relay/server/handshake.go +++ b/relay/server/handshake.go @@ -5,14 +5,8 @@ import ( "fmt" "time" - log "github.com/sirupsen/logrus" - "github.com/netbirdio/netbird/relay/server/listener" "github.com/netbirdio/netbird/shared/relay/messages" - //nolint:staticcheck - "github.com/netbirdio/netbird/shared/relay/messages/address" - //nolint:staticcheck - authmsg "github.com/netbirdio/netbird/shared/relay/messages/auth" ) const ( @@ -23,55 +17,30 @@ const ( type Validator interface { Validate(any) error - // Deprecated: Use Validate instead. - ValidateHelloMsgType(any) error } -// preparedMsg contains the marshalled success response messages +// preparedMsg contains the marshalled success response message type preparedMsg struct { - responseHelloMsg []byte - responseAuthMsg []byte + responseAuthMsg []byte } func newPreparedMsg(instanceURL string) (*preparedMsg, error) { - rhm, err := marshalResponseHelloMsg(instanceURL) - if err != nil { - return nil, err - } - ram, err := messages.MarshalAuthResponse(instanceURL) if err != nil { return nil, fmt.Errorf("failed to marshal auth response msg: %w", err) } return &preparedMsg{ - responseHelloMsg: rhm, - responseAuthMsg: ram, + responseAuthMsg: ram, }, nil } -func marshalResponseHelloMsg(instanceURL string) ([]byte, error) { - addr := &address.Address{URL: instanceURL} - addrData, err := addr.Marshal() - if err != nil { - return nil, fmt.Errorf("failed to marshal response address: %w", err) - } - - //nolint:staticcheck - responseMsg, err := messages.MarshalHelloResponse(addrData) - if err != nil { - return nil, fmt.Errorf("failed to marshal hello response: %w", err) - } - return responseMsg, nil -} - type handshake struct { conn listener.Conn validator Validator preparedMsg *preparedMsg - handshakeMethodAuth bool - peerID *messages.PeerID + peerID *messages.PeerID } func (h *handshake) handshakeReceive(ctx context.Context) (*messages.PeerID, error) { @@ -93,17 +62,11 @@ func (h *handshake) handshakeReceive(ctx context.Context) (*messages.PeerID, err return nil, fmt.Errorf("determine message type from %s: %w", h.conn.RemoteAddr(), err) } - var peerID *messages.PeerID - switch msgType { - //nolint:staticcheck - case messages.MsgTypeHello: - peerID, err = h.handleHelloMsg(buf) - case messages.MsgTypeAuth: - h.handshakeMethodAuth = true - peerID, err = h.handleAuthMsg(buf) - default: + if msgType != messages.MsgTypeAuth { return nil, fmt.Errorf("invalid message type %d from %s", msgType, h.conn.RemoteAddr()) } + + peerID, err := h.handleAuthMsg(buf) if err != nil { return peerID, err } @@ -112,46 +75,17 @@ func (h *handshake) handshakeReceive(ctx context.Context) (*messages.PeerID, err } func (h *handshake) handshakeResponse(ctx context.Context) error { - var responseMsg []byte - if h.handshakeMethodAuth { - responseMsg = h.preparedMsg.responseAuthMsg - } else { - responseMsg = h.preparedMsg.responseHelloMsg - } - - if _, err := h.conn.Write(ctx, responseMsg); err != nil { + if _, err := h.conn.Write(ctx, h.preparedMsg.responseAuthMsg); err != nil { return fmt.Errorf("handshake response write to %s (%s): %w", h.peerID, h.conn.RemoteAddr(), err) } return nil } -func (h *handshake) handleHelloMsg(buf []byte) (*messages.PeerID, error) { - //nolint:staticcheck - peerID, authData, err := messages.UnmarshalHelloMsg(buf) - if err != nil { - return nil, fmt.Errorf("unmarshal hello message: %w", err) - } - - log.Warnf("peer %s (%s) is using deprecated initial message type", peerID, h.conn.RemoteAddr()) - - authMsg, err := authmsg.UnmarshalMsg(authData) - if err != nil { - return nil, fmt.Errorf("unmarshal auth message: %w", err) - } - - //nolint:staticcheck - if err := h.validator.ValidateHelloMsgType(authMsg.AdditionalData); err != nil { - return nil, fmt.Errorf("validate %s (%s): %w", peerID, h.conn.RemoteAddr(), err) - } - - return peerID, nil -} - func (h *handshake) handleAuthMsg(buf []byte) (*messages.PeerID, error) { rawPeerID, authPayload, err := messages.UnmarshalAuthMsg(buf) if err != nil { - return nil, fmt.Errorf("unmarshal hello message: %w", err) + return nil, fmt.Errorf("unmarshal auth message: %w", err) } if err := h.validator.Validate(authPayload); err != nil { diff --git a/shared/relay/auth/allow/allow_all.go b/shared/relay/auth/allow/allow_all.go index 2d30c59c9..3074b8b1d 100644 --- a/shared/relay/auth/allow/allow_all.go +++ b/shared/relay/auth/allow/allow_all.go @@ -8,7 +8,3 @@ type Auth struct { func (a *Auth) Validate(any) error { return nil } - -func (a *Auth) ValidateHelloMsgType(any) error { - return nil -} diff --git a/shared/relay/auth/hmac/token.go b/shared/relay/auth/hmac/token.go index 581b1d6fd..c908efbff 100644 --- a/shared/relay/auth/hmac/token.go +++ b/shared/relay/auth/hmac/token.go @@ -1,10 +1,8 @@ package hmac import ( - "bytes" "crypto/hmac" "encoding/base64" - "encoding/gob" "fmt" "hash" "strconv" @@ -18,14 +16,6 @@ type Token struct { Signature string } -func unmarshalToken(payload []byte) (Token, error) { - var creds Token - buffer := bytes.NewBuffer(payload) - decoder := gob.NewDecoder(buffer) - err := decoder.Decode(&creds) - return creds, err -} - // TimedHMAC generates a token with TTL and uses a pre-shared secret known to the relay server type TimedHMAC struct { secret string diff --git a/shared/relay/auth/hmac/validator.go b/shared/relay/auth/hmac/validator.go deleted file mode 100644 index b0b7542be..000000000 --- a/shared/relay/auth/hmac/validator.go +++ /dev/null @@ -1,33 +0,0 @@ -package hmac - -import ( - "crypto/sha256" - "fmt" - "time" - - log "github.com/sirupsen/logrus" -) - -type TimedHMACValidator struct { - *TimedHMAC -} - -func NewTimedHMACValidator(secret string, duration time.Duration) *TimedHMACValidator { - ta := NewTimedHMAC(secret, duration) - return &TimedHMACValidator{ - ta, - } -} - -func (a *TimedHMACValidator) Validate(credentials any) error { - b, ok := credentials.([]byte) - if !ok { - return fmt.Errorf("invalid credentials type") - } - c, err := unmarshalToken(b) - if err != nil { - log.Debugf("failed to unmarshal token: %s", err) - return err - } - return a.TimedHMAC.Validate(sha256.New, c) -} diff --git a/shared/relay/auth/validator.go b/shared/relay/auth/validator.go index 8e339bb2e..158b32e1b 100644 --- a/shared/relay/auth/validator.go +++ b/shared/relay/auth/validator.go @@ -1,28 +1,19 @@ package auth import ( - "time" - - auth "github.com/netbirdio/netbird/shared/relay/auth/hmac" authv2 "github.com/netbirdio/netbird/shared/relay/auth/hmac/v2" ) type TimedHMACValidator struct { authenticatorV2 *authv2.Validator - authenticator *auth.TimedHMACValidator } -func NewTimedHMACValidator(secret []byte, duration time.Duration) *TimedHMACValidator { +func NewTimedHMACValidator(secret []byte) *TimedHMACValidator { return &TimedHMACValidator{ authenticatorV2: authv2.NewValidator(secret), - authenticator: auth.NewTimedHMACValidator(string(secret), duration), } } func (a *TimedHMACValidator) Validate(credentials any) error { return a.authenticatorV2.Validate(credentials) } - -func (a *TimedHMACValidator) ValidateHelloMsgType(credentials any) error { - return a.authenticator.Validate(credentials) -} diff --git a/shared/relay/messages/auth/auth.go b/shared/relay/messages/auth/auth.go deleted file mode 100644 index 9c2511f2f..000000000 --- a/shared/relay/messages/auth/auth.go +++ /dev/null @@ -1,43 +0,0 @@ -// Deprecated: This package is deprecated and will be removed in a future release. -package auth - -import ( - "bytes" - "encoding/gob" - "fmt" -) - -type Algorithm int - -const ( - AlgoUnknown Algorithm = iota - AlgoHMACSHA256 - AlgoHMACSHA512 -) - -func (a Algorithm) String() string { - switch a { - case AlgoHMACSHA256: - return "HMAC-SHA256" - case AlgoHMACSHA512: - return "HMAC-SHA512" - default: - return "Unknown" - } -} - -type Msg struct { - AuthAlgorithm Algorithm - AdditionalData []byte -} - -func UnmarshalMsg(data []byte) (*Msg, error) { - var msg *Msg - - buf := bytes.NewBuffer(data) - dec := gob.NewDecoder(buf) - if err := dec.Decode(&msg); err != nil { - return nil, fmt.Errorf("decode Msg: %w", err) - } - return msg, nil -} diff --git a/shared/relay/messages/message.go b/shared/relay/messages/message.go index 54671f5df..e2c13ae12 100644 --- a/shared/relay/messages/message.go +++ b/shared/relay/messages/message.go @@ -42,10 +42,6 @@ const ( offsetAuthPeerID = sizeOfProtoHeader + sizeOfMagicByte headerTotalSizeAuth = sizeOfProtoHeader + headerSizeAuth - // hello message - headerSizeHello = sizeOfMagicByte + peerIDSize - headerSizeHelloResp = 0 - // transport headerSizeTransport = peerIDSize offsetTransportID = sizeOfProtoHeader @@ -113,7 +109,6 @@ func DetermineClientMessageType(msg []byte) (MsgType, error) { msgType := MsgType(msg[1]) switch msgType { case - MsgTypeHello, MsgTypeAuth, MsgTypeTransport, MsgTypeClose, @@ -148,67 +143,6 @@ func DetermineServerMessageType(msg []byte) (MsgType, error) { } } -// Deprecated: Use MarshalAuthMsg instead. -// MarshalHelloMsg initial hello message -// The Hello message is the first message sent by a client after establishing a connection with the Relay server. This -// message is used to authenticate the client with the server. The authentication is done using an HMAC method. -// The protocol does not limit to use HMAC, it can be any other method. If the authentication failed the server will -// close the network connection without any response. -func MarshalHelloMsg(peerID PeerID, additions []byte) ([]byte, error) { - msg := make([]byte, sizeOfProtoHeader+sizeOfMagicByte, sizeOfProtoHeader+headerSizeHello+len(additions)) - - msg[0] = byte(CurrentProtocolVersion) - msg[1] = byte(MsgTypeHello) - - copy(msg[sizeOfProtoHeader:sizeOfProtoHeader+sizeOfMagicByte], magicHeader) - - msg = append(msg, peerID[:]...) - msg = append(msg, additions...) - - return msg, nil -} - -// Deprecated: Use UnmarshalAuthMsg instead. -// UnmarshalHelloMsg extracts peerID and the additional data from the hello message. The Additional data is used to -// authenticate the client with the server. -func UnmarshalHelloMsg(msg []byte) (*PeerID, []byte, error) { - if len(msg) < sizeOfProtoHeader+headerSizeHello { - return nil, nil, ErrInvalidMessageLength - } - if !bytes.Equal(msg[sizeOfProtoHeader:sizeOfProtoHeader+sizeOfMagicByte], magicHeader) { - return nil, nil, errors.New("invalid magic header") - } - - peerID := PeerID(msg[sizeOfProtoHeader+sizeOfMagicByte : sizeOfProtoHeader+headerSizeHello]) - - return &peerID, msg[headerSizeHello:], nil -} - -// Deprecated: Use MarshalAuthResponse instead. -// MarshalHelloResponse creates a response message to the hello message. -// In case of success connection the server response with a Hello Response message. This message contains the server's -// instance URL. This URL will be used by choose the common Relay server in case if the peers are in different Relay -// servers. -func MarshalHelloResponse(additionalData []byte) ([]byte, error) { - msg := make([]byte, sizeOfProtoHeader, sizeOfProtoHeader+headerSizeHelloResp+len(additionalData)) - - msg[0] = byte(CurrentProtocolVersion) - msg[1] = byte(MsgTypeHelloResponse) - - msg = append(msg, additionalData...) - - return msg, nil -} - -// Deprecated: Use UnmarshalAuthResponse instead. -// UnmarshalHelloResponse extracts the additional data from the hello response message. -func UnmarshalHelloResponse(msg []byte) ([]byte, error) { - if len(msg) < sizeOfProtoHeader+headerSizeHelloResp { - return nil, ErrInvalidMessageLength - } - return msg, nil -} - // MarshalAuthMsg initial authentication message // The Auth message is the first message sent by a client after establishing a connection with the Relay server. This // message is used to authenticate the client with the server. The authentication is done using an HMAC method. diff --git a/shared/relay/messages/message_test.go b/shared/relay/messages/message_test.go index 59a89cad1..271a14bc7 100644 --- a/shared/relay/messages/message_test.go +++ b/shared/relay/messages/message_test.go @@ -4,28 +4,12 @@ import ( "testing" ) -func TestMarshalHelloMsg(t *testing.T) { - peerID := HashID("abdFAaBcawquEiCMzAabYosuUaGLtSNhKxz+") - msg, err := MarshalHelloMsg(peerID, nil) - if err != nil { - t.Fatalf("error: %v", err) - } - - msgType, err := DetermineClientMessageType(msg) - if err != nil { - t.Fatalf("error: %v", err) - } - - if msgType != MsgTypeHello { - t.Errorf("expected %d, got %d", MsgTypeHello, msgType) - } - - receivedPeerID, _, err := UnmarshalHelloMsg(msg) - if err != nil { - t.Fatalf("error: %v", err) - } - if receivedPeerID.String() != peerID.String() { - t.Errorf("expected %s, got %s", peerID, receivedPeerID) +func TestDetermineClientMessageTypeRejectsHello(t *testing.T) { + // The deprecated Hello message (type 1) is no longer accepted by the + // server, closing the pre-auth gob decode path (GHSA-v5w2-pqxj-6r94). + msg := []byte{byte(CurrentProtocolVersion), byte(MsgTypeHello)} + if _, err := DetermineClientMessageType(msg); err == nil { + t.Fatalf("expected hello message type to be rejected") } }