On 7/10/2013 10:54 p.m., Vonlanthen, Elmar wrote:
Hello all

There are some problems with the helper module ntlm_fake_auth. I did the tests 
with Squid-3.2.13 but 3.3.9 is affected as well.

Hi Volanthen,
  First off thank you for testing this in such detail.

FYI: in future this type of code related post should be sent to squid-dev mailing list. Thank you.

The big problem we have is that none of the current development team are able to test the NTLM helpers properly. So your assistance in that area is very welcome. I am applying the latter two fixes now. The first one is affected by those and the behaviour may change as a result.

The first problem is that the NTLM response header of type "TT" will be 
generated wrong.

This one has been generated with the new helper ntlm_fake_auth:
ntlm_fake_auth.cc(219): pid=29811 :sending 'TT' to squid with data:
[0000]   4E 54 4C 4D 53 53 50 00   02 00 00 00 09 00 09 00   NTLMSSP. ........
[0010]   28 00 00 00 07 82 08 A2   CE 7D 62 FA 44 55 80 E0   ........ ..b.DU..
[0020]   00 00 00 00 00 00 3A 00   57 4F 52 4B 47 52 4F 55   ........ WORKGROU
[0030]   50                                                  P

And this one with the old helper fakeauth_auth (which is working):
ntlm-auth[31700](fakeauth_auth.c:421): sending 'TT' to squid with data:
[0000]   4E 54 4C 4D 53 53 50 00   02 00 00 00 0A 00 0A 00   NTLMSSP. ........
[0010]   30 00 00 00 07 82 08 A2   7B E5 5C 0B 49 DB 6D 36   0....... ....I.m6
[0020]   00 00 00 00 00 00 00 00   00 00 00 00 3A 00 00 00   ........ ........
[0030]   00 00 00 00 00 00 00 00   00 00 00 00               ........ ....

It seems that the total length of the header has a wrong size and the char ":" (0x3a) 
will be placed in the field "reserved". The client doesn't accept the packet with the new 
response header and is sending a RST.

Total length being the wrong size can be expected after the 3rd problem you mention below. The entire structure is dependent on field sizes being calculated and encoded correctly. Otherwise random extra space or barbage ends up in the output. Those 00 octets at the end look suspiciously like such. Can you please test this problem again and re-do the analysis after the size and case patches are applied. A re-analysis on fixed field contents may help with that TODO answer...


Now, if I tweak the header with setting authenticate_ntlm_domain to an empty 
string and tweaking the target value and payload, it is working (ugly 
workaround, I know):

diff -aur a/helpers/ntlm_auth/fake/ntlm_fake_auth.cc 
b/helpers/ntlm_auth/fake/ntlm_fake_auth.cc
--- a/helpers/ntlm_auth/fake/ntlm_fake_auth.cc  2013-09-30 11:48:40.231386531 
+0200
+++ b/helpers/ntlm_auth/fake/ntlm_fake_auth.cc  2013-10-01 10:28:07.727699795 
+0200
@@ -96,7 +96,7 @@
  #define SEND4(X,Y,Z,W) {debug("sending '" X "' to squid\n",Y,Z,W); printf(X 
"\n",Y,Z,W);}
  #endif
-const char *authenticate_ntlm_domain = "WORKGROUP";
+const char *authenticate_ntlm_domain = "";
  int strip_domain_enabled = 0;
  int NTLM_packet_debug_enabled = 0;
@@ -209,8 +209,14 @@
              } else {
                  ntlm_make_challenge(&chal, authenticate_ntlm_domain, NULL, 
nonce, NTLM_NONCE_LEN, NTLM_NEGOTIATE_ASCII);
              }
+            // doesn't work with this:
              // TODO: find out what this context means, and why only the fake 
auth helper contains it.
-            chal.context_high = htole32(0x003a<<16);
+            //chal.context_high = htole32(0x003a<<16);
+            // twead payload, offset and length to get it working:
+            chal.payload[4] = 0x3a;
+            chal.target.offset = 48;
+            chal.target.len = 10;
+            chal.target.maxlen = 18;
len = sizeof(chal) - sizeof(chal.payload) + le16toh(chal.target.maxlen);
              data = (char *) base64_encode_bin((char *) &chal, len);

In the code there is a comment "TODO: find out what this context means...". I 
think there is really some work to do. ;-)

Yes there is. The other helpers are working perfectly fine without this context value being set. Which is why it was elided from these versions. Before this gets changed yet again, do you have any explanation for what those seemingly arbitrary random values mean?


Another problem is the presentation of domain and username. First the domain 
was previously shown in uppercase. This could make problems with case sensitive 
acls:
And the last problem is a wrong specification of the string length for domain 
and username. The last char of each string is missing. The problem is that the 
string length will be initialized with -1:

Okay. Slightly polished patches applied for these.

Amos

Reply via email to