merged branch sstok/fix_digest_authentication (PR #5874)

This PR was merged into the 2.0 branch.

Commits
-------

f2cbea3 [Security] remove escape charters from username provided by Digest DigestAuthenticationListener
80f6992 [Security] added test extra for digest authentication
d66b03c fixed CS
694697d [Security] Fixed digest authentication
c067586 [Security] Fixed digest authentication

Discussion
----------

Fix digest authentication

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets:
Todo: -
License of the code: MIT
Documentation PR: -
Replaces: #5485

This adds the missing fixes.

My only concerns is the ```\"``` removing.
```\"``` is only needed for the HTTP transport, but keeping them would require to also store the username with the escapes as well.

---------------------------------------------------------------------------

by fabpot at 2012-10-30T11:25:28Z

The digest authentication mechanism is not that widespread due to its limitation. And the transport is not HTTP, I think we are talking about very few cases.

---------------------------------------------------------------------------

by sstok at 2012-10-30T12:49:14Z

Apache seems to remove (ignore) escape characters.

```c
if (auth_line[0] == '=') {
            auth_line++;
            while (apr_isspace(auth_line[0])) {
                auth_line++;
            }

            vv = 0;
            if (auth_line[0] == '\"') {         /* quoted string */
                auth_line++;
                while (auth_line[0] != '\"' && auth_line[0] != '\0') {
                    if (auth_line[0] == '\\' && auth_line[1] != '\0') {
                        auth_line++;            /* escaped char */
                    }
                    value[vv++] = *auth_line++;
                }
                if (auth_line[0] != '\0') {
                    auth_line++;
                }
            }
            else {                               /* token */
                while (auth_line[0] != ',' && auth_line[0] != '\0'
                       && !apr_isspace(auth_line[0])) {
                    value[vv++] = *auth_line++;
                }
            }
            value[vv] = '\0';
        }
```

But would this change be a BC break for people already using quotes but without a comma and thus they never hit this bug?

The change it self is minimum, just calling ```str_replace('\\\\', '\\', str_replace('\\"', '"', $value))``` when getting the username.

---------------------------------------------------------------------------

by fabpot at 2012-11-13T13:00:12Z

@sstok Doing the same as Apache seems the best option here (just document the BC break).

---------------------------------------------------------------------------

by sstok at 2012-11-15T16:05:00Z

Hopefully I did this correct, but the needed escapes seem correctly removed.
`\"` is changed to `"` `\\` is changed to `\`
`\'` it kept as it is, as this needs no correcting.

@Vincent-Simonin Can you verify please.

---------------------------------------------------------------------------

by Vincent-Simonin at 2012-11-19T09:28:18Z

Authentication didn't work with this configuration :

```
providers:
    in_memory:
        name: in_memory
        users:
            te"st: { password: test, roles: [ 'ROLE_USER' ] }
```

`te"st` was set in authentication form's user field.

(Must we also escape `"` in configuration file ?)

Tests were performed with nginx.

---------------------------------------------------------------------------

by sstok at 2012-11-19T09:33:34Z

Yes. YAML escapes using an duplicate quote, like SQL.

```yaml
providers:
    in_memory:
        name: in_memory
        users:
            "te""st": { password: test, roles: [ 'ROLE_USER' ] }
```

src/Symfony/Component/Security/Http/Firewall/DigestAuthenticationListener.php

@@ -141,11 +141,12 @@ class DigestData
     public function __construct($header)
     {
         $this->header = $header;
-        $parts = preg_split('/, /', $header);
+        preg_match_all('/(\w+)=("((?:[^"\\\\]|\\\\.)+)"|([^\s,$]+))/', $header, $matches, PREG_SET_ORDER);
         $this->elements = array();
-        foreach ($parts as $part) {
-            list($key, $value) = explode('=', $part);
-            $this->elements[$key] = '"' === $value[0] ? substr($value, 1, -1) : $value;
+        foreach ($matches as $match) {
+            if (isset($match[1]) && isset($match[3])) {
+                $this->elements[$match[1]] = isset($match[4]) ? $match[4] : $match[3];
+            }
         }
     }
 
@@ -156,7 +157,7 @@ class DigestData
 
     public function getUsername()
     {
-        return $this->elements['username'];
+        return strtr($this->elements['username'], array("\\\"" => "\"", "\\\\" => "\\"));
     }
 
     public function validateAndDecode($entryPointKey, $expectedRealm)
@@ -188,7 +189,7 @@ class DigestData
         $this->nonceExpiryTime = $nonceTokens[0];
 
         if (md5($this->nonceExpiryTime.':'.$entryPointKey) !== $nonceTokens[1]) {
-            new BadCredentialsException(sprintf('Nonce token compromised "%s".', $nonceAsPlainText));
+            throw new BadCredentialsException(sprintf('Nonce token compromised "%s".', $nonceAsPlainText));
         }
     }
 

tests/Symfony/Tests/Component/Security/Http/Firewall/DigestDataTest.php

@@ -0,0 +1,181 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Security\Tests\Http\Firewall;
+
+use Symfony\Component\Security\Http\Firewall\DigestData;
+
+class DigestDataTest extends \PHPUnit_Framework_TestCase
+{
+    public function testGetResponse()
+    {
+        $digestAuth = new DigestData(
+            'username="user", realm="Welcome, robot!", ' .
+            'nonce="MTM0NzMyMTgyMy42NzkzOmRlZjM4NmIzOGNjMjE0OWJiNDU0MDAxNzJmYmM1MmZl", ' .
+            'uri="/path/info?p1=5&p2=5", cnonce="MDIwODkz", nc=00000001, qop="auth", ' .
+            'response="b52938fc9e6d7c01be7702ece9031b42"'
+        );
+
+        $this->assertEquals('b52938fc9e6d7c01be7702ece9031b42', $digestAuth->getResponse());
+    }
+
+    public function testGetUsername()
+    {
+        $digestAuth = new DigestData(
+            'username="user", realm="Welcome, robot!", ' .
+            'nonce="MTM0NzMyMTgyMy42NzkzOmRlZjM4NmIzOGNjMjE0OWJiNDU0MDAxNzJmYmM1MmZl", ' .
+            'uri="/path/info?p1=5&p2=5", cnonce="MDIwODkz", nc=00000001, qop="auth", ' .
+            'response="b52938fc9e6d7c01be7702ece9031b42"'
+        );
+
+        $this->assertEquals('user', $digestAuth->getUsername());
+    }
+
+    public function testGetUsernameWithQuote()
+    {
+        $digestAuth = new DigestData(
+            'username="\"user\"", realm="Welcome, robot!", ' .
+            'nonce="MTM0NzMyMTgyMy42NzkzOmRlZjM4NmIzOGNjMjE0OWJiNDU0MDAxNzJmYmM1MmZl", ' .
+            'uri="/path/info?p1=5&p2=5", cnonce="MDIwODkz", nc=00000001, qop="auth", ' .
+            'response="b52938fc9e6d7c01be7702ece9031b42"'
+        );
+
+        $this->assertEquals('"user"', $digestAuth->getUsername());
+    }
+
+    public function testGetUsernameWithQuoteAndEscape()
+    {
+        $digestAuth = new DigestData(
+            'username="\"u\\\\\"ser\"", realm="Welcome, robot!", ' .
+            'nonce="MTM0NzMyMTgyMy42NzkzOmRlZjM4NmIzOGNjMjE0OWJiNDU0MDAxNzJmYmM1MmZl", ' .
+            'uri="/path/info?p1=5&p2=5", cnonce="MDIwODkz", nc=00000001, qop="auth", ' .
+            'response="b52938fc9e6d7c01be7702ece9031b42"'
+        );
+
+        $this->assertEquals('"u\\"ser"', $digestAuth->getUsername());
+    }
+
+    public function testGetUsernameWithSingleQuote()
+    {
+        $digestAuth = new DigestData(
+            'username="\"u\'ser\"", realm="Welcome, robot!", ' .
+            'nonce="MTM0NzMyMTgyMy42NzkzOmRlZjM4NmIzOGNjMjE0OWJiNDU0MDAxNzJmYmM1MmZl", ' .
+            'uri="/path/info?p1=5&p2=5", cnonce="MDIwODkz", nc=00000001, qop="auth", ' .
+            'response="b52938fc9e6d7c01be7702ece9031b42"'
+        );
+
+        $this->assertEquals('"u\'ser"', $digestAuth->getUsername());
+    }
+
+    public function testGetUsernameWithSingleQuoteAndEscape()
+    {
+        $digestAuth = new DigestData(
+            'username="\"u\\\'ser\"", realm="Welcome, robot!", ' .
+            'nonce="MTM0NzMyMTgyMy42NzkzOmRlZjM4NmIzOGNjMjE0OWJiNDU0MDAxNzJmYmM1MmZl", ' .
+            'uri="/path/info?p1=5&p2=5", cnonce="MDIwODkz", nc=00000001, qop="auth", ' .
+            'response="b52938fc9e6d7c01be7702ece9031b42"'
+        );
+
+        $this->assertEquals('"u\\\'ser"', $digestAuth->getUsername());
+    }
+
+    public function testGetUsernameWithEscape()
+    {
+        $digestAuth = new DigestData(
+            'username="\"u\\ser\"", realm="Welcome, robot!", ' .
+            'nonce="MTM0NzMyMTgyMy42NzkzOmRlZjM4NmIzOGNjMjE0OWJiNDU0MDAxNzJmYmM1MmZl", ' .
+            'uri="/path/info?p1=5&p2=5", cnonce="MDIwODkz", nc=00000001, qop="auth", ' .
+            'response="b52938fc9e6d7c01be7702ece9031b42"'
+        );
+
+        $this->assertEquals('"u\\ser"', $digestAuth->getUsername());
+    }
+
+    public function testValidateAndDecode()
+    {
+        $time = microtime(true);
+        $key = 'ThisIsAKey';
+        $nonce = base64_encode($time . ':' . md5($time . ':' . $key));
+
+        $digestAuth = new DigestData(
+            'username="user", realm="Welcome, robot!", nonce="' . $nonce . '", ' .
+            'uri="/path/info?p1=5&p2=5", cnonce="MDIwODkz", nc=00000001, qop="auth", ' .
+            'response="b52938fc9e6d7c01be7702ece9031b42"'
+        );
+
+        try {
+            $digestAuth->validateAndDecode($key, 'Welcome, robot!');
+        } catch (\Exception $e) {
+            $this->fail(sprintf('testValidateAndDecode fail with message: %s', $e->getMessage()));
+        }
+    }
+
+    public function testCalculateServerDigest()
+    {
+        $this->calculateServerDigest('user', 'Welcome, robot!', 'pass,word=password', 'ThisIsAKey', '00000001', 'MDIwODkz', 'auth', 'GET', '/path/info?p1=5&p2=5');
+    }
+
+    public function testCalculateServerDigestWithQuote()
+    {
+        $this->calculateServerDigest('\"user\"', 'Welcome, \"robot\"!', 'pass,word=password', 'ThisIsAKey', '00000001', 'MDIwODkz', 'auth', 'GET', '/path/info?p1=5&p2=5');
+    }
+
+    public function testCalculateServerDigestWithQuoteAndEscape()
+    {
+        $this->calculateServerDigest('\"u\\\\\"ser\"', 'Welcome, \"robot\"!', 'pass,word=password', 'ThisIsAKey', '00000001', 'MDIwODkz', 'auth', 'GET', '/path/info?p1=5&p2=5');
+    }
+
+    public function testCalculateServerDigestEscape()
+    {
+        $this->calculateServerDigest('\"u\\ser\"', 'Welcome, \"robot\"!', 'pass,word=password', 'ThisIsAKey', '00000001', 'MDIwODkz', 'auth', 'GET', '/path/info?p1=5&p2=5');
+        $this->calculateServerDigest('\"u\\ser\\\\\"', 'Welcome, \"robot\"!', 'pass,word=password', 'ThisIsAKey', '00000001', 'MDIwODkz', 'auth', 'GET', '/path/info?p1=5&p2=5');
+    }
+
+    public function testIsNonceExpired()
+    {
+        $time = microtime(true) + 10;
+        $key = 'ThisIsAKey';
+        $nonce = base64_encode($time . ':' . md5($time . ':' . $key));
+
+        $digestAuth = new DigestData(
+            'username="user", realm="Welcome, robot!", nonce="' . $nonce . '", ' .
+            'uri="/path/info?p1=5&p2=5", cnonce="MDIwODkz", nc=00000001, qop="auth", ' .
+            'response="b52938fc9e6d7c01be7702ece9031b42"'
+        );
+
+        $digestAuth->validateAndDecode($key, 'Welcome, robot!');
+
+        $this->assertFalse($digestAuth->isNonceExpired());
+    }
+
+    protected function setUp()
+    {
+        class_exists('Symfony\Component\Security\Http\Firewall\DigestAuthenticationListener', true);
+    }
+
+    private function calculateServerDigest($username, $realm, $password, $key, $nc, $cnonce, $qop, $method, $uri)
+    {
+        $time = microtime(true);
+        $nonce = base64_encode($time . ':' . md5($time . ':' . $key));
+
+        $response = md5(
+            md5($username . ':' . $realm . ':' . $password) . ':' . $nonce . ':' . $nc . ':' . $cnonce . ':' . $qop . ':' . md5($method . ':' . $uri)
+        );
+
+        $digest = sprintf('username="%s", realm="%s", nonce="%s", uri="%s", cnonce="%s", nc=%s, qop="%s", response="%s"',
+            $username, $realm, $nonce, $uri, $cnonce, $nc, $qop, $response
+        );
+
+        $digestAuth = new DigestData($digest);
+
+        $this->assertEquals($digestAuth->getResponse(), $digestAuth->calculateServerDigest($password, $method));
+    }
+}