starred/pocket-id
1
0
Fork 0
mirror of https://github.com/pocket-id/pocket-id.git synced 2025-12-22 01:11:41 +03:00
Code Issues 121 Packages Projects Releases 16 Wiki Activity

fix: use constant time comparisons when validating PKCE challenges (#1047)

Browse Source
This commit is contained in:
Alessandro (Ale) Segala
2025-10-23 23:30:50 -07:00
committed by GitHub
parent 35d913f905
commit eb3963d0fc
2 changed files with 39 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
backend/internal/service/oidc_service.go
View File

@@ -3,6 +3,7 @@ package service
import ( import (
"context" "context"
"crypto/sha256" "crypto/sha256"
"crypto/subtle"
"crypto/tls" "crypto/tls"
"encoding/base64" "encoding/base64"
"encoding/json" "encoding/json"
@@ -394,7 +395,7 @@ func (s *OidcService) createTokenFromAuthorizationCode(ctx context.Context, inpu
// If the client is public or PKCE is enabled, the code verifier must match the code challenge // If the client is public or PKCE is enabled, the code verifier must match the code challenge
if client.IsPublic || client.PkceEnabled { if client.IsPublic || client.PkceEnabled {
if !s.validateCodeVerifier(input.CodeVerifier, *authorizationCodeMetaData.CodeChallenge, *authorizationCodeMetaData.CodeChallengeMethodSha256) { if !validateCodeVerifier(input.CodeVerifier, *authorizationCodeMetaData.CodeChallenge, *authorizationCodeMetaData.CodeChallengeMethodSha256) {
return CreatedTokens{}, &common.OidcInvalidCodeVerifierError{} return CreatedTokens{}, &common.OidcInvalidCodeVerifierError{}
} }
} }
@@ -1083,13 +1084,20 @@ func (s *OidcService) createAuthorizationCode(ctx context.Context, clientID stri
return randomString, nil return randomString, nil
} }
func (s *OidcService) validateCodeVerifier(codeVerifier, codeChallenge string, codeChallengeMethodSha256 bool) bool { func validateCodeVerifier(codeVerifier, codeChallenge string, codeChallengeMethodSha256 bool) bool {
if codeVerifier == "" || codeChallenge == "" { if codeVerifier == "" || codeChallenge == "" {
return false return false
} }
if !codeChallengeMethodSha256 { if !codeChallengeMethodSha256 {
return codeVerifier == codeChallenge return subtle.ConstantTimeCompare([]byte(codeVerifier), []byte(codeChallenge)) == 1
}
// Base64 URL decode the challenge
// If it's not valid base64url, fail the operation
codeChallengeBytes, err := base64.RawURLEncoding.DecodeString(codeChallenge)
if err != nil {
return false
} }
// Compute SHA-256 hash of the codeVerifier // Compute SHA-256 hash of the codeVerifier
@@ -1097,10 +1105,7 @@ func (s *OidcService) validateCodeVerifier(codeVerifier, codeChallenge string, c
h.Write([]byte(codeVerifier)) h.Write([]byte(codeVerifier))
codeVerifierHash := h.Sum(nil) codeVerifierHash := h.Sum(nil)
// Base64 URL encode the verifier hash return subtle.ConstantTimeCompare(codeVerifierHash, codeChallengeBytes) == 1
encodedVerifierHash := base64.RawURLEncoding.EncodeToString(codeVerifierHash)
return encodedVerifierHash == codeChallenge
} }
func (s *OidcService) getCallbackURL(client *model.OidcClient, inputCallbackURL string, tx *gorm.DB, ctx context.Context) (callbackURL string, err error) { func (s *OidcService) getCallbackURL(client *model.OidcClient, inputCallbackURL string, tx *gorm.DB, ctx context.Context) (callbackURL string, err error) {

27
backend/internal/service/oidc_service_test.go
View File

@@ -5,6 +5,8 @@ import (
"crypto/ecdsa" "crypto/ecdsa"
"crypto/elliptic" "crypto/elliptic"
"crypto/rand" "crypto/rand"
"crypto/sha256"
"encoding/base64"
"encoding/json" "encoding/json"
"net/http" "net/http"
"testing" "testing"
@@ -510,3 +512,28 @@ func TestOidcService_verifyClientCredentialsInternal(t *testing.T) {
}) })
}) })
} }
func TestValidateCodeVerifier_Plain(t *testing.T) {
require.False(t, validateCodeVerifier("", "", false))
require.False(t, validateCodeVerifier("", "", true))
t.Run("plain", func(t *testing.T) {
require.False(t, validateCodeVerifier("", "challenge", false))
require.False(t, validateCodeVerifier("verifier", "", false))
require.True(t, validateCodeVerifier("plainVerifier", "plainVerifier", false))
require.False(t, validateCodeVerifier("plainVerifier", "otherVerifier", false))
})
t.Run("SHA 256", func(t *testing.T) {
codeVerifier := "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk"
hash := sha256.Sum256([]byte(codeVerifier))
codeChallenge := base64.RawURLEncoding.EncodeToString(hash[:])
require.True(t, validateCodeVerifier(codeVerifier, codeChallenge, true))
require.False(t, validateCodeVerifier("wrongVerifier", codeChallenge, true))
require.False(t, validateCodeVerifier(codeVerifier, "!", true))
// Invalid base64
require.False(t, validateCodeVerifier("NOT!VALID", codeChallenge, true))
})
}