Merge pull request #2748 from vert-x3/backport/2746

Ensuring that cookie sessions are reusable across nodes

Author: vietj

vertx-web-session-stores/vertx-web-sstore-cookie/src/main/java/io/vertx/ext/web/sstore/cookie/CookieSessionStore.java

@@ -23,26 +23,48 @@
 
 /**
  * A SessionStore that uses a Cookie to store the session data. All data is stored in
- * encrypted form using {@code AES-256 with AES/CBC/PKCS5Padding}.
+ * encrypted form using {@code AES-256 with AES/GCM/NoPadding}.
  *
  * @author <a href="mailto:plopes@redhat.com">Paulo Lopes</a>
  */
 @VertxGen
 public interface CookieSessionStore extends SessionStore {
 
   /**
+   * @deprecated use {@link #create(Vertx, String)}
+   * 
    * Creates a CookieSessionStore.
+   * 
+   * This factory method is deprecated and will be removed in a future version.
+   * The salt value is ignored and should not be used. This was an artifact of
+   * the original implementation which used a different encryption scheme.
    *
-   * Cookie data will be encrypted using the given secret and salt. The secret as the name
+   * @param vertx a vert.x instance
+   * @param secret a secret to derive a secure private key
+   * @param salt ignored
+   * @return the store
+   */
+  @Deprecated
+  static CookieSessionStore create(Vertx vertx, String secret, Buffer salt) {
+    return create(vertx, secret);
+  }
+
+  /**
+   * Creates a CookieSessionStore.
+   *
+   * Cookie data will be encrypted using the given secret. The secret as the name
    * reflects, should never leave the server, otherwise user agents could tamper
-   * with the payload. The salt adds an extra later of security and should be a random.
+   * with the payload.
+   * 
+   * The choice of GCM, ensures that no (IV, Key) is reusable, which means that
+   * there is no need for a salt. Also encrypting the same session multiple times
+   * will render different outputs, which prevents rainbow attacks.
    *
    * @param vertx a vert.x instance
    * @param secret a secret to derive a secure private key
-   * @param salt a binary salt used in the key derivation
    * @return the store
    */
-  static CookieSessionStore create(Vertx vertx, String secret, Buffer salt) {
-    return new CookieSessionStoreImpl(vertx, secret, salt);
+  static CookieSessionStore create(Vertx vertx, String secret) {
+    return new CookieSessionStoreImpl(vertx, secret);
   }
 }

vertx-web-session-stores/vertx-web-sstore-cookie/src/main/java/io/vertx/ext/web/sstore/cookie/impl/CookieSession.java

@@ -22,8 +22,14 @@
 import javax.crypto.BadPaddingException;
 import javax.crypto.Cipher;
 import javax.crypto.IllegalBlockSizeException;
+import javax.crypto.NoSuchPaddingException;
+import javax.crypto.spec.GCMParameterSpec;
+import javax.crypto.spec.SecretKeySpec;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
+import java.security.NoSuchAlgorithmException;
+import java.security.InvalidAlgorithmParameterException;
+import java.security.InvalidKeyException;
 import java.util.Base64;
 
 /**
@@ -34,6 +40,11 @@ public class CookieSession extends AbstractSession {
   private static final Base64.Encoder BASE64_URL_ENCODER = Base64.getUrlEncoder().withoutPadding();
   private static final Base64.Decoder BASE64_URL_DECODER = Base64.getUrlDecoder();
 
+  private static final String AES_ALGORITHM_GCM = "AES/GCM/NoPadding";
+
+  private static final int IV_LENGTH = 12;
+  private static final int TAG_LENGTH = 16;
+
   public static String base64UrlEncode(byte[] bytes) {
     return BASE64_URL_ENCODER.encodeToString(bytes);
   }
@@ -44,23 +55,23 @@ public static byte[] base64UrlDecode(String base64) {
 
   private static final Charset UTF8 = StandardCharsets.UTF_8;
 
-  private final Cipher encrypt;
-  private final Cipher decrypt;
+  private final SecretKeySpec aesKey;
+  private final VertxContextPRNG prng;
   // track the original version
   private int oldVersion = 0;
   // track the original crc
   private int oldCrc = 0;
 
-  public CookieSession(Cipher encrypt, Cipher decrypt, VertxContextPRNG prng, long timeout, int length) {
+  public CookieSession(SecretKeySpec aesKey, VertxContextPRNG prng, long timeout, int length) {
     super(prng, timeout, length);
-    this.encrypt = encrypt;
-    this.decrypt = decrypt;
+    this.prng = prng;
+    this.aesKey = aesKey;
   }
 
-  public CookieSession(Cipher encrypt, Cipher decrypt, VertxContextPRNG prng) {
+  public CookieSession(SecretKeySpec aesKey, VertxContextPRNG prng) {
     super(prng);
-    this.encrypt = encrypt;
-    this.decrypt = decrypt;
+    this.prng = prng;
+    this.aesKey = aesKey;
   }
 
   @Override
@@ -76,8 +87,8 @@ public String value() {
     writeDataToBuffer(buff);
 
     try {
-      return base64UrlEncode(encrypt.doFinal(buff.getBytes()));
-    } catch (IllegalBlockSizeException | BadPaddingException e) {
+      return encrypt(buff);
+    } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException | InvalidAlgorithmParameterException | IllegalBlockSizeException | BadPaddingException e) {
       throw new RuntimeException(e);
     }
   }
@@ -99,21 +110,8 @@ protected CookieSession setValue(String payload) {
       throw new NullPointerException();
     }
 
-//    String[] tokens = payload.split("\\.");
-//    if (tokens.length != 2) {
-//      // no signature present, force a regeneration
-//      // by claiming this session as invalid
-//      return null;
-//    }
-//
-//    String signature = base64UrlEncode(mac.doFinal(tokens[0].getBytes(StandardCharsets.US_ASCII)));
-//
-//    if(!signature.equals(tokens[1])) {
-//      throw new RuntimeException("Session data was Tampered!");
-//    }
-
     try {
-      final Buffer buffer = Buffer.buffer(decrypt.doFinal(base64UrlDecode(payload)));
+      final Buffer buffer = decrypt(payload);
 
       // reconstruct the session
       int pos = 0;
@@ -133,7 +131,7 @@ protected CookieSession setValue(String payload) {
       // defaults
       oldVersion = version();
       oldCrc = crc();
-    } catch (IllegalBlockSizeException | BadPaddingException e) {
+    } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException | InvalidAlgorithmParameterException | IllegalBlockSizeException | BadPaddingException e) {
       // this is a bad session, force a regeneration
       return null;
     }
@@ -144,4 +142,43 @@ protected CookieSession setValue(String payload) {
   int oldVersion() {
     return oldVersion;
   }
+
+  private String encrypt(Buffer data) throws NoSuchAlgorithmException, NoSuchPaddingException, InvalidKeyException, InvalidAlgorithmParameterException, IllegalBlockSizeException, BadPaddingException {
+    // Initialization Vector
+    byte[] iv = new byte[IV_LENGTH];
+    prng.nextBytes(iv);
+
+    // get a cipher
+    Cipher cipher = Cipher.getInstance(AES_ALGORITHM_GCM);
+    GCMParameterSpec gcmSpec = new GCMParameterSpec(TAG_LENGTH * 8, iv);
+    cipher.init(Cipher.ENCRYPT_MODE, aesKey, gcmSpec);
+
+    byte[] encryptedBytes = cipher.doFinal(data.getBytes());
+
+    // combine IV and cipher data
+    byte[] combinedIvAndCipherText = new byte[iv.length + encryptedBytes.length];
+    System.arraycopy(iv, 0, combinedIvAndCipherText, 0, iv.length);
+    System.arraycopy(encryptedBytes, 0, combinedIvAndCipherText, iv.length, encryptedBytes.length);
+
+    return base64UrlEncode(combinedIvAndCipherText);
+  }
+
+  private Buffer decrypt(String data) throws NoSuchAlgorithmException, NoSuchPaddingException, InvalidKeyException, InvalidAlgorithmParameterException, IllegalBlockSizeException, BadPaddingException {
+    byte[] decodedCipherText = base64UrlDecode(data);
+
+    // extract the IV
+    byte[] iv = new byte[IV_LENGTH];
+    System.arraycopy(decodedCipherText, 0, iv, 0, iv.length);
+    byte[] encryptedText = new byte[decodedCipherText.length - IV_LENGTH];
+    System.arraycopy(decodedCipherText, IV_LENGTH, encryptedText, 0, encryptedText.length);
+
+    // get a cipher
+    Cipher cipher = Cipher.getInstance(AES_ALGORITHM_GCM);
+    GCMParameterSpec gcmSpec = new GCMParameterSpec(TAG_LENGTH * 8, iv);
+    cipher.init(Cipher.DECRYPT_MODE, aesKey, gcmSpec);
+
+    byte[] decryptedBytes = cipher.doFinal(encryptedText);
+
+    return Buffer.buffer(decryptedBytes);
+  }
 }

vertx-web-session-stores/vertx-web-sstore-cookie/src/main/java/io/vertx/ext/web/sstore/cookie/impl/CookieSessionStoreImpl.java

@@ -18,45 +18,36 @@
 import io.vertx.codegen.annotations.Nullable;
 import io.vertx.core.Future;
 import io.vertx.core.Vertx;
-import io.vertx.core.buffer.Buffer;
 import io.vertx.core.internal.ContextInternal;
 import io.vertx.core.json.JsonObject;
 import io.vertx.ext.auth.prng.VertxContextPRNG;
 import io.vertx.ext.web.Session;
 import io.vertx.ext.web.sstore.SessionStore;
 import io.vertx.ext.web.sstore.cookie.CookieSessionStore;
 
-import javax.crypto.Cipher;
-import javax.crypto.NoSuchPaddingException;
-import javax.crypto.SecretKey;
-import javax.crypto.SecretKeyFactory;
-import javax.crypto.spec.IvParameterSpec;
-import javax.crypto.spec.PBEKeySpec;
 import javax.crypto.spec.SecretKeySpec;
-import java.security.InvalidAlgorithmParameterException;
-import java.security.InvalidKeyException;
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
-import java.security.spec.InvalidKeySpecException;
-import java.security.spec.KeySpec;
 import java.util.Objects;
 
 /**
  * @author <a href="mailto:plopes@redhat.com">Paulo Lopes</a>
  */
 public class CookieSessionStoreImpl implements CookieSessionStore {
 
+  private static final String SHA_CRYPT = "SHA-256";
+  private static final String AES_ALGORITHM = "AES";
+ 
   public CookieSessionStoreImpl() {
     // required for the service loader
   }
 
-  public CookieSessionStoreImpl(Vertx vertx, String secret, Buffer salt) {
-    init(vertx, new JsonObject()
-      .put("secret", secret)
-      .put("salt", salt));
+  public CookieSessionStoreImpl(Vertx vertx, String secret) {
+    init(vertx, new JsonObject().put("secret", secret));
   }
 
-  private Cipher encrypt;
-  private Cipher decrypt;
+  private SecretKeySpec aesKey;
   private VertxContextPRNG random;
   private ContextInternal ctx;
 
@@ -67,38 +58,13 @@ public SessionStore init(Vertx vertx, JsonObject options) {
     this.ctx = (ContextInternal) vertx.getOrCreateContext();
 
     Objects.requireNonNull(options.getValue("secret"), "secret must be set");
-    Objects.requireNonNull(options.getValue("salt"), "salt must be set");
 
     try {
-      byte[] iv = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
-      if (options.containsKey("iv")) {
-        byte[] tmp = options.getBinary("iv");
-        for (int i = 0; i < tmp.length && i < iv.length; i++) {
-          iv[i] = tmp[i];
-        }
-      } else {
-        random.nextBytes(iv);
-      }
-      IvParameterSpec ivspec = new IvParameterSpec(iv);
-
-      SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256");
-      KeySpec spec = new PBEKeySpec(
-        options.getString("secret").toCharArray(),
-        options.getBinary("salt"),
-        65536,
-        256);
-
-      SecretKey tmp = factory.generateSecret(spec);
-      SecretKeySpec secretKey = new SecretKeySpec(tmp.getEncoded(), "AES");
-
-      encrypt = Cipher.getInstance("AES/CBC/PKCS5Padding");
-      encrypt.init(Cipher.ENCRYPT_MODE, secretKey, ivspec);
-
-      decrypt = Cipher.getInstance("AES/CBC/PKCS5Padding");
-      decrypt.init(Cipher.DECRYPT_MODE, secretKey, ivspec);
-
-    } catch (NoSuchAlgorithmException | InvalidKeyException | InvalidKeySpecException |
-             InvalidAlgorithmParameterException | NoSuchPaddingException e) {
+      // AES Key generation
+      MessageDigest sha256 = MessageDigest.getInstance(SHA_CRYPT);
+      byte[] keyBytes = sha256.digest(options.getString("secret").getBytes(StandardCharsets.UTF_8));
+      aesKey = new SecretKeySpec(keyBytes, AES_ALGORITHM);
+    } catch (NoSuchAlgorithmException e) {
       throw new RuntimeException(e);
     }
 
@@ -112,18 +78,18 @@ public long retryTimeout() {
 
   @Override
   public Session createSession(long timeout) {
-    return new CookieSession(encrypt, decrypt, random, timeout, DEFAULT_SESSIONID_LENGTH);
+    return new CookieSession(aesKey, random, timeout, DEFAULT_SESSIONID_LENGTH);
   }
 
   @Override
   public Session createSession(long timeout, int length) {
-    return new CookieSession(encrypt, decrypt, random, timeout, length);
+    return new CookieSession(aesKey, random, timeout, length);
   }
 
   @Override
   public Future<@Nullable Session> get(String cookieValue) {
     try {
-      Session session = new CookieSession(encrypt, decrypt, random).setValue(cookieValue);
+      Session session = new CookieSession(aesKey, random).setValue(cookieValue);
 
       if (session == null) {
         return ctx.succeededFuture();

vertx-web-session-stores/vertx-web-sstore-cookie/src/test/java/io/vertx/ext/web/sstore/cookie/tests/CookieSessionCreateTest.java

@@ -37,24 +37,9 @@ public class CookieSessionCreateTest {
   public RunTestOnContext rule = new RunTestOnContext();
 
   @Test
-  public void testCreateStoreAll() throws Exception {
-    SessionStore store = SessionStore.create(rule.vertx(), new JsonObject().put("secret", "KeyboardCat!").put("salt", "salt").put("iv", "iv"));
-    assertNotNull(store);
-    assertTrue(store instanceof CookieSessionStore);
-  }
-
-  @Test
-  public void testCreateStoreNoIV() throws Exception {
-    SessionStore store = SessionStore.create(rule.vertx(), new JsonObject().put("secret", "KeyboardCat!").put("salt", "salt"));
-    assertNotNull(store);
-    assertTrue(store instanceof CookieSessionStore);
-  }
-
-  @Test
-  public void testCreateStoreNoSalt() throws Exception {
+  public void testCreateStore() throws Exception {
     SessionStore store = SessionStore.create(rule.vertx(), new JsonObject().put("secret", "KeyboardCat!"));
     assertNotNull(store);
-    // salt is missing, default to LocalStore as we don't want to leak session data if config isn't complete
-    assertFalse(store instanceof CookieSessionStore);
+    assertTrue(store instanceof CookieSessionStore);
   }
 }