Skip to content

Commit

Permalink
Add OIDC fix for ID token nonce claim validation (#6760)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jjngx
jjngx authored Nov 6, 2024
1 parent cb9204f commit b90209f
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 20 deletions.
2 changes: 1 addition & 1 deletion .github/scripts/variables.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ get_docker_md5() {
}

get_go_code_md5() {
find . -type f \( -name "*.go" -o -name go.mod -o -name go.sum -o -name "*.tmpl" -o -name "version.txt" \) -not -path "./site*" -exec md5sum {} + | LC_ALL=C sort | md5sum | awk '{ print $1 }'
find . -type f \( -name "*.go" -o -name go.mod -o -name go.sum -o -name "*.tmpl" -o -name "version.txt" -o -name "*.js" \) -not -path "./site*" -exec md5sum {} + | LC_ALL=C sort | md5sum | awk '{ print $1 }'
}

get_tests_md5() {
Expand Down
62 changes: 43 additions & 19 deletions internal/configs/oidc/openid_connect.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
*
* Copyright (C) 2020 Nginx, Inc.
*/
var newSession = false; // Used by oidcAuth() and validateIdToken()

export default {auth, codeExchange, validateIdToken, logout};

function retryOriginalRequest(r) {
Expand Down Expand Up @@ -32,8 +30,6 @@ function auth(r, afterSyncCheck) {
}

if (!r.variables.refresh_token || r.variables.refresh_token == "-") {
newSession = true;

// Check we have all necessary configuration variables (referenced only by njs)
var oidcConfigurables = ["authz_endpoint", "scopes", "hmac_key", "cookie_flags"];
var missingConfig = [];
Expand All @@ -54,7 +50,7 @@ function auth(r, afterSyncCheck) {

// Pass the refresh token to the /_refresh location so that it can be
// proxied to the IdP in exchange for a new id_token
r.subrequest("/_refresh", "token=" + r.variables.refresh_token,
r.subrequest("/_refresh", generateTokenRequestParams(r, "refresh_token"),
function(reply) {
if (reply.status != 200) {
// Refresh request failed, log the reason
Expand Down Expand Up @@ -142,7 +138,7 @@ function codeExchange(r) {

// Pass the authorization code to the /_token location so that it can be
// proxied to the IdP in exchange for a JWT
r.subrequest("/_token",idpClientAuth(r), function(reply) {
r.subrequest("/_token", generateTokenRequestParams(r, "authorization_code"), function(reply) {
if (reply.status == 504) {
r.error("OIDC timeout connecting to IdP when sending authorization code");
r.return(504);
Expand Down Expand Up @@ -200,7 +196,7 @@ function codeExchange(r) {

r.headersOut["Set-Cookie"] = "auth_token=" + r.variables.request_id + "; " + r.variables.oidc_cookie_flags;
r.return(302, r.variables.redirect_base + decodeURIComponent(r.variables.cookie_auth_redir));
}
}
);
} catch (e) {
r.error("OIDC authorization code sent but token response is not JSON. " + reply.responseText);
Expand Down Expand Up @@ -241,10 +237,9 @@ function validateIdToken(r) {
validToken = false;
}

// If we receive a nonce in the ID Token then we will use the auth_nonce cookies
// to check that the JWT can be validated as being directly related to the
// original request by this client. This mitigates against token replay attacks.
if (newSession) {
// According to OIDC Core 1.0 Section 2:
// "If present in the ID Token, Clients MUST verify that the nonce Claim Value is equal to the value of the nonce parameter sent in the Authentication Request."
if (r.variables.jwt_claim_nonce) {
var client_nonce_hash = "";
if (r.variables.cookie_auth_nonce) {
var c = require('crypto');
Expand All @@ -255,6 +250,9 @@ function validateIdToken(r) {
r.error("OIDC ID Token validation error: nonce from token (" + r.variables.jwt_claim_nonce + ") does not match client (" + client_nonce_hash + ")");
validToken = false;
}
} else if (!r.variables.refresh_token || r.variables.refresh_token == "-") {
r.error("OIDC ID Token validation error: missing nonce claim in ID Token during initial authentication.");
validToken = false;
}

if (validToken) {
Expand Down Expand Up @@ -297,7 +295,7 @@ function logout(r) {

// Construct logout arguments for RP-initiated logout
var logoutArgs = "?post_logout_redirect_uri=" + encodeURIComponent(logoutRedirectUrl) +
"&id_token_hint=" + encodeURIComponent(r.variables.session_jwt);
"&id_token_hint=" + encodeURIComponent(r.variables.session_jwt);
performLogout(r.variables.oidc_end_session_endpoint + logoutArgs);
} else {
// Fallback to traditional logout approach
Expand Down Expand Up @@ -337,12 +335,38 @@ function getAuthZArgs(r) {
return authZArgs;
}

function idpClientAuth(r) {
// If PKCE is enabled we have to use the code_verifier
if ( r.variables.oidc_pkce_enable == 1 ) {
r.variables.pkce_id = r.variables.arg_state;
return "code=" + r.variables.arg_code + "&code_verifier=" + r.variables.pkce_code_verifier;
} else {
return "code=" + r.variables.arg_code + "&client_secret=" + r.variables.oidc_client_secret;
function generateTokenRequestParams(r, grant_type) {
var body = "grant_type=" + grant_type + "&client_id=" + r.variables.oidc_client;

switch(grant_type) {
case "authorization_code":
body += "&code=" + r.variables.arg_code + "&redirect_uri=" + r.variables.redirect_base + r.variables.redir_location;
if (r.variables.oidc_pkce_enable == 1) {
r.variables.pkce_id = r.variables.arg_state;
body += "&code_verifier=" + r.variables.pkce_code_verifier;
}
break;
case "refresh_token":
body += "&refresh_token=" + r.variables.refresh_token;
break;
default:
r.error("Unsupported grant type: " + grant_type);
return;
}

var options = {
body: body,
method: "POST"
};

if (r.variables.oidc_pkce_enable != 1) {
if (r.variables.oidc_client_auth_method === "client_secret_basic") {
let auth_basic = "Basic " + Buffer.from(r.variables.oidc_client + ":" + r.variables.oidc_client_secret).toString('base64');
options.args = "secret_basic=" + auth_basic;
} else {
options.body += "&client_secret=" + r.variables.oidc_client_secret;
}
}

return options;
}

0 comments on commit b90209f

Please sign in to comment.