On 29/04/11 12:38, Alex Rousskov wrote:
On 04/27/2011 11:01 PM, Amos Jeffries wrote:
* also fixes a bug where if the input text or output buffer was too
short the coder functions would crop a few bytes off the end of the result.
What do we do now in those cases? Sorry, I cannot tell for sure from the
patch (not enough context), but it seems like we may still crop a few
bytes off the end of the result (but possibly fewer bytes), no?
It will truncate the result at the last full "hunk".
Right now we have a static buffer which is a multiple of base64 "hunks".
So guaranteed not to have unusual output length, though if not big
enough it may truncate a whole count of "hunks".
It is not a problem now with static buffers, but becomes obvious during
debugging when the caller could give any size of buffer space.
Markus may be able to fill in better on this but I believe it was hit
when debugging Kerberos with its dynamic length tokens.
* optimized to short-circuit on several more variations of empty input
and nil result buffer.
Whether short-circuiting is an optimization or expense depends on how
often we get those special cases. I can speculate that zero result
buffer length, for example, is very rare so we should not check for it
as an optimization, unless the check is needed for other reasons.
Okay.
The result length will also avoid buffer overwrites when we start using
this for zero-copy header coding.
The other NULL and input-'\0' byte checks are paranoia.
+ extern int base64_decode(char *result, const char *p, unsigned int
result_size);
I find it rather confusing when the output buffer parameter is called
result and the buffer size is called result size: It feels like I am
being asked to provide the result of the function before calling the
function, but perhaps it is just me.
Yeah. I'm making it result_max_size to be a bit clearer.
Are you aware of any convention about src,dst or dst,src pairing and
which goes first?
All I can think of is the snprintf(out, out_max, ...) example.
I'm tempted to alter the API to be (outBuf, outBufsz, inBuf, inBufSz)
before we get too many more callers for the new functions.
+ // Ensures a nul-terminated result. Will always return non-NULL.
...
+old_base64_decode(const char *p)
{
...
if (!p)
return NULL;
...
Please sync the docs with the code. FWIW, it feels like it would be
safer to just return a pointer to a buffer with nothing but "\0" there,
but I did not check how the callers are using the result.
Please check other non-NULL claims.
Oops cut-n-paste bug. Fixed. The other claims are checked and correct.
In going through the callers of old decode I've upgraded them all to
the new API and killed this wrapper entirely now.
+ /// \return number of bytes filled in result.
+ extern int base64_decode(char *result, const char *p, unsigned int
result_size);
Do we ever use the exact returned value? If not, perhaps it is better to
assert that the result fits and return nothing or result itself? A few
use cases I saw in the patch would produce wrong/weird results if the
result does not fit. Same for the encoding function.
We do. NTLM helpers use it to verify packet length matches the packet
apparently received.
This is a now visible with the removal of old_base64_decode().
+ * AUTHOR: Unknown
No objection, but I am pretty sure this will raise red flags during the
next Debian or similar thorough audit. If we do not know who the author
is, we cannot be sure about the license of that code (there cannot be a
valid license without the copyright owner). I know that you may not
consider "AUTHOR" equivalent to the "copyright owner" but others may
make that assumption. I would remove that line or both of them.
Hmm, Okay. Dropped. The mention of GNU sources covers the logic origins.
I'm just not sure who wrote "with adjustments" on it.
+int
+base64_decode_len(const char *data)
+{
+ int i, j;
+
+ if (!data || !*data)
+ return 0;
+
+ j = 0;
+ for (i = strlen(data) - 1; i>= 0; i--) {
The "j" variable can be declared when initialized.
The "i" variable can be made local to the loop.
Can it? this being one of the rare old-C code sections I took it as
having the declare before any code style all the rest does. Trying the
change now.
+ if (j+4<= result_size) {
+ // Speed optimization: plenty of space, avoid some per-byte checks.
+ result[j++] = (val>> 16)& 0xff; /* High 8 bits */
+ result[j++] = (val>> 8)& 0xff; /* Mid 8 bits */
+ result[j++] = val& 0xff; /* Low 8 bits */
+ } else {
+ // part-quantum goes a bit slower with per-byte checks
+ result[j++] = (val>> 16)& 0xff; /* High 8 bits */
+ if (j == result_size)
+ return j;
+ result[j++] = (val>> 8)& 0xff; /* Mid 8 bits */
+ if (j == result_size)
+ return j;
+ result[j++] = val& 0xff; /* Low 8 bits */
+ }
+ if (j == result_size)
+ return j;
Again, if the callers do not really care, then return or assert as soon
as you see that the result is not going to fit. No need to carefully
produce a malformed result in that case.
They do care. And the result is not completely needless, the helper
debugging uses it to diagnose the exact point of failure.
+ int alen = base64_encode_len(blen);
alen can be declared const here.
I assume you mean in tools.cahcemgr,cc? after fixing the issues above we
no longer have any "int alen" in the sources, and there are many
base64_decode_len().
Which reminds me, you notice how I've made a point of mentioning the
altered source file where each audit comment related to? Please make a
habit to doing that too. It's much easier to find and fix the problems.
+ /// Base-64 encode a string into a given buffer.
+ /// Will not terminate the resulting string.
+ /// \return the number of bytes filled in result.
+ extern int base64_encode(char *result, const char *data, int result_size,
int data_size);
+
+ /// Base-64 encode a string into a given buffer.
+ /// Will terminate the resulting string.
+ /// \return the number of bytes filled in result. Including the terminator.
+ extern int base64_encode_str(char *result, const char *data, int
result_size, int data_size);
Consider s/base64_encode_str/base64_encode_and_terminate/
I considered and picked _str to represent the nil-terminated C-string
nature of the output.
+ /// Calculate the buffer size required to hold the encoded form of a
string of length 'len'.
+ extern int base64_encode_len(int len);
Please document whether base64_encode_len() is suitable for
base64_encode_str() or just for base64_encode().
It is big enough to hold both terminated and non-terminated forms.
The magic numbers still need to be tracked down and documented why they
exist.
+ // Old decoder. Now a wrapper for the new.
+ // Old decoder. Now a wrapper for the new.
One of them is an encoder, but I would actually remove both of those
lines because after your commit, there will be no "old en/decoder" left.
As submitted earlier the "old" API of receiving back a status array was
still present with the "old_" prefix as documented.
New patch attached to fix all these issues.
* Old decoder is now dead and its callers converted.
* There is still two old API encoders (just no longer easily
identified as such per your patch redux request).
* The *_bin() encoder is de-duped as well now.
Amos
--
Please be using
Current Stable Squid 2.7.STABLE9 or 3.1.12
Beta testers wanted for 3.2.0.7 and 3.1.12.1
=== modified file 'helpers/negotiate_auth/kerberos/Makefile.am'
--- helpers/negotiate_auth/kerberos/Makefile.am 2010-08-13 12:05:25 +0000
+++ helpers/negotiate_auth/kerberos/Makefile.am 2011-04-16 15:24:58 +0000
@@ -5,17 +5,22 @@
libexec_PROGRAMS = negotiate_kerberos_auth negotiate_kerberos_auth_test
-SOURCE = negotiate_kerberos_auth.cc base64.cc base64.h
-SOURCE_test = negotiate_kerberos_auth_test.cc base64.cc base64.h
-
-negotiate_kerberos_auth_SOURCES = $(SOURCE)
AM_CPPFLAGS = $(INCLUDES) -I$(srcdir)
-negotiate_kerberos_auth_test_SOURCES = $(SOURCE_test)
-
-
+
+negotiate_kerberos_auth_SOURCES = negotiate_kerberos_auth.cc
negotiate_kerberos_auth_LDFLAGS =
-negotiate_kerberos_auth_LDADD = $(COMPAT_LIB) $(XTRA_LIBS) $(KRB5LIBS)
+negotiate_kerberos_auth_LDADD = \
+ $(top_builddir)/lib/libmiscencoding.la \
+ $(COMPAT_LIB) \
+ $(KRB5LIBS) \
+ $(XTRA_LIBS)
+
+negotiate_kerberos_auth_test_SOURCES = negotiate_kerberos_auth_test.cc
negotiate_kerberos_auth_test_LDFLAGS =
-negotiate_kerberos_auth_test_LDADD = $(COMPAT_LIB) $(XTRA_LIBS) $(KRB5LIBS)
+negotiate_kerberos_auth_test_LDADD = \
+ $(top_builddir)/lib/libmiscencoding.la \
+ $(COMPAT_LIB) \
+ $(KRB5LIBS) \
+ $(XTRA_LIBS)
man_MANS = negotiate_kerberos_auth.8
=== removed file 'helpers/negotiate_auth/kerberos/base64.cc'
--- helpers/negotiate_auth/kerberos/base64.cc 2010-11-20 11:31:38 +0000
+++ helpers/negotiate_auth/kerberos/base64.cc 1970-01-01 00:00:00 +0000
@@ -1,161 +0,0 @@
-/*
- * Markus Moeller has modified the following code from Squid
- */
-
-#include "config.h"
-#include "base64.h"
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-
-
-static void ska_base64_init(void);
-
-static int base64_initialized = 0;
-#define BASE64_VALUE_SZ 256
-int base64_value[BASE64_VALUE_SZ];
-const char base64_code[] =
- "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
-
-
-static void
-ska_base64_init(void)
-{
- int i;
-
- for (i = 0; i < BASE64_VALUE_SZ; i++)
- base64_value[i] = -1;
-
- for (i = 0; i < 64; i++)
- base64_value[(int) base64_code[i]] = i;
- base64_value[(int) '='] = 0;
-
- base64_initialized = 1;
-}
-
-void
-ska_base64_decode(char *result, const char *data, int result_size)
-{
- int j;
- int c;
- long val;
- if (!data)
- return;
- if (!base64_initialized)
- ska_base64_init();
- val = c = 0;
-
- for (j = 0; *data; data++) {
- unsigned int k = ((unsigned char) *data) % BASE64_VALUE_SZ;
- if (base64_value[k] < 0)
- continue;
- val <<= 6;
- val += base64_value[k];
- if (++c < 4)
- continue;
- /* One quantum of four encoding characters/24 bit */
- if (j >= result_size)
- break;
- result[j++] = val >> 16; /* High 8 bits */
- if (j >= result_size)
- break;
- result[j++] = (val >> 8) & 0xff; /* Mid 8 bits */
- if (j >= result_size)
- break;
- result[j++] = val & 0xff; /* Low 8 bits */
- val = c = 0;
- }
- return;
-}
-
-/* adopted from http://ftp.sunet.se/pub2/gnu/vm/base64-encode.c with adjustments */
-void
-ska_base64_encode(char *result, const char *data, int result_size,
- int data_size)
-{
- int bits = 0;
- int char_count = 0;
- int out_cnt = 0;
-
- if (!data)
- return;
-
- if (!base64_initialized)
- ska_base64_init();
-
- while (data_size--) {
- int c = (unsigned char) *data++;
- bits += c;
- char_count++;
- if (char_count == 3) {
- if (out_cnt >= result_size)
- break;
- result[out_cnt++] = base64_code[bits >> 18];
- if (out_cnt >= result_size)
- break;
- result[out_cnt++] = base64_code[(bits >> 12) & 0x3f];
- if (out_cnt >= result_size)
- break;
- result[out_cnt++] = base64_code[(bits >> 6) & 0x3f];
- if (out_cnt >= result_size)
- break;
- result[out_cnt++] = base64_code[bits & 0x3f];
- bits = 0;
- char_count = 0;
- } else {
- bits <<= 8;
- }
- }
- if (char_count != 0) {
- bits <<= 16 - (8 * char_count);
- if (out_cnt >= result_size)
- goto end;
- result[out_cnt++] = base64_code[bits >> 18];
- if (out_cnt >= result_size)
- goto end;
- result[out_cnt++] = base64_code[(bits >> 12) & 0x3f];
- if (char_count == 1) {
- if (out_cnt >= result_size)
- goto end;
- result[out_cnt++] = '=';
- if (out_cnt >= result_size)
- goto end;
- result[out_cnt++] = '=';
- } else {
- if (out_cnt >= result_size)
- goto end;
- result[out_cnt++] = base64_code[(bits >> 6) & 0x3f];
- if (out_cnt >= result_size)
- goto end;
- result[out_cnt++] = '=';
- }
- }
-end:
- if (out_cnt >= result_size) {
- result[result_size - 1] = '\0'; /* terminate */
- } else {
- result[out_cnt] = '\0'; /* terminate */
- }
- return;
-}
-
-int
-ska_base64_encode_len(int len)
-{
- return ((len + 2) / 3 * 4) + 1;
-}
-
-int
-ska_base64_decode_len(const char *data)
-{
- int i, j;
-
- j = 0;
- for (i = strlen(data) - 1; i >= 0; i--) {
- if (data[i] == '=')
- j++;
- if (data[i] != '=')
- break;
- }
- return strlen(data) / 4 * 3 - j;
-}
=== removed file 'helpers/negotiate_auth/kerberos/base64.h'
--- helpers/negotiate_auth/kerberos/base64.h 2010-08-14 00:12:49 +0000
+++ helpers/negotiate_auth/kerberos/base64.h 1970-01-01 00:00:00 +0000
@@ -1,10 +0,0 @@
-/*
- * Markus Moeller has modified the following code from Squid
- */
-
-void ska_base64_decode(char *result, const char *data, int result_size);
-void ska_base64_encode(char *result, const char *data, int result_size,
- int data_size);
-
-int ska_base64_encode_len(int len);
-int ska_base64_decode_len(const char *data);
=== modified file 'helpers/negotiate_auth/kerberos/negotiate_kerberos_auth.cc'
--- helpers/negotiate_auth/kerberos/negotiate_kerberos_auth.cc 2011-03-14 06:15:51 +0000
+++ helpers/negotiate_auth/kerberos/negotiate_kerberos_auth.cc 2011-04-20 13:46:46 +0000
@@ -374,13 +374,12 @@
fprintf(stdout, "BH Invalid negotiate request\n");
continue;
}
- input_token.length = ska_base64_decode_len(buf + 3);
+ input_token.length = base64_decode_len(buf + 3);
debug((char *) "%s| %s: DEBUG: Decode '%s' (decoded length: %d).\n",
LogTime(), PROGRAM, buf + 3, (int) input_token.length);
input_token.value = xmalloc(input_token.length);
- ska_base64_decode((char *) input_token.value, buf + 3, input_token.length);
-
+ base64_decode((char *) input_token.value, buf + 3, input_token.length);
if ((input_token.length >= sizeof ntlmProtocol + 1) &&
(!memcmp(input_token.value, ntlmProtocol, sizeof ntlmProtocol))) {
@@ -427,14 +426,14 @@
if (output_token.length) {
spnegoToken = (const unsigned char *) output_token.value;
spnegoTokenLength = output_token.length;
- token = (char *) xmalloc(ska_base64_encode_len(spnegoTokenLength));
+ token = (char *) xmalloc(base64_encode_len(spnegoTokenLength));
if (token == NULL) {
debug((char *) "%s| %s: ERROR: Not enough memory\n", LogTime(), PROGRAM);
fprintf(stdout, "BH Not enough memory\n");
goto cleanup;
}
- ska_base64_encode(token, (const char *) spnegoToken,
- ska_base64_encode_len(spnegoTokenLength), spnegoTokenLength);
+ base64_encode_str(token, (const char *) spnegoToken,
+ base64_encode_len(spnegoTokenLength), spnegoTokenLength);
if (check_gss_err(major_status, minor_status, "gss_accept_sec_context()", log))
goto cleanup;
=== modified file 'helpers/negotiate_auth/kerberos/negotiate_kerberos_auth_test.cc'
--- helpers/negotiate_auth/kerberos/negotiate_kerberos_auth_test.cc 2011-03-14 06:15:51 +0000
+++ helpers/negotiate_auth/kerberos/negotiate_kerberos_auth_test.cc 2011-04-20 13:45:51 +0000
@@ -196,9 +196,9 @@
goto cleanup;
if (output_token.length) {
- token = (char *) xmalloc(ska_base64_encode_len(output_token.length));
- ska_base64_encode(token, (const char *) output_token.value,
- ska_base64_encode_len(output_token.length), output_token.length);
+ token = (char *) xmalloc(base64_encode_len(output_token.length));
+ base64_encode_str(token, (const char *) output_token.value,
+ base64_encode_len(output_token.length), output_token.length);
}
cleanup:
gss_delete_sec_context(&minor_status, &gss_context, NULL);
=== modified file 'helpers/negotiate_auth/wrapper/Makefile.am'
--- helpers/negotiate_auth/wrapper/Makefile.am 2011-04-15 11:51:15 +0000
+++ helpers/negotiate_auth/wrapper/Makefile.am 2011-04-16 04:35:08 +0000
@@ -4,5 +4,8 @@
libexec_PROGRAMS = negotiate_wrapper_auth
-negotiate_wrapper_auth_SOURCES = negotiate_wrapper.cc nw_base64.cc nw_base64.h
-negotiate_wrapper_auth_LDADD = $(COMPAT_LIB) $(XTRA_LIBS)
+negotiate_wrapper_auth_SOURCES = negotiate_wrapper.cc
+negotiate_wrapper_auth_LDADD = \
+ $(top_builddir)/lib/libmiscencoding.la \
+ $(COMPAT_LIB) \
+ $(XTRA_LIBS)
=== modified file 'helpers/negotiate_auth/wrapper/negotiate_wrapper.cc'
--- helpers/negotiate_auth/wrapper/negotiate_wrapper.cc 2011-04-19 13:20:31 +0000
+++ helpers/negotiate_auth/wrapper/negotiate_wrapper.cc 2011-04-20 06:11:07 +0000
@@ -26,7 +26,7 @@
*/
#include "config.h"
-#include "nw_base64.h"
+#include "base64.h"
#if HAVE_STRING_H
#include <string.h>
@@ -346,7 +346,7 @@
fprintf(stdout, "BH Invalid negotiate request\n");
continue;
}
- length = nw_base64_decode_len(buf + 3);
+ length = base64_decode_len(buf + 3);
if (debug)
fprintf(stderr, "%s| %s: Decode '%s' (decoded length: %d).\n",
LogTime(), PROGRAM, buf + 3, (int) length);
@@ -356,7 +356,7 @@
return 1;
}
- nw_base64_decode(token, buf + 3, length);
+ base64_decode(token, buf + 3, length);
if ((static_cast<size_t>(length) >= sizeof(ntlmProtocol) + 1) &&
(!memcmp(token, ntlmProtocol, sizeof ntlmProtocol))) {
=== removed file 'helpers/negotiate_auth/wrapper/nw_base64.cc'
--- helpers/negotiate_auth/wrapper/nw_base64.cc 2011-04-16 14:43:18 +0000
+++ helpers/negotiate_auth/wrapper/nw_base64.cc 1970-01-01 00:00:00 +0000
@@ -1,83 +0,0 @@
-/*
- * Markus Moeller has modified the following code from Squid
- */
-#include "config.h"
-#include "nw_base64.h"
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-
-
-static void nw_base64_init(void);
-
-static int base64_initialized = 0;
-#define BASE64_VALUE_SZ 256
-int base64_value[BASE64_VALUE_SZ];
-const char base64_code[] =
- "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
-
-
-static void
-nw_base64_init(void)
-{
- int i;
-
- for (i = 0; i < BASE64_VALUE_SZ; i++)
- base64_value[i] = -1;
-
- for (i = 0; i < 64; i++)
- base64_value[(int) base64_code[i]] = i;
- base64_value[(int)'='] = 0;
-
- base64_initialized = 1;
-}
-
-void
-nw_base64_decode(char *result, const char *data, int result_size)
-{
- int j;
- int c;
- long val;
- if (!data)
- return;
- if (!base64_initialized)
- nw_base64_init();
- val = c = 0;
-
- for (j = 0; *data; data++) {
- unsigned int k = ((unsigned char) *data) % BASE64_VALUE_SZ;
- if (base64_value[k] < 0)
- continue;
- val <<= 6;
- val += base64_value[k];
- if (++c < 4)
- continue;
- /* One quantum of four encoding characters/24 bit */
- if (j >= result_size)
- break;
- result[j++] = val >> 16; /* High 8 bits */
- if (j >= result_size)
- break;
- result[j++] = (val >> 8) & 0xff; /* Mid 8 bits */
- if (j >= result_size)
- break;
- result[j++] = val & 0xff; /* Low 8 bits */
- val = c = 0;
- }
- return;
-}
-
-int
-nw_base64_decode_len(const char *data)
-{
- int i, j;
-
- j = 0;
- for (i = strlen(data) - 1; i >= 0; i--) {
- if (data[i] == '=')
- j++;
- if (data[i] != '=')
- break;
- }
- return strlen(data) / 4 * 3 - j;
-}
=== removed file 'helpers/negotiate_auth/wrapper/nw_base64.h'
--- helpers/negotiate_auth/wrapper/nw_base64.h 2011-04-15 11:51:15 +0000
+++ helpers/negotiate_auth/wrapper/nw_base64.h 1970-01-01 00:00:00 +0000
@@ -1,11 +0,0 @@
-#ifndef _NW_BASE64_H
-#define _NW_BASE64_H
-
-/*
- * Markus Moeller has modified the following code from Squid
- */
-
-void nw_base64_decode(char *result, const char *data, int result_size);
-int nw_base64_decode_len(const char *data);
-
-#endif
=== modified file 'helpers/ntlm_auth/fake/ntlm_fake_auth.cc'
--- helpers/ntlm_auth/fake/ntlm_fake_auth.cc 2011-01-25 21:30:11 +0000
+++ helpers/ntlm_auth/fake/ntlm_fake_auth.cc 2011-04-29 13:02:08 +0000
@@ -149,6 +149,7 @@
main(int argc, char *argv[])
{
char buf[HELPER_INPUT_BUFFER];
+ char decodedBuf[HELPER_INPUT_BUFFER];
int buflen = 0;
char user[NTLM_MAX_FIELD_LENGTH], domain[NTLM_MAX_FIELD_LENGTH];
char *p;
@@ -173,13 +174,16 @@
if ((p = strchr(buf, '\n')) != NULL)
*p = '\0'; /* strip \n */
buflen = strlen(buf); /* keep this so we only scan the buffer for \0 once per loop */
- if (buflen > 3)
- packet = (ntlmhdr*)base64_decode(buf + 3);
+ if (buflen > 3) {
+ decodedLen = base64_decode(decodedBuf, buf+3, sizeof(decodedBuf));
+ decodedBuf[decodedLen] = '\0';
+ packet = (ntlmhdr*)decodedBuf;
+ }
if (buflen > 3 && NTLM_packet_debug_enabled) {
strncpy(helper_command, buf, 2);
helper_command[2] = '\0';
debug("Got '%s' from Squid with data:\n", helper_command);
- hex_dump((unsigned char*)packet, ((buflen - 3) * 3) / 4);
+ hex_dump((unsigned char *)decodedBuf, decodedLen);
} else
debug("Got '%s' from Squid\n", buf);
=== modified file 'helpers/ntlm_auth/smb_lm/ntlm_smb_lm_auth.cc'
--- helpers/ntlm_auth/smb_lm/ntlm_smb_lm_auth.cc 2011-04-09 02:37:59 +0000
+++ helpers/ntlm_auth/smb_lm/ntlm_smb_lm_auth.cc 2011-04-29 13:06:29 +0000
@@ -506,9 +506,9 @@
{
ntlmhdr *fast_header;
char buf[NTLM_BLOB_BUFFER_SIZE];
+ char decoded[NTLM_BLOB_BUFFER_SIZE];
const char *ch;
- char *ch2, *decoded, *cred = NULL;
- int plen;
+ char *ch2, *cred = NULL;
if (fgets(buf, NTLM_BLOB_BUFFER_SIZE, stdin) == NULL) {
fprintf(stderr, "fgets() failed! dying..... errno=%d (%s)\n", errno,
@@ -525,12 +525,9 @@
if (memcmp(buf, "KK ", 3) == 0) { /* authenticate-request */
/* figure out what we got */
- decoded = base64_decode(buf + 3);
- /* Note: we don't need to manage memory at this point, since
- * base64_decode returns a pointer to static storage.
- */
+ int decodeLen = base64_decode(decoded, buf+3, sizeof(decoded), ch-buf+3);
- if (!decoded) { /* decoding failure, return error */
+ if (!*decoded) { /* decoding failure, return error */
SEND("NA Packet format error, couldn't base64-decode");
return;
}
@@ -553,10 +550,9 @@
/* notreached */
case NTLM_AUTHENTICATE:
/* check against the DC */
- plen = strlen(buf) * 3 / 4; /* we only need it here. Optimization */
signal(SIGALRM, timeout_during_auth);
alarm(30);
- cred = ntlm_check_auth((ntlm_authenticate *) decoded, plen);
+ cred = ntlm_check_auth((ntlm_authenticate *) decoded, decodedLen);
alarm(0);
signal(SIGALRM, SIG_DFL);
if (got_timeout != 0) {
@@ -585,7 +581,7 @@
* without it.. */
if (nb_error != 0) { /* netbios-level error */
send_bh_or_ld("NetBios error!",
- (ntlm_authenticate *) decoded, plen);
+ (ntlm_authenticate *) decoded, decodedLen);
fprintf(stderr, "NetBios error code %d (%s)\n", nb_error,
RFCNB_Error_Strings[abs(nb_error)]);
return;
@@ -596,7 +592,7 @@
SEND("NA Authentication failed");
/*
* send_bh_or_ld("SMB success, but no creds. Internal error?",
- * (ntlm_authenticate *) decoded, plen);
+ * (ntlm_authenticate *) decoded, decodedLen);
*/
return;
case SMBC_ERRDOS:
@@ -619,7 +615,7 @@
return;
default:
send_bh_or_ld("DOS Error",
- (ntlm_authenticate *) decoded, plen);
+ (ntlm_authenticate *) decoded, decodedLen);
return;
}
case SMBC_ERRSRV: /* server errors */
@@ -634,16 +630,16 @@
return;
default:
send_bh_or_ld("Server Error",
- (ntlm_authenticate *) decoded, plen);
+ (ntlm_authenticate *) decoded, decodedLen);
return;
}
case SMBC_ERRHRD: /* hardware errors don't really matter */
send_bh_or_ld("Domain Controller Hardware error",
- (ntlm_authenticate *) decoded, plen);
+ (ntlm_authenticate *) decoded, decodedLen);
return;
case SMBC_ERRCMD:
send_bh_or_ld("Domain Controller Command Error",
- (ntlm_authenticate *) decoded, plen);
+ (ntlm_authenticate *) decoded, decodedLen);
return;
}
SEND("BH unknown internal error.");
=== modified file 'include/base64.h'
--- include/base64.h 2010-11-02 00:12:43 +0000
+++ include/base64.h 2011-04-29 14:15:21 +0000
@@ -5,9 +5,47 @@
extern "C" {
#endif
- extern char *base64_decode(const char *coded);
+ // Decoding functions
+
+ /// Calculate the decoded length of a given nul-terminated encoded string.
+ /// NULL pointer and empty strings are accepted, result is zero.
+ /// Any return value <= zero means no decoded result can be produced.
+ extern int base64_decode_len(const char *encodedData);
+
+ /// Decode a base-64 encoded blob into a provided buffer.
+ /// Will not terminate the resulting string.
+ /// In-place decoding overlap is supported if result is equal or earlier that the source pointer.
+ ///
+ /// \return number of bytes filled in result.
+ extern int base64_decode(char *result, const char *p, unsigned int result_max_size);
+
+
+ // Encoding functions
+
+ /// Calculate the buffer size required to hold the encoded form of
+ /// a string of length 'decodedLen' including all terminator bytes.
+ extern int base64_encode_len(int decodedLen);
+
+ /// Base-64 encode a string into a given buffer.
+ /// Will not terminate the resulting string.
+ /// \return the number of bytes filled in result.
+ extern int base64_encode(char *result, const char *data, int result_max_size, int data_size);
+
+ /// Base-64 encode a string into a given buffer.
+ /// Will terminate the resulting string.
+ /// \return the number of bytes filled in result. Including the terminator.
+ extern int base64_encode_str(char *result, const char *data, int result_max_size, int data_size);
+
+ // Old encoder. Now a wrapper for the new. Takes a binary array of known length.
+ // Output is presented in a static buffer which will only remain valid until next call.
+ // Ensures a nul-terminated result. Will always return non-NULL.
+ extern const char *base64_encode_bin(const char *data, int len);
+
+ // Old encoder. Now a wrapper for the new.
+ // Output is presented in a static buffer which will only remain valid until next call.
+ // Ensures a nul-terminated result. Will always return non-NULL.
extern const char *base64_encode(const char *decoded);
- extern const char *base64_encode_bin(const char *data, int len);
+
#ifdef __cplusplus
}
=== modified file 'lib/base64.c'
--- lib/base64.c 2010-11-01 05:44:28 +0000
+++ lib/base64.c 2011-04-29 14:06:07 +0000
@@ -1,5 +1,9 @@
/*
* $Id$
+ *
+ * AUTHOR: Markus Moeller
+ *
+ * Encoders adopted from http://ftp.sunet.se/pub2/gnu/vm/base64-encode.c with adjustments.
*/
#include "config.h"
@@ -37,19 +41,35 @@
base64_initialized = 1;
}
-char *
-base64_decode(const char *p)
-{
- static char result[BASE64_RESULT_SZ];
- int j;
+int
+base64_decode_len(const char *data)
+{
+ if (!data || !*data)
+ return 0;
+
+ int terminatorLen = 0;
+ int dataLen = strlen(data);
+ for (int i = dataLen - 1; i >= 0; i--) {
+ if (data[i] == '=')
+ terminatorLen++;
+ if (data[i] != '=')
+ break;
+ }
+ return dataLen / 4 * 3 - terminatorLen;
+}
+
+int
+base64_decode(char *result, const char *p, unsigned int result_size)
+{
+ int j = 0;
int c;
long val;
- if (!p)
- return NULL;
+ if (!p || !result || result_size == 0)
+ return j;
if (!base64_initialized)
base64_init();
val = c = 0;
- for (j = 0; *p && j + 4 < BASE64_RESULT_SZ; p++) {
+ for (; *p; p++) {
unsigned int k = ((unsigned char) *p) % BASE64_VALUE_SZ;
if (base64_value[k] < 0)
continue;
@@ -58,85 +78,104 @@
if (++c < 4)
continue;
/* One quantum of four encoding characters/24 bit */
- result[j++] = (val >> 16) & 0xff; /* High 8 bits */
- result[j++] = (val >> 8) & 0xff; /* Mid 8 bits */
- result[j++] = val & 0xff; /* Low 8 bits */
+ if (j+4 <= result_size) {
+ // Speed optimization: plenty of space, avoid some per-byte checks.
+ result[j++] = (val >> 16) & 0xff; /* High 8 bits */
+ result[j++] = (val >> 8) & 0xff; /* Mid 8 bits */
+ result[j++] = val & 0xff; /* Low 8 bits */
+ } else {
+ // part-quantum goes a bit slower with per-byte checks
+ result[j++] = (val >> 16) & 0xff; /* High 8 bits */
+ if (j == result_size)
+ return j;
+ result[j++] = (val >> 8) & 0xff; /* Mid 8 bits */
+ if (j == result_size)
+ return j;
+ result[j++] = val & 0xff; /* Low 8 bits */
+ }
+ if (j == result_size)
+ return j;
val = c = 0;
}
- result[j] = 0;
- return result;
-}
-
-/* adopted from http://ftp.sunet.se/pub2/gnu/vm/base64-encode.c with adjustments */
+ return j;
+}
+
+int
+base64_encode_len(int len)
+{
+ // NP: some magic numbers + potential nil-terminator
+ return ((len + 2) / 3 * 4) + 1;
+}
+
const char *
-base64_encode(const char *decoded_str)
+base64_encode(const char *data)
{
static char result[BASE64_RESULT_SZ];
- int bits = 0;
- int char_count = 0;
- int out_cnt = 0;
- int c;
-
- if (!decoded_str)
- return decoded_str;
-
- if (!base64_initialized)
- base64_init();
-
- while ((c = (unsigned char) *decoded_str++) && out_cnt < sizeof(result) - 5) {
- bits += c;
- char_count++;
- if (char_count == 3) {
- result[out_cnt++] = base64_code[bits >> 18];
- result[out_cnt++] = base64_code[(bits >> 12) & 0x3f];
- result[out_cnt++] = base64_code[(bits >> 6) & 0x3f];
- result[out_cnt++] = base64_code[bits & 0x3f];
- bits = 0;
- char_count = 0;
- } else {
- bits <<= 8;
- }
- }
- if (char_count != 0) {
- bits <<= 16 - (8 * char_count);
- result[out_cnt++] = base64_code[bits >> 18];
- result[out_cnt++] = base64_code[(bits >> 12) & 0x3f];
- if (char_count == 1) {
- result[out_cnt++] = '=';
- result[out_cnt++] = '=';
- } else {
- result[out_cnt++] = base64_code[(bits >> 6) & 0x3f];
- result[out_cnt++] = '=';
- }
- }
- result[out_cnt] = '\0'; /* terminate */
+ base64_encode_str(result, decoded_str, sizeof(result), strlen(decoded_str));
return result;
}
-/* adopted from http://ftp.sunet.se/pub2/gnu/vm/base64-encode.c with adjustments */
const char *
base64_encode_bin(const char *data, int len)
{
static char result[BASE64_RESULT_SZ];
+ base64_encode_str(result, decoded_str, sizeof(result), len);
+ return result;
+}
+
+int
+base64_encode_str(char *result, const char *data, int result_size, int data_size)
+{
+ int used = base64_encode(result, data, result_size, data_size);
+ /* terminate */
+ if (used >= result_size) {
+ result[result_size - 1] = '\0';
+ return result_size;
+ } else {
+ result[used++] = '\0';
+ }
+ return used;
+}
+
+/* adopted from http://ftp.sunet.se/pub2/gnu/vm/base64-encode.c with adjustments */
+int
+base64_encode(char *result, const char *data, int result_size, int data_size)
+{
int bits = 0;
int char_count = 0;
int out_cnt = 0;
- if (!data)
- return data;
+ if (!data || !*data || !result || result_size < 1 || data_size < 1)
+ return 0;
if (!base64_initialized)
base64_init();
- while (len-- && out_cnt < sizeof(result) - 5) {
+ while (data_size--) {
int c = (unsigned char) *data++;
bits += c;
char_count++;
if (char_count == 3) {
- result[out_cnt++] = base64_code[bits >> 18];
- result[out_cnt++] = base64_code[(bits >> 12) & 0x3f];
- result[out_cnt++] = base64_code[(bits >> 6) & 0x3f];
- result[out_cnt++] = base64_code[bits & 0x3f];
+ if (out_cnt >= result_size)
+ break;
+ if (out_cnt+4 <= result_size) {
+ result[out_cnt++] = base64_code[bits >> 18];
+ result[out_cnt++] = base64_code[(bits >> 12) & 0x3f];
+ result[out_cnt++] = base64_code[(bits >> 6) & 0x3f];
+ result[out_cnt++] = base64_code[bits & 0x3f];
+ } else {
+ // part-quantum goes a bit slower with per-byte checks
+ result[out_cnt++] = base64_code[bits >> 18];
+ if (out_cnt >= result_size)
+ break;
+ result[out_cnt++] = base64_code[(bits >> 12) & 0x3f];
+ if (out_cnt >= result_size)
+ break;
+ result[out_cnt++] = base64_code[(bits >> 6) & 0x3f];
+ if (out_cnt >= result_size)
+ break;
+ result[out_cnt++] = base64_code[bits & 0x3f];
+ }
bits = 0;
char_count = 0;
} else {
@@ -145,16 +184,27 @@
}
if (char_count != 0) {
bits <<= 16 - (8 * char_count);
+ if (out_cnt >= result_size)
+ return result_size;
result[out_cnt++] = base64_code[bits >> 18];
+ if (out_cnt >= result_size)
+ return result_size;
result[out_cnt++] = base64_code[(bits >> 12) & 0x3f];
if (char_count == 1) {
+ if (out_cnt >= result_size)
+ return result_size;
result[out_cnt++] = '=';
+ if (out_cnt >= result_size)
+ return result_size;
result[out_cnt++] = '=';
} else {
+ if (out_cnt >= result_size)
+ return result_size;
result[out_cnt++] = base64_code[(bits >> 6) & 0x3f];
+ if (out_cnt >= result_size)
+ return result_size;
result[out_cnt++] = '=';
}
}
- result[out_cnt] = '\0'; /* terminate */
- return result;
+ return (out_cnt >= result_size?result_size:out_cnt);
}
=== modified file 'src/HttpHeader.cc'
--- src/HttpHeader.cc 2011-03-22 12:23:25 +0000
+++ src/HttpHeader.cc 2011-04-29 13:13:00 +0000
@@ -1414,7 +1414,10 @@
if (!*field) /* no authorization cookie */
return NULL;
- return base64_decode(field);
+ static char decodedAuthToken[8192];
+ int decodedLen = base64_decode(decodedAuthToken, fields, sizeof(decodedAuthToken));
+ decodedAuthToken[decodedLen] = '\0';
+ return decodedAuthToken;
}
ETag
=== modified file 'tools/cachemgr.cc'
--- tools/cachemgr.cc 2011-04-12 11:33:32 +0000
+++ tools/cachemgr.cc 2011-04-29 13:37:29 +0000
@@ -1053,16 +1053,17 @@
return;
/* host | time | user | passwd */
- snprintf(buf, sizeof(buf), "%s|%d|%s|%s",
- req->hostname,
- (int) now,
- req->user_name ? req->user_name : "",
- req->passwd);
-
+ int bufLen = snprintf(buf, sizeof(buf), "%s|%d|%s|%s",
+ req->hostname,
+ (int) now,
+ req->user_name ? req->user_name : "",
+ req->passwd);
debug("cmgr: pre-encoded for pub: %s\n", buf);
- debug("cmgr: encoded: '%s'\n", base64_encode(buf));
- req->pub_auth = xstrdup(base64_encode(buf));
+ int encodedLen = base64_encode_len(bufLen);
+ req->pub_auth = (char *) xmalloc(encodedLen);
+ base64_encode_str(req->pub_auth, buf, encodedLen, bufLen);
+ debug("cmgr: encoded: '%s'\n", req->pub_auth);
}
static void
@@ -1080,7 +1081,9 @@
if (!req->pub_auth || strlen(req->pub_auth) < 4 + strlen(safe_str(req->hostname)))
return;
- buf = xstrdup(base64_decode(req->pub_auth));
+ const int decodedLen = base64_decode_len(req->pub_auth);
+ buf = (char*)xmalloc(decodedLen);
+ base64_decode(buf, req->pub_auth, decodedLen);
debug("cmgr: length ok\n");
@@ -1136,16 +1139,18 @@
{
static char buf[1024];
size_t stringLength = 0;
- const char *str64;
if (!req->passwd)
return "";
- snprintf(buf, sizeof(buf), "%s:%s",
- req->user_name ? req->user_name : "",
- req->passwd);
+ int bufLen = snprintf(buf, sizeof(buf), "%s:%s",
+ req->user_name ? req->user_name : "",
+ req->passwd);
- str64 = base64_encode(buf);
+ int encodedLen = base64_encode_len(bufLen);
+ char *str64 = static_cast<char*>(xmalloc(encodedLen));
+ if (base64_encode_str(str64, buf, encodedLen, bufLen) == 0)
+ return "";
stringLength += snprintf(buf, sizeof(buf), "Authorization: Basic %s\r\n", str64);
