feat: Enforce signing and verifying of cookie value

Because a cookie is sent by an untrusted client we need to sign and
verify it's content to be sure not to accept any garbage or tampered
stuff. This reduces the risk of being vulnerable to deserializiation
attacks because we only accept stuff we ourself signed before.

Author: mvitz

src/main/java/com/innoq/spring/cookie/flash/CookieFlashMapManager.java

@@ -15,6 +15,7 @@
  */
 package com.innoq.spring.cookie.flash;
 
+import com.innoq.spring.cookie.security.CookieValueSigner;
 import org.springframework.util.Assert;
 import org.springframework.web.servlet.FlashMap;
 import org.springframework.web.servlet.support.AbstractFlashMapManager;
@@ -24,23 +25,30 @@
 import javax.servlet.http.HttpServletResponse;
 import java.util.List;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.security.MessageDigest.isEqual;
 import static org.springframework.web.util.WebUtils.getCookie;
 
 public final class CookieFlashMapManager extends AbstractFlashMapManager {
 
     private static final String DEFAULT_COOKIE_NAME = "flash";
 
     private final FlashMapListCodec codec;
+    private final CookieValueSigner signer;
     private final String cookieName;
 
-    public CookieFlashMapManager(FlashMapListCodec codec) {
-        this(codec, DEFAULT_COOKIE_NAME);
+    public CookieFlashMapManager(FlashMapListCodec codec,
+            CookieValueSigner signer) {
+        this(codec, signer, DEFAULT_COOKIE_NAME);
     }
 
-    public CookieFlashMapManager(FlashMapListCodec codec, String cookieName) {
+    public CookieFlashMapManager(FlashMapListCodec codec,
+            CookieValueSigner signer, String cookieName) {
         Assert.notNull(codec, "FlashMapListCodec must not be null");
+        Assert.notNull(signer, "CookieValueSigner must not be null");
         Assert.hasText(cookieName, "Cookie name must not be null or empty");
         this.codec = codec;
+        this.signer = signer;
         this.cookieName = cookieName;
     }
 
@@ -52,7 +60,7 @@ protected List<FlashMap> retrieveFlashMaps(HttpServletRequest request) {
         }
 
         final String value = cookie.getValue();
-        return codec.decode(value);
+        return decode(value);
     }
 
     @Override
@@ -65,7 +73,7 @@ protected void updateFlashMaps(List<FlashMap> flashMaps,
         if (flashMaps.isEmpty()) {
             cookie.setMaxAge(0);
         } else {
-            final String value = codec.encode(flashMaps);
+            final String value = encode(flashMaps);
             cookie.setValue(value);
         }
         response.addCookie(cookie);
@@ -75,4 +83,38 @@ protected void updateFlashMaps(List<FlashMap> flashMaps,
     protected Object getFlashMapsMutex(HttpServletRequest request) {
         return null;
     }
+
+    private List<FlashMap> decode(String value) {
+        final String[] signatureAndPayload = reverse(value).split("--", 2);
+        if (signatureAndPayload.length != 2) {
+            // TODO logging
+            return null;
+        }
+
+        final String signature = reverse(signatureAndPayload[0]);
+        final String payload = reverse(signatureAndPayload[1]);
+
+        if (!isVerified(payload, signature)) {
+            // TODO logging
+            return null;
+        }
+
+        return codec.decode(payload);
+    }
+
+
+    private String encode(List<FlashMap> flashMaps) {
+        final String payload = codec.encode(flashMaps);
+        final String signature = signer.sign(payload);
+        return payload + "--" + signature;
+    }
+
+    private boolean isVerified(String payload, String digest) {
+        final String signature = signer.sign(payload);
+        return isEqual(digest.getBytes(UTF_8), signature.getBytes(UTF_8));
+    }
+
+    private static String reverse(String s) {
+        return new StringBuilder(s).reverse().toString();
+    }
 }

src/main/java/com/innoq/spring/cookie/security/CookieValueSigner.java

@@ -0,0 +1,27 @@
+/**
+ * Copyright 2018 innoQ Deutschland GmbH
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.innoq.spring.cookie.security;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+public interface CookieValueSigner {
+
+    String sign(String payload);
+
+    static CookieValueSigner hmacSha1(String secret) {
+        return new HmacCookieValueSigner("HmacSHA1", secret.getBytes(UTF_8));
+    }
+}

src/main/java/com/innoq/spring/cookie/security/HmacCookieValueSigner.java

@@ -0,0 +1,64 @@
+/**
+ * Copyright 2018 innoQ Deutschland GmbH
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.innoq.spring.cookie.security;
+
+import javax.crypto.Mac;
+import javax.crypto.spec.SecretKeySpec;
+import java.security.InvalidKeyException;
+import java.security.Key;
+import java.security.NoSuchAlgorithmException;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+final class HmacCookieValueSigner implements CookieValueSigner {
+
+    private final String algorithm;
+    private final byte[] secret;
+
+    HmacCookieValueSigner(String algorithm, byte[] secret) {
+        this.algorithm = algorithm;
+        this.secret = secret;
+    }
+
+    @Override
+    public String sign(String payload) {
+        try {
+            final byte[] data  = payload.getBytes(UTF_8);
+            final Key key = new SecretKeySpec(secret, algorithm);
+
+            final Mac mac = Mac.getInstance(algorithm);
+            mac.init(key);
+            final byte[] result = mac.doFinal(data);
+
+            return bytesToHex(result);
+        } catch (NoSuchAlgorithmException | InvalidKeyException e) {
+            // TODO: use own exception?
+            throw new IllegalStateException("Unable to sign payload", e);
+        }
+    }
+
+    private static String bytesToHex(byte[] bytes) {
+        final StringBuilder hex = new StringBuilder();
+        for (byte b : bytes) {
+            hex.append(byteToHex(b));
+        }
+        return hex.toString();
+    }
+
+    private static String byteToHex(byte b) {
+        return Integer.toString((b &  0xff) + 0x100, 16).substring(1);
+    }
+}

src/test/java/com/innoq/spring/cookie/flash/CookieFlashMapManagerTest.java

@@ -16,6 +16,7 @@
 package com.innoq.spring.cookie.flash;
 
 import com.innoq.spring.cookie.flash.codec.jackson.JacksonFlashMapListCodec;
+import com.innoq.spring.cookie.security.CookieValueSigner;
 import org.junit.jupiter.api.Test;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
@@ -33,7 +34,7 @@
 class CookieFlashMapManagerTest {
 
     CookieFlashMapManager sut = new CookieFlashMapManager(
-        JacksonFlashMapListCodec.create());
+        JacksonFlashMapListCodec.create(), CookieValueSigner.hmacSha1("abc"));
 
     @Test
     void retrieveFlashMaps_withNoCookiePresent_returnsNull() {
@@ -46,7 +47,7 @@ void retrieveFlashMaps_withNoCookiePresent_returnsNull() {
 
     @Test
     void retrieveFlashMaps_withValidCookie_returnsFlashMaps() {
-        String cookieValue = "W3siYXR0cmlidXRlcyI6eyJmb28iOm51bGwsImJhciI6NDcxMSwiYmF6IjoibG9yZW0gaXBzdW0ifSwiZXhwaXJhdGlvblRpbWUiOjQ3MTEsInRhcmdldFJlcXVlc3RQYXJhbXMiOnsiZm9vIjpbXSwiYmFyIjpbImZvbyJdLCJiYXoiOlsibG9yZW0iLCJpcHN1bSJdfSwidGFyZ2V0UmVxdWVzdFBhdGgiOiIvZm9vIn1dCg==";
+        String cookieValue = "W3siYXR0cmlidXRlcyI6eyJmb28iOm51bGwsImJhciI6NDcxMSwiYmF6IjoibG9yZW0gaXBzdW0ifSwiZXhwaXJhdGlvblRpbWUiOjQ3MTEsInRhcmdldFJlcXVlc3RQYXJhbXMiOnsiZm9vIjpbXSwiYmFyIjpbImZvbyJdLCJiYXoiOlsibG9yZW0iLCJpcHN1bSJdfSwidGFyZ2V0UmVxdWVzdFBhdGgiOiIvZm9vIn1dCg==--aa17ee8faf0bbe77a0949de6d5c593bd1e39718c";
 
         MockHttpServletRequest request = new MockHttpServletRequest("GET", "/");
         request.setCookies(new Cookie("flash", cookieValue));
@@ -82,7 +83,7 @@ void updateFlashMaps_withSingleFlashMap_writesCookie() {
             .hasFieldOrPropertyWithValue("httpOnly", true);
 
         String cookieValue = response.getCookie("flash").getValue();
-        assertThat(cookieValue).isEqualTo("W3siYXR0cmlidXRlcyI6e30sImV4cGlyYXRpb25UaW1lIjotMSwidGFyZ2V0UmVxdWVzdFBhcmFtcyI6e30sInRhcmdldFJlcXVlc3RQYXRoIjpudWxsfV0=");
+        assertThat(cookieValue).isEqualTo("W3siYXR0cmlidXRlcyI6e30sImV4cGlyYXRpb25UaW1lIjotMSwidGFyZ2V0UmVxdWVzdFBhcmFtcyI6e30sInRhcmdldFJlcXVlc3RQYXRoIjpudWxsfV0=--daa79b20816b076ceb9f628bef9a82792fe9b5fa");
     }
 
     @Test