diff --git a/pkg/service/turn.go b/pkg/service/turn.go index 6ce70c773..47b82d03e 100644 --- a/pkg/service/turn.go +++ b/pkg/service/turn.go @@ -210,28 +210,28 @@ func (h *TURNAuthHandler) CreateUsername(apiKey string, pID livekit.ParticipantI return base62.EncodeToString(fmt.Appendf(nil, "%s|%s|%d", apiKey, pID, expiry)), expiry } -func (h *TURNAuthHandler) ParseUsername(username string) (apiKey string, pID livekit.ParticipantID, expiry int64, err error) { +func (h *TURNAuthHandler) ParseUsername(username string) (string, livekit.ParticipantID, int64, error) { decoded, err := base62.DecodeString(username) if err != nil { return "", "", 0, err } parts := strings.Split(string(decoded), "|") - if len(parts) != 2 && len(parts) != 3 { + if len(parts) != 3 { return "", "", 0, errors.New("invalid username") } - expiry = 0 - if len(parts) == 3 { - var err error - if expiry, err = strconv.ParseInt(parts[2], 10, 64); err != nil { - return "", "", 0, err - } + expiry, err := strconv.ParseInt(parts[2], 10, 64) + if err != nil { + return "", "", 0, err + } + if expiry == 0 { + return "", "", 0, ErrExpired } return parts[0], livekit.ParticipantID(parts[1]), expiry, nil } func (h *TURNAuthHandler) CreatePassword(apiKey string, pID livekit.ParticipantID, expiry int64) (string, error) { - if expiry != 0 && time.Now().After(time.Unix(expiry, 0)) { + if expiry == 0 || time.Now().After(time.Unix(expiry, 0)) { return "", ErrExpired } return h.computePassword(apiKey, pID, expiry) @@ -243,10 +243,7 @@ func (h *TURNAuthHandler) computePassword(apiKey string, pID livekit.Participant return "", ErrInvalidAPIKey } - keyInput := fmt.Sprintf("%s|%s", secret, pID) - if expiry != 0 { - keyInput = fmt.Sprintf("%s|%s|%d", secret, pID, expiry) - } + keyInput := fmt.Sprintf("%s|%s|%d", secret, pID, expiry) sum := sha256.Sum256([]byte(keyInput)) return base62.EncodeToString(sum[:]), nil @@ -259,26 +256,26 @@ func (h *TURNAuthHandler) HandleAuth(ra *turn.RequestAttributes) (userID string, return "", nil, false } parts := strings.Split(string(decoded), "|") - if len(parts) != 2 && len(parts) != 3 { + if len(parts) != 3 { return "", nil, false } - expiry := int64(0) - if len(parts) == 3 { - var err error - if expiry, err = strconv.ParseInt(parts[2], 10, 64); err != nil { + expiry, err := strconv.ParseInt(parts[2], 10, 64) + if err != nil { + return "", nil, false + } + if expiry == 0 { + return "", nil, false + } + expiryTime := time.Unix(expiry, 0) + if time.Now().After(expiryTime) { + // TTL only applies to initial allocation. Refresh / CreatePermission / + // ChannelBind / Send / Data requests are still authenticated against the + // username/password but skip the TTL check so long-running sessions can + // keep refreshing past the credential expiry. + if ra.Method == stun.MethodAllocate { + logger.Infow("TURN credential expired", "username", decoded, "participantID", parts[1], "expiry", expiryTime, "method", ra.Method) return "", nil, false } - expiryTime := time.Unix(expiry, 0) - if time.Now().After(expiryTime) { - // TTL only applies to initial allocation. Refresh / CreatePermission / - // ChannelBind / Send / Data requests are still authenticated against the - // username/password but skip the TTL check so long-running sessions can - // keep refreshing past the credential expiry. - if ra.Method == stun.MethodAllocate { - logger.Infow("TURN credential expired", "username", decoded, "participantID", parts[1], "expiry", expiryTime, "method", ra.Method) - return "", nil, false - } - } } password, err := h.computePassword(parts[0], livekit.ParticipantID(parts[1]), expiry) if err != nil { diff --git a/pkg/service/turn_test.go b/pkg/service/turn_test.go index 7f8d69601..32abd28d8 100644 --- a/pkg/service/turn_test.go +++ b/pkg/service/turn_test.go @@ -15,9 +15,11 @@ package service import ( + "fmt" "net" "testing" + "github.com/jxskiss/base62" "github.com/pion/stun/v3" "github.com/pion/turn/v5" "github.com/stretchr/testify/require" @@ -126,3 +128,81 @@ func TestTURNAuthHandler_HandleAuth_WrongUsernameRejected(t *testing.T) { }) require.False(t, ok) } + +func TestTURNAuthHandler_HandleAuth_TwoPartUsernameRejected(t *testing.T) { + h := newTestTurnAuthHandler() + pID := livekit.ParticipantID("PA_two_part") + + username := base62.EncodeToString(fmt.Appendf(nil, "%s|%s", turnTestAPIKey, pID)) + + for _, method := range []stun.Method{ + stun.MethodAllocate, + stun.MethodRefresh, + stun.MethodCreatePermission, + stun.MethodChannelBind, + stun.MethodSend, + } { + t.Run(method.String(), func(t *testing.T) { + _, _, ok := h.HandleAuth(&turn.RequestAttributes{ + Username: username, + Realm: LivekitRealm, + SrcAddr: &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 1234}, + Method: method, + }) + require.False(t, ok, "Two-part username must be rejected") + }) + } +} + +func TestTURNAuthHandler_HandleAuth_ZeroExpiryRejected(t *testing.T) { + h := newTestTurnAuthHandler() + pID := livekit.ParticipantID("PA_zero_expiry") + + username := base62.EncodeToString(fmt.Appendf(nil, "%s|%s|%d", turnTestAPIKey, pID, 0)) + + for _, method := range []stun.Method{ + stun.MethodAllocate, + stun.MethodRefresh, + stun.MethodCreatePermission, + stun.MethodChannelBind, + stun.MethodSend, + } { + t.Run(method.String(), func(t *testing.T) { + _, _, ok := h.HandleAuth(&turn.RequestAttributes{ + Username: username, + Realm: LivekitRealm, + SrcAddr: &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 1234}, + Method: method, + }) + require.False(t, ok, "Username with expiry=0 must be rejected") + }) + } +} + +func TestTURNAuthHandler_ParseUsername_TwoPartRejected(t *testing.T) { + h := newTestTurnAuthHandler() + pID := livekit.ParticipantID("PA_parse_two_part") + + username := base62.EncodeToString(fmt.Appendf(nil, "%s|%s", turnTestAPIKey, pID)) + + _, _, _, err := h.ParseUsername(username) + require.Error(t, err) +} + +func TestTURNAuthHandler_ParseUsername_ZeroExpiryRejected(t *testing.T) { + h := newTestTurnAuthHandler() + pID := livekit.ParticipantID("PA_parse_zero_expiry") + + username := base62.EncodeToString(fmt.Appendf(nil, "%s|%s|%d", turnTestAPIKey, pID, 0)) + + _, _, _, err := h.ParseUsername(username) + require.ErrorIs(t, err, ErrExpired) +} + +func TestTURNAuthHandler_CreatePassword_ZeroExpiryRejected(t *testing.T) { + h := newTestTurnAuthHandler() + pID := livekit.ParticipantID("PA_password_zero_expiry") + + _, err := h.CreatePassword(turnTestAPIKey, pID, 0) + require.ErrorIs(t, err, ErrExpired) +} diff --git a/test/singlenode_test.go b/test/singlenode_test.go index c6345a27f..6d571dadf 100644 --- a/test/singlenode_test.go +++ b/test/singlenode_test.go @@ -27,6 +27,7 @@ import ( "testing" "time" + "github.com/jxskiss/base62" "github.com/pion/sdp/v3" "github.com/pion/stun/v3" "github.com/pion/turn/v5" @@ -1432,12 +1433,13 @@ func TestTurnAuthFailure(t *testing.T) { require.NoError(t, err) require.NotEqual(t, validPassword, mismatchedExpiryPassword) - // password whose hash was generated without an expiry component (the - // pre-expiry form of the username/password pair); should not authenticate - // against a username that carries an expiry. - passwordWithoutExpiry, err := authHandler.CreatePassword(testApiKey, pID, 0) - require.NoError(t, err) - require.NotEqual(t, validPassword, passwordWithoutExpiry) + // username carrying expiry=0 must be rejected outright; constructed + // directly because CreateUsername always stamps a real expiry. + zeroExpiryUsername := base62.EncodeToString(fmt.Appendf(nil, "%s|%s|%d", testApiKey, pID, 0)) + + // username with only apiKey|pID (no expiry component) is the legacy + // pre-expiry form and must be rejected. + twoPartUsername := base62.EncodeToString(fmt.Appendf(nil, "%s|%s", testApiKey, pID)) testCases := []struct { name string @@ -1470,9 +1472,14 @@ func TestTurnAuthFailure(t *testing.T) { password: mismatchedExpiryPassword, }, { - name: "password-missing-expiry", - username: validUsername, - password: passwordWithoutExpiry, + name: "zero-expiry-username", + username: zeroExpiryUsername, + password: validPassword, + }, + { + name: "two-part-username", + username: twoPartUsername, + password: validPassword, }, }