mirror of
https://github.com/lukaszraczylo/traefikoidc.git
synced 2026-10-18 04:47:28 +00:00
fixup! Security improvements have been implemented and verified across four main areas:
This commit is contained in:
@@ -90,6 +90,11 @@ func parseJWT(tokenString string) (*JWT, error) {
|
|||||||
// - algorithm matches expected value to prevent algorithm switching attacks
|
// - algorithm matches expected value to prevent algorithm switching attacks
|
||||||
// Returns an error if any validation fails.
|
// Returns an error if any validation fails.
|
||||||
func (j *JWT) Verify(issuerURL, clientID string) error {
|
func (j *JWT) Verify(issuerURL, clientID string) error {
|
||||||
|
// Debug logging of validation parameters
|
||||||
|
fmt.Printf("Validating token against:\nIssuer: %s\nClient ID: %s\n", issuerURL, clientID)
|
||||||
|
// Debug logging of token header
|
||||||
|
fmt.Printf("Token header: %+v\n", j.Header)
|
||||||
|
|
||||||
// Validate algorithm to prevent algorithm switching attacks
|
// Validate algorithm to prevent algorithm switching attacks
|
||||||
alg, ok := j.Header["alg"].(string)
|
alg, ok := j.Header["alg"].(string)
|
||||||
if !ok {
|
if !ok {
|
||||||
@@ -102,11 +107,14 @@ func (j *JWT) Verify(issuerURL, clientID string) error {
|
|||||||
"ES256": true, "ES384": true, "ES512": true,
|
"ES256": true, "ES384": true, "ES512": true,
|
||||||
}
|
}
|
||||||
if !supportedAlgs[alg] {
|
if !supportedAlgs[alg] {
|
||||||
return fmt.Errorf("unsupported algorithm")
|
return fmt.Errorf("unsupported algorithm: %s", alg)
|
||||||
}
|
}
|
||||||
|
|
||||||
claims := j.Claims
|
claims := j.Claims
|
||||||
|
|
||||||
|
// Debug logging of all claims
|
||||||
|
fmt.Printf("Token claims: %+v\n", claims)
|
||||||
|
|
||||||
iss, ok := claims["iss"].(string)
|
iss, ok := claims["iss"].(string)
|
||||||
if !ok {
|
if !ok {
|
||||||
return fmt.Errorf("missing 'iss' claim")
|
return fmt.Errorf("missing 'iss' claim")
|
||||||
@@ -146,9 +154,10 @@ func (j *JWT) Verify(issuerURL, clientID string) error {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Validate jti (JWT ID) claim if present to prevent replay attacks
|
// Validate jti (JWT ID) claim if present
|
||||||
if _, ok := claims["jti"].(string); !ok {
|
if jti, ok := claims["jti"].(string); ok {
|
||||||
return fmt.Errorf("missing 'jti' claim")
|
// Could add replay detection here if needed
|
||||||
|
_ = jti
|
||||||
}
|
}
|
||||||
|
|
||||||
sub, ok := claims["sub"].(string)
|
sub, ok := claims["sub"].(string)
|
||||||
@@ -167,6 +176,10 @@ func (j *JWT) Verify(issuerURL, clientID string) error {
|
|||||||
// - expectedAudience: The expected audience value
|
// - expectedAudience: The expected audience value
|
||||||
// Returns an error if validation fails.
|
// Returns an error if validation fails.
|
||||||
func verifyAudience(tokenAudience interface{}, expectedAudience string) error {
|
func verifyAudience(tokenAudience interface{}, expectedAudience string) error {
|
||||||
|
// Debug logging
|
||||||
|
fmt.Printf("Verifying audience:\nToken aud: %+v\nExpected: %s\n",
|
||||||
|
tokenAudience, expectedAudience)
|
||||||
|
|
||||||
switch aud := tokenAudience.(type) {
|
switch aud := tokenAudience.(type) {
|
||||||
case string:
|
case string:
|
||||||
if aud != expectedAudience {
|
if aud != expectedAudience {
|
||||||
@@ -196,8 +209,13 @@ func verifyAudience(tokenAudience interface{}, expectedAudience string) error {
|
|||||||
// - expectedIssuer: The expected issuer URL
|
// - expectedIssuer: The expected issuer URL
|
||||||
// Returns an error if validation fails.
|
// Returns an error if validation fails.
|
||||||
func verifyIssuer(tokenIssuer, expectedIssuer string) error {
|
func verifyIssuer(tokenIssuer, expectedIssuer string) error {
|
||||||
|
// Debug logging
|
||||||
|
fmt.Printf("Verifying issuer:\nToken iss: %s\nExpected: %s\n",
|
||||||
|
tokenIssuer, expectedIssuer)
|
||||||
|
|
||||||
if tokenIssuer != expectedIssuer {
|
if tokenIssuer != expectedIssuer {
|
||||||
return fmt.Errorf("invalid issuer")
|
return fmt.Errorf("invalid issuer (token: %s, expected: %s)",
|
||||||
|
tokenIssuer, expectedIssuer)
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
@@ -212,8 +230,25 @@ const clockSkewTolerance = 2 * time.Minute
|
|||||||
// Returns an error if the token has expired.
|
// Returns an error if the token has expired.
|
||||||
func verifyExpiration(expiration float64) error {
|
func verifyExpiration(expiration float64) error {
|
||||||
expirationTime := time.Unix(int64(expiration), 0)
|
expirationTime := time.Unix(int64(expiration), 0)
|
||||||
if time.Now().Add(clockSkewTolerance).After(expirationTime) {
|
// Truncate current time to seconds for consistent comparison
|
||||||
return fmt.Errorf("token has expired")
|
now := time.Now().Truncate(time.Second)
|
||||||
|
skewedNow := now.Add(clockSkewTolerance)
|
||||||
|
|
||||||
|
// Debug logging
|
||||||
|
fmt.Printf("Token exp: %v\nCurrent time: %v\nSkewed time: %v\nSkew: %v\n",
|
||||||
|
expirationTime.UTC(),
|
||||||
|
now.UTC(),
|
||||||
|
skewedNow.UTC(),
|
||||||
|
clockSkewTolerance)
|
||||||
|
|
||||||
|
// Allow tokens that expire exactly now
|
||||||
|
if expirationTime.Equal(now) {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
if skewedNow.After(expirationTime) {
|
||||||
|
return fmt.Errorf("token has expired (exp: %v, now: %v)",
|
||||||
|
expirationTime.UTC(), now.UTC())
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
@@ -225,8 +260,25 @@ func verifyExpiration(expiration float64) error {
|
|||||||
// Returns an error if the token was issued in the future.
|
// Returns an error if the token was issued in the future.
|
||||||
func verifyIssuedAt(issuedAt float64) error {
|
func verifyIssuedAt(issuedAt float64) error {
|
||||||
issuedAtTime := time.Unix(int64(issuedAt), 0)
|
issuedAtTime := time.Unix(int64(issuedAt), 0)
|
||||||
if time.Now().Add(-clockSkewTolerance).Before(issuedAtTime) {
|
// Truncate current time to seconds for consistent comparison
|
||||||
return fmt.Errorf("token used before issued")
|
now := time.Now().Truncate(time.Second)
|
||||||
|
skewedNow := now.Add(-clockSkewTolerance)
|
||||||
|
|
||||||
|
// Debug logging
|
||||||
|
fmt.Printf("Token iat: %v\nCurrent time: %v\nSkewed time: %v\nSkew: %v\n",
|
||||||
|
issuedAtTime.UTC(),
|
||||||
|
now.UTC(),
|
||||||
|
skewedNow.UTC(),
|
||||||
|
clockSkewTolerance)
|
||||||
|
|
||||||
|
// Allow tokens issued in the same second as current time
|
||||||
|
if issuedAtTime.Equal(now) {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
if skewedNow.Before(issuedAtTime) {
|
||||||
|
return fmt.Errorf("token used before issued (iat: %v, now: %v)",
|
||||||
|
issuedAtTime.UTC(), now.UTC())
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
@@ -238,8 +290,25 @@ func verifyIssuedAt(issuedAt float64) error {
|
|||||||
// Returns an error if the token is not yet valid.
|
// Returns an error if the token is not yet valid.
|
||||||
func verifyNotBefore(notBefore float64) error {
|
func verifyNotBefore(notBefore float64) error {
|
||||||
notBeforeTime := time.Unix(int64(notBefore), 0)
|
notBeforeTime := time.Unix(int64(notBefore), 0)
|
||||||
if time.Now().Add(-clockSkewTolerance).Before(notBeforeTime) {
|
// Truncate current time to seconds for consistent comparison
|
||||||
return fmt.Errorf("token not yet valid")
|
now := time.Now().Truncate(time.Second)
|
||||||
|
skewedNow := now.Add(-clockSkewTolerance)
|
||||||
|
|
||||||
|
// Debug logging
|
||||||
|
fmt.Printf("Token nbf: %v\nCurrent time: %v\nSkewed time: %v\nSkew: %v\n",
|
||||||
|
notBeforeTime.UTC(),
|
||||||
|
now.UTC(),
|
||||||
|
skewedNow.UTC(),
|
||||||
|
clockSkewTolerance)
|
||||||
|
|
||||||
|
// Allow tokens that become valid exactly now
|
||||||
|
if notBeforeTime.Equal(now) {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
if skewedNow.Before(notBeforeTime) {
|
||||||
|
return fmt.Errorf("token not yet valid (nbf: %v, now: %v)",
|
||||||
|
notBeforeTime.UTC(), now.UTC())
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
@@ -255,6 +324,9 @@ func verifyNotBefore(notBefore float64) error {
|
|||||||
// - alg: The signature algorithm identifier
|
// - alg: The signature algorithm identifier
|
||||||
// Returns an error if signature verification fails.
|
// Returns an error if signature verification fails.
|
||||||
func verifySignature(tokenString string, publicKeyPEM []byte, alg string) error {
|
func verifySignature(tokenString string, publicKeyPEM []byte, alg string) error {
|
||||||
|
// Debug logging
|
||||||
|
fmt.Printf("Verifying signature with algorithm: %s\n", alg)
|
||||||
|
|
||||||
// Split the token into its three parts
|
// Split the token into its three parts
|
||||||
parts := strings.Split(tokenString, ".")
|
parts := strings.Split(tokenString, ".")
|
||||||
if len(parts) != 3 {
|
if len(parts) != 3 {
|
||||||
|
|||||||
+30
-19
@@ -67,17 +67,22 @@ func (ts *TestSuite) Setup() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Create a test JWT token signed with the RSA private key
|
// Create a test JWT token signed with the RSA private key
|
||||||
|
// Create timestamps with proper clock skew
|
||||||
now := time.Now()
|
now := time.Now()
|
||||||
|
exp := now.Add(1 * time.Hour).Unix()
|
||||||
|
iat := now.Add(-2 * time.Minute).Unix() // Account for clock skew
|
||||||
|
nbf := now.Add(-2 * time.Minute).Unix() // Account for clock skew
|
||||||
|
|
||||||
ts.token, err = createTestJWT(ts.rsaPrivateKey, "RS256", "test-key-id", map[string]interface{}{
|
ts.token, err = createTestJWT(ts.rsaPrivateKey, "RS256", "test-key-id", map[string]interface{}{
|
||||||
"iss": "https://test-issuer.com",
|
"iss": "https://test-issuer.com",
|
||||||
"aud": "test-client-id",
|
"aud": "test-client-id",
|
||||||
"exp": now.Add(1 * time.Hour).Unix(),
|
"exp": exp,
|
||||||
"iat": now.Add(-5 * time.Minute).Unix(), // Set issued time in the past to handle clock skew
|
"iat": iat,
|
||||||
"nbf": now.Add(-5 * time.Minute).Unix(), // Set not before time in the past
|
"nbf": nbf,
|
||||||
"sub": "test-subject",
|
"sub": "test-subject",
|
||||||
"email": "[email protected]",
|
"email": "[email protected]",
|
||||||
"nonce": "test-nonce",
|
"nonce": "test-nonce",
|
||||||
"jti": generateRandomString(16), // Add JWT ID for replay protection
|
"jti": generateRandomString(16),
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
ts.t.Fatalf("Failed to create test JWT: %v", err)
|
ts.t.Fatalf("Failed to create test JWT: %v", err)
|
||||||
@@ -1443,6 +1448,12 @@ func TestServeHTTPRolesAndGroups(t *testing.T) {
|
|||||||
ts := &TestSuite{t: t}
|
ts := &TestSuite{t: t}
|
||||||
ts.Setup()
|
ts.Setup()
|
||||||
|
|
||||||
|
// Create consistent timestamps for all test cases
|
||||||
|
now := time.Now()
|
||||||
|
exp := now.Add(1 * time.Hour).Unix()
|
||||||
|
iat := now.Add(-2 * time.Minute).Unix() // Account for clock skew
|
||||||
|
nbf := now.Add(-2 * time.Minute).Unix() // Account for clock skew
|
||||||
|
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
allowedRolesAndGroups map[string]struct{}
|
allowedRolesAndGroups map[string]struct{}
|
||||||
@@ -1459,9 +1470,9 @@ func TestServeHTTPRolesAndGroups(t *testing.T) {
|
|||||||
claims: map[string]interface{}{
|
claims: map[string]interface{}{
|
||||||
"iss": "https://test-issuer.com",
|
"iss": "https://test-issuer.com",
|
||||||
"aud": "test-client-id",
|
"aud": "test-client-id",
|
||||||
"exp": time.Now().Add(1 * time.Hour).Unix(),
|
"exp": exp,
|
||||||
"iat": time.Now().Add(-5 * time.Minute).Unix(),
|
"iat": iat,
|
||||||
"nbf": time.Now().Add(-5 * time.Minute).Unix(),
|
"nbf": nbf,
|
||||||
"sub": "test-subject",
|
"sub": "test-subject",
|
||||||
"roles": []interface{}{"admin", "user"},
|
"roles": []interface{}{"admin", "user"},
|
||||||
"groups": []interface{}{"group1"},
|
"groups": []interface{}{"group1"},
|
||||||
@@ -1485,9 +1496,9 @@ func TestServeHTTPRolesAndGroups(t *testing.T) {
|
|||||||
claims: map[string]interface{}{
|
claims: map[string]interface{}{
|
||||||
"iss": "https://test-issuer.com",
|
"iss": "https://test-issuer.com",
|
||||||
"aud": "test-client-id",
|
"aud": "test-client-id",
|
||||||
"exp": time.Now().Add(1 * time.Hour).Unix(),
|
"exp": exp,
|
||||||
"iat": time.Now().Add(-5 * time.Minute).Unix(),
|
"iat": iat,
|
||||||
"nbf": time.Now().Add(-5 * time.Minute).Unix(),
|
"nbf": nbf,
|
||||||
"sub": "test-subject",
|
"sub": "test-subject",
|
||||||
"roles": []interface{}{"user"},
|
"roles": []interface{}{"user"},
|
||||||
"groups": []interface{}{"allowed-group"},
|
"groups": []interface{}{"allowed-group"},
|
||||||
@@ -1512,9 +1523,9 @@ func TestServeHTTPRolesAndGroups(t *testing.T) {
|
|||||||
claims: map[string]interface{}{
|
claims: map[string]interface{}{
|
||||||
"iss": "https://test-issuer.com",
|
"iss": "https://test-issuer.com",
|
||||||
"aud": "test-client-id",
|
"aud": "test-client-id",
|
||||||
"exp": time.Now().Add(1 * time.Hour).Unix(),
|
"exp": exp,
|
||||||
"iat": time.Now().Add(-5 * time.Minute).Unix(),
|
"iat": iat,
|
||||||
"nbf": time.Now().Add(-5 * time.Minute).Unix(),
|
"nbf": nbf,
|
||||||
"sub": "test-subject",
|
"sub": "test-subject",
|
||||||
"roles": []interface{}{"user"},
|
"roles": []interface{}{"user"},
|
||||||
"groups": []interface{}{"regular-group"},
|
"groups": []interface{}{"regular-group"},
|
||||||
@@ -1532,9 +1543,9 @@ func TestServeHTTPRolesAndGroups(t *testing.T) {
|
|||||||
claims: map[string]interface{}{
|
claims: map[string]interface{}{
|
||||||
"iss": "https://test-issuer.com",
|
"iss": "https://test-issuer.com",
|
||||||
"aud": "test-client-id",
|
"aud": "test-client-id",
|
||||||
"exp": time.Now().Add(1 * time.Hour).Unix(),
|
"exp": exp,
|
||||||
"iat": time.Now().Add(-5 * time.Minute).Unix(),
|
"iat": iat,
|
||||||
"nbf": time.Now().Add(-5 * time.Minute).Unix(),
|
"nbf": nbf,
|
||||||
"sub": "test-subject",
|
"sub": "test-subject",
|
||||||
"roles": []interface{}{"user"},
|
"roles": []interface{}{"user"},
|
||||||
"groups": []interface{}{"regular-group"},
|
"groups": []interface{}{"regular-group"},
|
||||||
@@ -1556,9 +1567,9 @@ func TestServeHTTPRolesAndGroups(t *testing.T) {
|
|||||||
claims: map[string]interface{}{
|
claims: map[string]interface{}{
|
||||||
"iss": "https://test-issuer.com",
|
"iss": "https://test-issuer.com",
|
||||||
"aud": "test-client-id",
|
"aud": "test-client-id",
|
||||||
"exp": time.Now().Add(1 * time.Hour).Unix(),
|
"exp": exp,
|
||||||
"iat": time.Now().Add(-5 * time.Minute).Unix(),
|
"iat": iat,
|
||||||
"nbf": time.Now().Add(-5 * time.Minute).Unix(),
|
"nbf": nbf,
|
||||||
"sub": "test-subject",
|
"sub": "test-subject",
|
||||||
"jti": generateRandomString(16),
|
"jti": generateRandomString(16),
|
||||||
},
|
},
|
||||||
|
|||||||
Reference in New Issue
Block a user