Hello,

Le 30/01/2017 à 16:04, FUSTE Emmanuel a écrit :
> Le 29/01/2017 à 14:33, Amos Jeffries a écrit :
>> On 21/01/2017 2:05 a.m., FUSTE Emmanuel wrote:
>>> Hello,
>>>
>>> We have to support many historic digest auth implementation for which
>>> the realm is not included in the digest password attribute:
>>> The password is effectively stored as "HA1" instead of "REALM:HA1".
>>> I would like to kill our own homegrown helpers and use the Squid
>>> provided one.
>>>
>>>     Is something like the attached patch is acceptable/could be included
>>> in a future Squid release ?
>>>
>>> Best regards,
>>> Emmanuel.
>>> --
>> Sorry it has taken me so long to respond on this. I wanted to verify the
>> importance of realm in the H(A1) digest. Why it was marked as REQUIRED
>> to begin with.
> No, thank you to have taken the time.
>> Sadly I have to report my suspicion was correct and I vote -1 on this in
>> its current form. BUT I can think of two ways to get around it, see end
>> of this mail.
>>
>>
>> The problems are that;
>>    1) the realm is both the salting value used within the hash and the
>> scope limitation on what inputs from HTTP are used to compare against
>> the A1, and
>>    2) Squid does not itself verify the realm received was the one offered
>> and leaves the comparison to the backend system. There is some
>> possibility the authentication system is using multiple security realms
>> and Squids realm string is just an offer.
>>
>>
>> Not having realm tied to the credentials in the backend storage leaves
>> this particular helper with no other option but to trust the realm sent
>> (probably) over clear-text by any client/attacker actually matches the
>> salting. That allows remote senders to manipulate the realm string they
>> send to perform a collision attack against the stored password.
>>    They no longer have to find and prove knowledge of the password. But
>> just find a collision for its hash vs arbitrary realm strings.
> Ok point taken.
>
>> Old Digest systems are not the safest things to begin with. They also
>> tend to use MD5 hashing which was the only one available for many years
>> and relatively easy to find collisions for.
>>
>>
>>
>> Workaround #1:
>>
>> Is there just one (or a few) realm that all your stored passwords use?
> Yes only one, as we lost the ability to iterate over multi-valued entry
> using the realm as a key.
>> I think we can accept the absence of realm per-password if there were a
>> global realm to check against instead. A command line option could be
>> added to pass an explicit realm (or multiple?) which the helper can use
>> to reject the possible attacks.
> Ok, so a rework of the patch with a mandatory command line option to
> specify the realm in case of null delimiter will be acceptable ?
> I think that "an explicit realm" is sufficient. To have multiple realm,
> something like  realm:HA1 must already be in place or is really
> completely broken by design.
> Mono "hard-coded in conf" realm is not flexible but not broken -> I know
> a lots of such setup. That is what I would like to address.
> I don't know any real setup which need the "multiple possible implicit
> realm in conf" way
>
New patch attached.
- Empty realm parameter is treated as no realm parameter supplied.
- realm parameter is mandatory if empty delimiter is supplied
- -f vs -F typo corrected in the help text
Tested in real conditions.

Emmanuel.
----

--- squid3-3.5.23/helpers/digest_auth/LDAP/ldap_backend.cc.manu 2017-01-18 
15:41:31.021707038 +0100
+++ squid3-3.5.23/helpers/digest_auth/LDAP/ldap_backend.cc      2017-02-07 
10:29:07.026005928 +0100
@@ -63,6 +63,7 @@
 static const char *binddn = NULL;
 static const char *bindpasswd = NULL;
 static const char *delimiter = ":";
+static const char *frealm = "";
 static int encrpass = 0;
 static int searchscope = LDAP_SCOPE_SUBTREE;
 static int persistent = 0;
@@ -267,7 +268,7 @@
             }
             value = values;
             while (*value) {
-                if (encrpass) {
+                if (encrpass && *delimiter ) {
                     const char *t = strtok(*value, delimiter);
                     if (t && strcmp(t, realm) == 0) {
                         password = strtok(NULL, delimiter);
@@ -451,6 +452,9 @@
         case 'l':
             delimiter = value;
             break;
+        case 'r':
+            frealm = value;
+            break;
         case 'b':
             userbasedn = value;
             break;
@@ -574,10 +578,11 @@
     if (!ldapServer)
         ldapServer = (char *) "localhost";
 
-    if (!userbasedn || !passattr) {
-        fprintf(stderr, "Usage: " PROGRAM_NAME " -b basedn -f filter [options] 
ldap_server_name\n\n");
+    if (!userbasedn || !passattr || (!*delimiter && !*frealm)) {
+        fprintf(stderr, "Usage: " PROGRAM_NAME " -b basedn -F filter [options] 
ldap_server_name\n\n");
         fprintf(stderr, "\t-A password attribute(REQUIRED)\t\tUser attribute 
that contains the password\n");
-        fprintf(stderr, "\t-l password realm delimiter(REQUIRED)\tCharater(s) 
that devides the password attribute\n\t\t\t\t\t\tin realm and password tokens, 
default ':' realm:password\n");
+        fprintf(stderr, "\t-l password realm delimiter(REQUIRED)\tCharater(s) 
that devides the password attribute\n\t\t\t\t\t\tin realm and password tokens, 
default ':' realm:password, could be\n\t\t\t\t\t\tempty string if the password 
is alone in the password attribute\n");
+        fprintf(stderr, "\t-r filtered realm\t\t\tonly honor Squid requests 
for this realm. Mandatory if the password is alone in\n\t\t\t\t\t\tthe password 
attribute, acting as the inplicit realm\n");
         fprintf(stderr, "\t-b basedn (REQUIRED)\t\t\tbase dn under where to 
search for users\n");
         fprintf(stderr, "\t-e Encrypted passwords(REQUIRED)\tPassword are 
stored encrypted using HHA1\n");
         fprintf(stderr, "\t-F filter\t\t\t\tuser search filter pattern. %%s = 
login\n");
@@ -646,7 +651,14 @@
 {
     char *password;
     ldapconnect();
-    password = getpassword(requestData->user, requestData->realm);
+    if (*frealm)
+       if (!*delimiter && strcmp(requestData->realm, frealm))
+            password = NULL;
+        else 
+            password = getpassword(requestData->user, (char*)frealm);
+    else
+           password = getpassword(requestData->user, requestData->realm);
+
     if (password != NULL) {
         if (encrpass)
             xstrncpy(requestData->HHA1, password, sizeof(requestData->HHA1));
@@ -660,4 +672,3 @@
     }
 
 }
-
_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev

Reply via email to