CVE-2014-1972

@@ -1,4 +1,4 @@
-// Copyright 2009 The Apache Software Foundation
+// Copyright 2009, 2012 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -14,32 +14,54 @@
 
 package org.apache.tapestry5.internal.services;
 
+import org.apache.tapestry5.SymbolConstants;
+import org.apache.tapestry5.internal.TapestryInternalUtils;
 import org.apache.tapestry5.internal.util.Base64InputStream;
+import org.apache.tapestry5.internal.util.MacOutputStream;
+import org.apache.tapestry5.ioc.annotations.Symbol;
 import org.apache.tapestry5.services.ClientDataEncoder;
 import org.apache.tapestry5.services.ClientDataSink;
 import org.apache.tapestry5.services.URLEncoder;
+import org.slf4j.Logger;
 
+import javax.crypto.spec.SecretKeySpec;
 import java.io.BufferedInputStream;
 import java.io.IOException;
 import java.io.ObjectInputStream;
+import java.io.UnsupportedEncodingException;
+import java.security.Key;
 import java.util.zip.GZIPInputStream;
 
 public class ClientDataEncoderImpl implements ClientDataEncoder
 {
     private final URLEncoder urlEncoder;
 
-    public ClientDataEncoderImpl(URLEncoder urlEncoder)
+    private final Key hmacKey;
+
+    public ClientDataEncoderImpl(URLEncoder urlEncoder, @Symbol(SymbolConstants.HMAC_PASSPHRASE) String passphrase, Logger logger) throws UnsupportedEncodingException
     {
         this.urlEncoder = urlEncoder;
+
+        if (passphrase.equals(""))
+        {
+            logger.error(String.format("The symbol '%s' has not been configured. " +
+                    "This is used to configure hash-based message authentication of Tapestry data stored in forms, or in the URL. " +
+                    "You application is less secure, and more vulnerable to denial-of-service attacks, when this symbol is left unconfigured.",
+                    SymbolConstants.HMAC_PASSPHRASE));
+
+            // Errors at lower levels if the passphrase is empty, so override the parameter to set a default value.
+            passphrase = "DEFAULT";
+        }
+
+        hmacKey = new SecretKeySpec(passphrase.getBytes("UTF8"), "HmacSHA1");
     }
 
     public ClientDataSink createSink()
     {
         try
         {
-            return new ClientDataSinkImpl(urlEncoder);
-        }
-        catch (IOException ex)
+            return new ClientDataSinkImpl(urlEncoder, hmacKey);
+        } catch (IOException ex)
         {
             throw new RuntimeException(ex);
         }
@@ -48,21 +70,57 @@ public ClientDataSink createSink()
     public ObjectInputStream decodeClientData(String clientData)
     {
         // The clientData is Base64 that's been gzip'ed (i.e., this matches
-        // what ClientDataSinkImpl does.
+        // what ClientDataSinkImpl does).
+
+        int colonx = clientData.indexOf(':');
+
+        if (colonx < 0)
+        {
+            throw new IllegalArgumentException("Client data must be prefixed with its HMAC code.");
+        }
+
+        // Extract the string presumably encoded by the server using the secret key.
+
+        String storedHmacResult = clientData.substring(0, colonx);
+
+        String clientStream = clientData.substring(colonx + 1);
 
         try
         {
-            BufferedInputStream buffered = new BufferedInputStream(
-                    new GZIPInputStream(new Base64InputStream(clientData)));
+            Base64InputStream b64in = new Base64InputStream(clientStream);
+
+            validateHMAC(storedHmacResult, b64in);
+
+            // After reading it once to validate, reset it for the actual read (which includes the GZip decompression).
+
+            b64in.reset();
+
+            BufferedInputStream buffered = new BufferedInputStream(new GZIPInputStream(b64in));
 
             return new ObjectInputStream(buffered);
-        }
-        catch (IOException ex)
+        } catch (IOException ex)
         {
             throw new RuntimeException(ex);
         }
     }
 
+    private void validateHMAC(String storedHmacResult, Base64InputStream b64in) throws IOException
+    {
+        MacOutputStream macOs = MacOutputStream.streamFor(hmacKey);
+
+        TapestryInternalUtils.copy(b64in, macOs);
+
+        macOs.close();
+
+        String actual = macOs.getResult();
+
+        if (!storedHmacResult.equals(actual))
+        {
+            throw new IOException("Client data associated with the current request appears to have been tampered with " +
+                    "(the HMAC signature does not match).");
+        }
+    }
+
     public ObjectInputStream decodeEncodedClientData(String clientData) throws IOException
     {
         return decodeClientData(urlEncoder.decode(clientData));

@@ -1,4 +1,4 @@
-// Copyright 2009 The Apache Software Foundation
+// Copyright 2009, 2012 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -15,13 +15,16 @@
 package org.apache.tapestry5.internal.services;
 
 import org.apache.tapestry5.internal.util.Base64OutputStream;
+import org.apache.tapestry5.internal.util.MacOutputStream;
+import org.apache.tapestry5.internal.util.TeeOutputStream;
 import org.apache.tapestry5.services.ClientDataSink;
 import org.apache.tapestry5.services.URLEncoder;
 
 import java.io.BufferedOutputStream;
 import java.io.IOException;
 import java.io.ObjectOutputStream;
 import java.io.OutputStream;
+import java.security.Key;
 import java.util.zip.GZIPOutputStream;
 
 public class ClientDataSinkImpl implements ClientDataSink
@@ -34,12 +37,17 @@ public class ClientDataSinkImpl implements ClientDataSink
 
     private boolean closed;
 
-    public ClientDataSinkImpl(URLEncoder urlEncoder) throws IOException
+    private final MacOutputStream macOutputStream;
+
+    public ClientDataSinkImpl(URLEncoder urlEncoder, Key hmacKey) throws IOException
     {
         this.urlEncoder = urlEncoder;
+
         base64OutputStream = new Base64OutputStream();
+        macOutputStream =  MacOutputStream.streamFor(hmacKey);
 
-        final BufferedOutputStream pipeline = new BufferedOutputStream(new GZIPOutputStream(base64OutputStream));
+        final BufferedOutputStream pipeline = new BufferedOutputStream(new GZIPOutputStream(
+                new TeeOutputStream(macOutputStream, base64OutputStream)));
 
         OutputStream guard = new OutputStream()
         {
@@ -92,14 +100,13 @@ public String getClientData()
             try
             {
                 objectOutputStream.close();
-            }
-            catch (IOException ex)
+            } catch (IOException ex)
             {
                 // Ignore.
             }
         }
 
-        return base64OutputStream.toBase64();
+        return macOutputStream.getResult() + ":" + base64OutputStream.toBase64();
     }
 
     public String getEncodedClientData()

@@ -0,0 +1,86 @@
+// Copyright 2012 The Apache Software Foundation
+//
+// 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 org.apache.tapestry5.internal.util;
+
+import org.apache.commons.codec.binary.Base64;
+import org.apache.tapestry5.ioc.internal.util.InternalUtils;
+
+import javax.crypto.Mac;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.security.Key;
+
+/**
+ * An output stream that wraps around a {@link Mac} (message authentication code algorithm).  This is currently
+ * used for symmetric (private) keys, but in theory could be used with assymetric (public/private) keys.
+ *
+ * @since 5.3.6
+ */
+public class MacOutputStream extends OutputStream
+{
+    private final Mac mac;
+
+    public static MacOutputStream streamFor(Key key) throws IOException
+    {
+        try
+        {
+            Mac mac = Mac.getInstance(key.getAlgorithm());
+            mac.init(key);
+
+            return new MacOutputStream(mac);
+        } catch (Exception ex)
+        {
+            throw new IOException("Unable to create MacOutputStream: " + InternalUtils.toMessage(ex), ex);
+        }
+    }
+
+    public MacOutputStream(Mac mac)
+    {
+        assert mac != null;
+
+        this.mac = mac;
+    }
+
+    /**
+     * Should only be invoked once, immediately after this stream is closed; it generates the final MAC result, and
+     * returns it as a Base64 encoded string.
+     *
+     * @return Base64 encoded MAC result
+     */
+    public String getResult()
+    {
+        byte[] result = mac.doFinal();
+
+        return new String(Base64.encodeBase64(result));
+    }
+
+    @Override
+    public void write(int b) throws IOException
+    {
+        mac.update((byte) b);
+    }
+
+    @Override
+    public void write(byte[] b) throws IOException
+    {
+        mac.update(b);
+    }
+
+    @Override
+    public void write(byte[] b, int off, int len) throws IOException
+    {
+        mac.update(b, off, len);
+    }
+}

@@ -0,0 +1,73 @@
+// Copyright 2012 The Apache Software Foundation
+//
+// 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 org.apache.tapestry5.internal.util;
+
+import java.io.IOException;
+import java.io.OutputStream;
+
+/**
+ * An output stream that acts like a "tee", copying all provided bytes to two output streams. This is used, for example,
+ * to accumulate a hash of content even as it is being written.
+ *
+ * @since 5.3.5
+ */
+public class TeeOutputStream extends OutputStream
+{
+    private final OutputStream left, right;
+
+    public TeeOutputStream(OutputStream left, OutputStream right)
+    {
+        assert left != null;
+        assert right != null;
+
+        this.left = left;
+        this.right = right;
+    }
+
+    @Override
+    public void write(int b) throws IOException
+    {
+        left.write(b);
+        right.write(b);
+    }
+
+    @Override
+    public void write(byte[] b) throws IOException
+    {
+        left.write(b);
+        right.write(b);
+    }
+
+    @Override
+    public void write(byte[] b, int off, int len) throws IOException
+    {
+        left.write(b, off, len);
+        right.write(b, off, len);
+    }
+
+    @Override
+    public void flush() throws IOException
+    {
+        left.flush();
+        right.flush();
+    }
+
+    @Override
+    public void close() throws IOException
+    {
+        left.close();
+        right.close();
+    }
+}

@@ -1,4 +1,4 @@
-// Copyright 2009 The Apache Software Foundation
+// Copyright 2009, 2012 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -21,6 +21,10 @@
  * A service used when a component or service needs to encode some amount of data on the client as a string. The string
  * may be a query parameter, hidden form field, or a portion of a URL.  The default implementation converts the object
  * output stream into a Base64 string.
+ * <p/>
+ * Starting in release 5.3.6, the encoded data incorporates an HMAC (hash based message authentication code) signature,
+ * as a prefix. HMAC requires a secret key, configured using the
+ * {@link org.apache.tapestry5.SymbolConstants#HMAC_PASSPHRASE} symbol.
  *
  * @since 5.1.0.1
  */
@@ -37,17 +41,22 @@ public interface ClientDataEncoder
     /**
      * Decodes data previously obtained from {@link ClientDataSink#getClientData()}.
      *
-     * @param clientData encoded client data
+     * @param clientData
+     *         encoded client data
      * @return stream of decoded data
+     * @throws IOException
+     *         if the client data has been corrupted (verified via the HMAC)
      */
     ObjectInputStream decodeClientData(String clientData) throws IOException;
 
     /**
-     * Decoes client data obtained via {@link ClientDataSink#getEncodedClientData()}.
+     * Decodes client data obtained via {@link ClientDataSink#getEncodedClientData()}.
      *
-     * @param clientData URLEncoded client data
+     * @param clientData
+     *         URLEncoded client data
      * @return stream of objects
      * @throws IOException
+     *         if the client data has been corrupted (verified via the HMAC)
      * @since 5.1.0.4
      */
     ObjectInputStream decodeEncodedClientData(String clientData) throws IOException;

@@ -2394,6 +2394,10 @@ public static void contributeFactoryDefaults(MappedConfiguration<String, Object>
 
         // By default, no page is on the whitelist unless it has the @WhitelistAccessOnly annotation
         configuration.add(MetaDataConstants.WHITELIST_ONLY_PAGE, false);
+
+        // Leaving this as the default results in a runtime error logged to the console (and a default password is used);
+        // you are expected to override this symbol.
+        configuration.add(SymbolConstants.HMAC_PASSPHRASE, "");
     }
 
     /**

@@ -365,4 +365,15 @@ public class SymbolConstants
      * Prefix to be used for all asset paths
      */
     public static final String ASSET_PATH_PREFIX = "tapestry.asset-path-prefix";
+
+    /**
+     * A passphrase used as the basis of hash-based message authentication (HMAC) for any object stream data stored on
+     * the client.  The default phrase is the empty string, which will result in a logged runtime <em>error</em>.
+     * You should configure this to a reasonable value (longer is better) and ensure that all servers in your cluster
+     * share the same value (configuring this in code, rather than the command line, is preferred).
+     *
+     * @see org.apache.tapestry5.services.ClientDataEncoder
+     * @since 5.3.6
+     */
+    public static final String HMAC_PASSPHRASE = "tapestry.hmac-passphrase";
 }

@@ -0,0 +1,108 @@
+package org.apache.tapestry5.internal.services
+
+import org.apache.tapestry5.ioc.test.TestBase
+import org.apache.tapestry5.services.ClientDataEncoder
+import org.easymock.EasyMock
+import org.slf4j.Logger
+import org.testng.annotations.Test
+
+class ClientDataEncoderImplTest extends TestBase {
+
+    def tryEncodeAndDecode(ClientDataEncoder cde) {
+        def now = new Date()
+        def input = "The current time is $now"
+
+
+        String clientData = convertToClientData cde, input
+
+        def stream = cde.decodeClientData clientData
+
+        def output = stream.readObject()
+
+        assert !input.is(output)
+        assert input == output
+    }
+
+    def String convertToClientData(ClientDataEncoder cde, input) {
+        def sink = cde.createSink()
+
+        sink.getObjectOutputStream().with { stream ->
+            stream << input
+            stream.close()
+        }
+
+        sink.clientData
+    }
+
+    def extractData(String encoded) {
+        def colonx = encoded.indexOf(':')
+
+        encoded.substring(colonx + 1)
+    }
+
+    @Test
+    void blank_passphrase_works_but_logs_error() {
+        Logger logger = newMock Logger
+
+        logger.error(EasyMock.isA(String))
+
+        replay()
+
+        ClientDataEncoder cde = new ClientDataEncoderImpl(null, "", logger)
+
+        tryEncodeAndDecode cde
+
+        verify()
+    }
+
+    @Test
+    void no_logged_error_with_non_blank_passphrase() {
+        ClientDataEncoder cde = new ClientDataEncoderImpl(null, "Testing, Testing, 1.., 2.., 3...", null)
+
+        tryEncodeAndDecode cde
+    }
+
+    @Test
+    void passphrase_affects_encoded_output() {
+        ClientDataEncoder first = new ClientDataEncoderImpl(null, "first passphrase", null)
+        ClientDataEncoder second = new ClientDataEncoderImpl(null, " different passphrase ", null)
+
+        def input = "current time millis is ${System.currentTimeMillis()} ms"
+
+        def output1 = convertToClientData first, input
+        def output2 = convertToClientData second, input
+
+        assert output1 != output2
+
+        assert extractData(output1) == extractData(output2)
+    }
+
+    @Test(expectedExceptions = IllegalArgumentException)
+    void decode_with_missing_hmac_prefix_is_a_failure() {
+        ClientDataEncoder cde = new ClientDataEncoderImpl(null, "a passphrase", null)
+
+        cde.decodeClientData("so completely invalid")
+    }
+
+    @Test
+    void incorrect_hmac_is_detected() {
+
+        // Simulate tampering by encoding with one passphrase and attempting to decode with a different
+        // passphrase.
+        ClientDataEncoder first = new ClientDataEncoderImpl(null, "first passphrase", null)
+        ClientDataEncoder second = new ClientDataEncoderImpl(null, " different passphrase ", null)
+
+        def input = "current time millis is ${System.currentTimeMillis()} ms"
+
+        def output = convertToClientData first, input
+
+        try {
+            second.decodeClientData(output)
+            unreachable()
+        }
+        catch (Exception e) {
+            assert e.message.contains("HMAC signature does not match")
+        }
+    }
+
+}

@@ -157,6 +157,8 @@ public static void contributeApplicationDefaults(MappedConfiguration<String, Str
         configuration.add(SymbolConstants.SECURE_ENABLED, "true");
 
         configuration.add("app.injected-symbol", "Symbol contributed to ApplicationDefaults");
+
+        configuration.add(SymbolConstants.HMAC_PASSPHRASE, "testing, testing, 1... 2... 3...");
     }
 
     public static void contributeIgnoredPathsFilter(Configuration<String> configuration)