support multiple subjects in oidc ping

Validate the subject in an oidc ping against a list of logged in subjects.

This resolves the issue that multiple connected FRP clients with different
OIDC clients result in a failing ping. The ping would fail because the
subject in memory would be the value of the last logged in FRPC.

This change also changes the constructor of OidcAuthVerifier to take
a TokenVerifier interface. This will not change production behavior, but makes
testing easier because we can inject a mock verifier during testing.

Resolves: #4466
This commit is contained in:
Rob Kenis 2024-10-08 15:43:02 +02:00
parent fe4ca1b54e
commit 98658a9354
No known key found for this signature in database
3 changed files with 84 additions and 10 deletions

View File

@ -16,7 +16,6 @@ package auth
import ( import (
"fmt" "fmt"
v1 "github.com/fatedier/frp/pkg/config/v1" v1 "github.com/fatedier/frp/pkg/config/v1"
"github.com/fatedier/frp/pkg/msg" "github.com/fatedier/frp/pkg/msg"
) )
@ -50,7 +49,8 @@ func NewAuthVerifier(cfg v1.AuthServerConfig) (authVerifier Verifier) {
case v1.AuthMethodToken: case v1.AuthMethodToken:
authVerifier = NewTokenAuth(cfg.AdditionalScopes, cfg.Token) authVerifier = NewTokenAuth(cfg.AdditionalScopes, cfg.Token)
case v1.AuthMethodOIDC: case v1.AuthMethodOIDC:
authVerifier = NewOidcAuthVerifier(cfg.AdditionalScopes, cfg.OIDC) tokenVerifier := NewTokenVerifier(cfg.OIDC)
authVerifier = NewOidcAuthVerifier(cfg.AdditionalScopes, tokenVerifier)
} }
return authVerifier return authVerifier
} }

View File

@ -87,14 +87,18 @@ func (auth *OidcAuthProvider) SetNewWorkConn(newWorkConnMsg *msg.NewWorkConn) (e
return err return err
} }
type TokenVerifier interface {
Verify(context.Context, string) (*oidc.IDToken, error)
}
type OidcAuthConsumer struct { type OidcAuthConsumer struct {
additionalAuthScopes []v1.AuthScope additionalAuthScopes []v1.AuthScope
verifier *oidc.IDTokenVerifier verifier TokenVerifier
subjectFromLogin string subjectsFromLogin []string
} }
func NewOidcAuthVerifier(additionalAuthScopes []v1.AuthScope, cfg v1.AuthOIDCServerConfig) *OidcAuthConsumer { func NewTokenVerifier(cfg v1.AuthOIDCServerConfig) TokenVerifier {
provider, err := oidc.NewProvider(context.Background(), cfg.Issuer) provider, err := oidc.NewProvider(context.Background(), cfg.Issuer)
if err != nil { if err != nil {
panic(err) panic(err)
@ -105,9 +109,14 @@ func NewOidcAuthVerifier(additionalAuthScopes []v1.AuthScope, cfg v1.AuthOIDCSer
SkipExpiryCheck: cfg.SkipExpiryCheck, SkipExpiryCheck: cfg.SkipExpiryCheck,
SkipIssuerCheck: cfg.SkipIssuerCheck, SkipIssuerCheck: cfg.SkipIssuerCheck,
} }
return provider.Verifier(&verifierConf)
}
func NewOidcAuthVerifier(additionalAuthScopes []v1.AuthScope, verifier TokenVerifier) *OidcAuthConsumer {
return &OidcAuthConsumer{ return &OidcAuthConsumer{
additionalAuthScopes: additionalAuthScopes, additionalAuthScopes: additionalAuthScopes,
verifier: provider.Verifier(&verifierConf), verifier: verifier,
subjectsFromLogin: []string{},
} }
} }
@ -116,7 +125,9 @@ func (auth *OidcAuthConsumer) VerifyLogin(loginMsg *msg.Login) (err error) {
if err != nil { if err != nil {
return fmt.Errorf("invalid OIDC token in login: %v", err) return fmt.Errorf("invalid OIDC token in login: %v", err)
} }
auth.subjectFromLogin = token.Subject if !slices.Contains(auth.subjectsFromLogin, token.Subject) {
auth.subjectsFromLogin = append(auth.subjectsFromLogin, token.Subject)
}
return nil return nil
} }
@ -125,11 +136,11 @@ func (auth *OidcAuthConsumer) verifyPostLoginToken(privilegeKey string) (err err
if err != nil { if err != nil {
return fmt.Errorf("invalid OIDC token in ping: %v", err) return fmt.Errorf("invalid OIDC token in ping: %v", err)
} }
if token.Subject != auth.subjectFromLogin { if !slices.Contains(auth.subjectsFromLogin, token.Subject) {
return fmt.Errorf("received different OIDC subject in login and ping. "+ return fmt.Errorf("received different OIDC subject in login and ping. "+
"original subject: %s, "+ "original subjects: %s, "+
"new subject: %s", "new subject: %s",
auth.subjectFromLogin, token.Subject) auth.subjectsFromLogin, token.Subject)
} }
return nil return nil
} }

63
pkg/auth/oidc_test.go Normal file
View File

@ -0,0 +1,63 @@
package auth_test
import (
"context"
"github.com/coreos/go-oidc/v3/oidc"
"github.com/fatedier/frp/pkg/auth"
v1 "github.com/fatedier/frp/pkg/config/v1"
"github.com/fatedier/frp/pkg/msg"
"github.com/stretchr/testify/require"
"testing"
"time"
)
type mockTokenVerifier struct {
}
func (m *mockTokenVerifier) Verify(ctx context.Context, subject string) (*oidc.IDToken, error) {
return &oidc.IDToken{
Subject: subject,
}, nil
}
func TestPingWithEmptySubjectFromLoginFails(t *testing.T) {
r := require.New(t)
consumer := auth.NewOidcAuthVerifier([]v1.AuthScope{v1.AuthScopeHeartBeats}, &mockTokenVerifier{})
err := consumer.VerifyPing(&msg.Ping{
PrivilegeKey: "ping-without-login",
Timestamp: time.Now().UnixMilli(),
})
r.Error(err)
r.Contains(err.Error(), "received different OIDC subject in login and ping")
}
func TestPingAfterLoginWithNewSubjectSucceeds(t *testing.T) {
r := require.New(t)
consumer := auth.NewOidcAuthVerifier([]v1.AuthScope{v1.AuthScopeHeartBeats}, &mockTokenVerifier{})
err := consumer.VerifyLogin(&msg.Login{
PrivilegeKey: "ping-after-login",
})
r.NoError(err)
err = consumer.VerifyPing(&msg.Ping{
PrivilegeKey: "ping-after-login",
Timestamp: time.Now().UnixMilli(),
})
r.NoError(err)
}
func TestPingAfterLoginWithDifferentSubjectFails(t *testing.T) {
r := require.New(t)
consumer := auth.NewOidcAuthVerifier([]v1.AuthScope{v1.AuthScopeHeartBeats}, &mockTokenVerifier{})
err := consumer.VerifyLogin(&msg.Login{
PrivilegeKey: "login-with-first-subject",
})
r.NoError(err)
err = consumer.VerifyPing(&msg.Ping{
PrivilegeKey: "ping-with-different-subject",
Timestamp: time.Now().UnixMilli(),
})
r.Error(err)
r.Contains(err.Error(), "received different OIDC subject in login and ping")
}