OpenConext / Stepup-Project

Managing issues for Stepup-* projects
0 stars 0 forks source link

Store last login in browser #393

Closed phavekes closed 2 days ago

phavekes commented 2 days ago

This issue is imported from pivotal - Originaly created at Sep 28, 2022 by Peter Havekes

When a user authenticates using a token (not SSO) ; Use a (persistent TBD) cookie to store:

The goal is to store all in information that is required to determine if the SSO session is valid in this cookie. We do not want to use the PHP session cookie for this purpose, but a separate cookie for this purpose only.

Store this data in an encrypted and authenticated cookie in the browser. The cookie should be valid for the SSO-session lifetime this is configured in the gateway. Set this cookie only if the user successfully authenticated the actual token. Do not set the cookie during a SSO authentication. See #183511012 for when NOT to set this cookie. Each eligible token authentication sets the SSO cookie, thereby overwriting a previous value of the SSO cookie.

The cookie must be scoped to the hostname of the gateway so that only the gateway receives this cookie from the user\'s browser.

Open Q&A (see comments below for discussion):

  1. Do we want to use a persistent cookie or do we want to use a session cookie? I.e. must the SSO session end when the user closes their browser or not? Make configurable: enum: persistent|session
  2. Name of the SSO cookie Make configurabel: string Note that the cookie name (sh|c)ould follow RFC 6265 (page 17) name convention.
  3. What mechanism to use to encrypt and sign (and decrypt and verify) the SSO cookie? Does Symfony offer a suitable mechanism. Either @pmeulen will provide pseudo-code to implement in vanilla PHP. Or we might use the Halite library to aid with the encryption/signing task.
  4. Do we want to add a SSO session ID to the cookie so that we can mention this ID in the logs? Instead of a generated id we decided upon adding a hash of the product of the contents of the cookie. Allowing to check against the current \'fingerprint\' of the current state of the cookie

Est: 10..12h

phavekes commented 2 days ago

Regarding the open questions:

  1. I think we already decided upon using a persistent cookie. I think this will be appreciated by the users. And as long as the cookie lifetime is not extremely long, this should balance the additional security concerns. The added security measures (auth & encryption of the cookie) should make hijacking attempts increasingly more difficult and maybe impossible given the cookie lifetime.
  2. I have no great preference to what cookie name we use. Maybe It\'s usefull to make it a configuration option? If that is not desired, we might simply want to call it: stepup.sso_2fa?
  3. Symfony only implements an interface to the low level PHP cookie features. They implemented that in the HTTPFoundation component. If we want to do additional encryption and authentication layering on the body of the cookie. That should be implemented by ourselves. Or possibly by using a helper library like: Paragonie Halite
  4. We surely can log this information. How useful would this information prove to be? Given the fact we have a finite cookie lifetime. And will probably never see the cookies IRL, making a cross reference less useful? Or am I overlooking another obvious benefit? (Michiel Kodde - Oct 25, 2022)
phavekes commented 2 days ago
@1 The only reservation that I have is that the user might not expect a persistent cookie, because the IdP\'s SSO cookies are typically session cookies. (Pieter van der Meulen - Oct 26, 2022)
phavekes commented 2 days ago
@2 Agreed to make the cookie name configurable through parameters.yaml (Pieter van der Meulen - Oct 26, 2022)
phavekes commented 2 days ago
@3 Halite looks like a solid library, nice find! Does it adds enough in addition to using Sodium directly to warrant adding another dependency? (Pieter van der Meulen - Oct 26, 2022)
phavekes commented 2 days ago
@1 we decided upon creating a parameter config defining the cookie type: persistent/session. (Michiel Kodde - Oct 26, 2022)
phavekes commented 2 days ago
@4 An SSO cookie is basically a short lived token. I want accountability for when we provide these, and when we use these.

(Pieter van der Meulen - Oct 26, 2022)

phavekes commented 2 days ago

See https://www.pivotaltracker.com/story/show/183511341 and https://www.pivotaltracker.com/story/show/183511348 for logging (Peter Havekes - Oct 26, 2022)

phavekes commented 2 days ago
@4 Instead of adding a uuid for a cookie. It might be even more interesting to add a sha256 hash of the contents of the hash. This enables us to see if the contents of the cookie changed. Between us issuing it, and the user showing it again in an SSO context. (Michiel Kodde - Oct 26, 2022)
phavekes commented 2 days ago

I updated the description to summarize the discussion in the thread above. (Michiel Kodde - Nov 2, 2022)

phavekes commented 2 days ago
Questions regarding the crypto solution we are about to implement.

First let me describe the workflow I\'m proposing.

Encryption

Asterisks mark questions I have about the process.

  1. The Cookie value is created during the 2FA workflow. It consists of the values described in the story (identityid, tokenid, timestamp,..)
  2. That Cookie value is then serialized into a JSON encoded string (for pure convenience, when we need to deserialize later down the road. PHP\'s serialize/deserialize sucks)
  3. The JSON Cookie value then undergoes a symetric crypotographic authentication. Based on an authentication secret. Yielding a MAC
  4. The JSON Cookie value is then encrypted using symetric encryption, using an encryption key ****
  5. That encrypted cookie value is then stored in the actual cookie

Decryption

Basically the oposite flow from the encryption

  1. The cookie value is read from the HTTP request
  2. The value is decrypted using the encryption key
  3. The decrypted value is then verified using the MAC that was created in the authentication step.
  4. The decrypted value (JSON Cookie value) is deserialized back into a PHP object.

Questions:

  1. From a workflow perspective, does performing the authentication on the to be encrypted \'payload\' make sense. Or is running an authenticatoin on the already encrypted data the industry standard?
  2. The authentication secret can be a configured secret (configurable in parameters.yaml). Or maybe we could create one based on a product of the SF authentication. SF, IdentityId, and a salt?
  3. The MAC that results from the authentication needs to be stored for later use in the verification step. Should we store that in our Gateway state store?
  4. I propose we store an encryption key in the parameters.yaml of StepupGateway. Is that okay?

@phavekes @pmeulen (Michiel Kodde - Nov 10, 2022)

phavekes commented 2 days ago
@3. The MAC can be stored in the cookie

@4. Yes, the key can be in the paramaters.yaml

(Peter Havekes - Nov 10, 2022)

phavekes commented 2 days ago
@1. You can do encryption and authentication separately, this is probably what you are most familiar with. You then need to choose what to do first (encryption or authentication). Typically you\'d want to encrypt first and then sign, this is because many encryption algorithms are vulnerable to oracle attacks when decrypting unverified data, so what you propose is the wrong way around. However there are now also algorithms that do the authentication and encryption in one go. E.g. GCM, that is the type of algorithm that I propose we use.

@2. I want to use a separate authentication and encryption key that is only used for this purpose. I don\'t see a reason to derive the key using e.g. IdentityId. Salting is the job of the algorithm we choose. @3. A MAC is not sensitive so can be stored in the cookie, like @phavekes said. For this design we do not want to store anything related to the SSO cookie in a session store. @4: Yes, and as said in point 1 above, do not use it for anything else. (Pieter van der Meulen - Nov 10, 2022)

phavekes commented 2 days ago
Thanks for the feedback kind sir\'s!

As requested during standup: here\'s some pseudo code denoting the current situation.

Encryption

Where Crypto is the Paragonie Halite library class)


$cookieValue = \'JSON string containing the idenity, SF, timestamp,..\';
$encryptionKey = \'the encryption key configured in parameters.yaml\'
$authKey = \'the auth key configured in parameters.yaml\';

$mac = Crypto::authenticate($cookieValue, $authKey);
$encryptedData = Crypto::encrypt($cookieValue, $encryptionKey);
// Todo return a value object containing the MAC + Encrypted data
return $encryptedData;

Decryption

$decryptedData = Crypto::decrypt($cookieData, $encryptionKey);
$verified = Crypto::verify($decryptedData, $authKey, $mac);
if (!$verified) {
   // Detonate a bomb
}
return CookieValue::fromJson($decryptedData);

As already stated in the feedback.

phavekes commented 2 days ago
@michielkodde You\'re using Halite above. It\'s encryption is already authenticated, so a separate authenticate/verify is unnecessary. (Pieter van der Meulen - Nov 10, 2022)
phavekes commented 2 days ago

Ah, so the auth feature is there just for when you want to use that feature, and not encrypt your payload. (Michiel Kodde - Nov 10, 2022)

phavekes commented 2 days ago

Example encrypting and decrypting using paragonie/halite 4.8, initialising the secret key from a hex key.


<?php

require_once \'vendor/autoload.php\';

// We want to encrypt and authenticate (i.e. protect against tampering) the cookie value using a long lived key.
// We can only use pseudorandom nonces, so nonce uniqueness depends on PRNG strength and nonce size.
// For the purpose of protecting the SSO, protecting the cookie against tampering is critical
// hiding it\'s contents is for defense in depth

// For Halite 5.0 the min supported version is PHP 8.0
// Halite 4.8 is the latest version that works with PHP 7.2. The discussion below is for 4.8.
// Halite uses libsodium

use ParagonIE\Halite\Halite;
use ParagonIE\HiddenString\HiddenString;

// The secret key is used for the authenticated encryption (AE) of the SSO cookies, it is stored in the parameters.yaml of the 
// Stepup-Gateway. This secret may only be used for the AE of the SSO cookies.
// The secret key must be 256 bits (32 bytes)
// We use hex encoding to store the key in the configuration, so the key will be 64 hex digits long
$hex_encoded_secret_key_from_parameters = "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f";

// Convert the key from the configuration from hex to binary. sodium_hex2bin
$secret_key_from_parameters = sodium_hex2bin($hex_encoded_secret_key_from_parameters);
assert( strlen($secret_key_from_parameters) == 32 );

$secret_key = new \ParagonIE\Halite\Symmetric\EncryptionKey(new HiddenString($secret_key_from_parameters) );

$plaintext_in = new HiddenString("The SSO cookie value to encrypt and MAC");

// Halite always uses authenticated encryption. See: https://github.com/paragonie/halite/blob/v4.x/doc/Classes/Symmetric/Crypto.md#encrypt
// It uses XSalsa20 for encryption and BLAKE2b for message Authentication (MAC)
// The keys used for encryption and message authentication are derived from the secret key using a HKDF using a salt
// This means that learning either derived key cannot lead to learning the other derived key,
// or the secret key input in the the HKDF. Encrypting many messages using the same secret key is not a problem in this
// design. This makes it a much safer setup than using GCM with the secret key directly:
// - GCM has a relatively short nonce (96 bits)
// - An attacker that is in possession of two different GCM messages that were encrypted using the same key
//   can not only decrypt the two messages, but can also recover (parts) of the encryption key.

// Encryption:
$encrypted_and_authenticated_data = \ParagonIE\Halite\Symmetric\Crypto::encrypt(
    $plaintext_in,
    $secret_key,
    Halite::ENCODE_BASE64URLSAFE // The default
);
echo \'Ciphertext = \' . $encrypted_and_authenticated_data . "\n";

// Decryption
$plaintext_out = \ParagonIE\Halite\Symmetric\Crypto::decrypt(
    $encrypted_and_authenticated_data,
    $secret_key,
    Halite::ENCODE_BASE64URLSAFE // The default
);
echo \'Plaintext = \' . $plaintext_out->getString(). "\n";

// TODO: Handle exceptions
``` (Pieter van der Meulen - Nov 10, 2022)
phavekes commented 2 days ago

Thanks, my second implementation matched your code example almost perfectly. I only added the hex2bin encoding/decoding step to the setting/parsing of the encryption key. And in addition to that, I reused most of your comments. See: https://github.com/OpenConext/Stepup-Gateway/commit/15b445e9d091d62016722a390852b5ae512d970d for the results. (Michiel Kodde - Nov 15, 2022)