On 06/19/2012 05:05 AM, Robert Collins wrote: > A few quick thoughts: > - We'd love patches that make squid explicitly aware of e.g. FIPS > mode, so that we can enforce it ourselves. We've no idea today when we > change something whether its going to impact on such external needs, > and frankly, tracking it is going to be tedious and error prone, > leading to the sort of flip-flop situation you have.
I've attached a first patch. Note that I have not been able to test the helpers yet. Since the md5 functions are called in void functions that cannot return errors up the chain, I had to go up the call chain a little bit. I did put in an assert() in case I missed a callchain somewhere. I've added some comments in the patch on how you can test this without actually running fully in fips mode. Let me know if this is something along the lines of what you were thinking of. > That said, if the FIPS standard doesn't like MD5, there is no need for > use to use it at all, we could use sha1 as a build time option for > cache keys (or take the first N bits of sha1), if that helps: that > would allow us to be entirely MD5 free when desired. I thought about doing that, but it would also require some tricks when people update squid with a cache based on md5. For now I added another entrance function into the MD5 code that you call when you know there is no fips issue. This is what the store_key/MemObject code now calls. I'm interested in feedback and would love something like this to get merged into upstream Paul
diff -aur squid-3.2.0.16-orig/helpers/basic_auth/NCSA/basic_ncsa_auth.cc squid-3.2.0.16/helpers/basic_auth/NCSA/basic_ncsa_auth.cc --- squid-3.2.0.16-orig/helpers/basic_auth/NCSA/basic_ncsa_auth.cc 2012-03-06 21:42:55.000000000 -0500 +++ squid-3.2.0.16/helpers/basic_auth/NCSA/basic_ncsa_auth.cc 2012-06-19 18:49:12.889646542 -0400 @@ -42,6 +42,8 @@ #include <errno.h> #endif +extern int GetFIPSEnabled(void); + static hash_table *hash = NULL; static HASHFREE my_free; @@ -52,6 +54,41 @@ char *passwd; } user_data; +/* + * Return 1 if we are in fips mode and we should disallow MD5 for any + * network and authentication use + * Quick and dirty way to test functionality in fips mode: + * echo "1" > /tmp/fips_enabled + * sudo mount --bind /tmp/fips_enabled /proc/sys/crypto/fips_enabled + * and umount to disable fips again + */ +static int +my_GetFIPSEnabled(void) +{ +#ifdef LINUX + FILE *f; + char d; + size_t size; + + f = fopen("/proc/sys/crypto/fips_enabled", "r"); + if (!f) + return 0; + + size = fread(&d, 1, 1, f); + fclose(f); + if (size != 1) + return 0; + if (d != '1') + return 0; + + return 1; +#else + return 0; /* we only know about Linux fips mode */ +#endif +} + + + static void my_free(void *p) { @@ -139,21 +176,29 @@ u = (user_data *) hash_lookup(hash, user); if (u == NULL) { SEND_ERR("No such user"); + } #if HAVE_CRYPT - } else if (strlen(passwd) <= 8 && strcmp(u->passwd, (char *) crypt(passwd, u->passwd)) == 0) { - // Bug 3107: crypt() DES functionality silently truncates long passwords. - SEND_OK(""); - } else if (strlen(passwd) > 8 && strcmp(u->passwd, (char *) crypt(passwd, u->passwd)) == 0) { - // Bug 3107: crypt() DES functionality silently truncates long passwords. - SEND_ERR("Password too long. Only 8 characters accepted."); + /* using the real crypt() call with return NULL for non-allowed algos passed via the salt */ + else { + char *crypted = crypt(passwd, u->passwd); /* avoid strcmp() on NULL */ + if (crypted && strlen(passwd) <= 8 && strcmp(u->passwd, crypted) == 0) { + // Bug 3107: crypt() DES functionality silently truncates long passwords. + SEND_OK(""); + } else if (strlen(passwd) > 8 && strcmp(u->passwd, crypted) == 0) { + // Bug 3107: crypt() DES functionality silently truncates long passwords. + SEND_ERR("Password too long. Only 8 characters accepted."); + } +#endif + else if (!my_GetFIPSEnabled() && strcmp(u->passwd, (char *) crypt_md5(passwd, u->passwd)) == 0) { + SEND_OK(""); + } else if (!my_GetFIPSEnabled() && strcmp(u->passwd, (char *) md5sum(passwd)) == 0) { + SEND_OK(""); + } else { + SEND_ERR("Wrong password"); + } +#if HAVE_CRYPT + } #endif - } else if (strcmp(u->passwd, (char *) crypt_md5(passwd, u->passwd)) == 0) { - SEND_OK(""); - } else if (strcmp(u->passwd, (char *) md5sum(passwd)) == 0) { - SEND_OK(""); - } else { - SEND_ERR("Wrong password"); - } } if (hash != NULL) { hashFreeItems(hash, my_free); diff -aur squid-3.2.0.16-orig/helpers/basic_auth/NCSA/crypt_md5.h squid-3.2.0.16/helpers/basic_auth/NCSA/crypt_md5.h --- squid-3.2.0.16-orig/helpers/basic_auth/NCSA/crypt_md5.h 2012-03-06 21:42:55.000000000 -0500 +++ squid-3.2.0.16/helpers/basic_auth/NCSA/crypt_md5.h 2012-06-19 18:47:05.979122455 -0400 @@ -20,4 +20,6 @@ /* MD5 hash without salt */ char *md5sum(const char *s); +extern int GetFIPSEnabled(void); + #endif /* _CRYPT_MD5_H */ diff -aur squid-3.2.0.16-orig/helpers/basic_auth/RADIUS/basic_radius_auth.cc squid-3.2.0.16/helpers/basic_auth/RADIUS/basic_radius_auth.cc --- squid-3.2.0.16-orig/helpers/basic_auth/RADIUS/basic_radius_auth.cc 2012-03-06 21:42:55.000000000 -0500 +++ squid-3.2.0.16/helpers/basic_auth/RADIUS/basic_radius_auth.cc 2012-06-19 18:47:05.980122467 -0400 @@ -294,6 +294,11 @@ socklen_t salen; int retry = retries; + if (GetFIPSEnabled()) { + debug("MD5 not allowed in FIPS mode"); + return -1; + } + /* * Build an authentication request */ diff -aur squid-3.2.0.16-orig/helpers/digest_auth/eDirectory/ldap_backend.cc squid-3.2.0.16/helpers/digest_auth/eDirectory/ldap_backend.cc --- squid-3.2.0.16-orig/helpers/digest_auth/eDirectory/ldap_backend.cc 2012-03-06 21:42:55.000000000 -0500 +++ squid-3.2.0.16/helpers/digest_auth/eDirectory/ldap_backend.cc 2012-06-19 18:47:05.980122467 -0400 @@ -676,6 +676,11 @@ xstrncpy(requestData->HHA1, password, sizeof(requestData->HHA1)); else { HASH HA1; +#ifdef LINUX + if (GetFIPSEnabled()) { + requestData->error = -1; /* not allowed to do user auth with md5 in fips */ + } else +#endif DigestCalcHA1("md5", requestData->user, requestData->realm, password, NULL, NULL, HA1, requestData->HHA1); } free(password); diff -aur squid-3.2.0.16-orig/helpers/digest_auth/eDirectory/ldap_backend.h squid-3.2.0.16/helpers/digest_auth/eDirectory/ldap_backend.h --- squid-3.2.0.16-orig/helpers/digest_auth/eDirectory/ldap_backend.h 2012-03-06 21:42:55.000000000 -0500 +++ squid-3.2.0.16/helpers/digest_auth/eDirectory/ldap_backend.h 2012-06-19 18:47:05.980122467 -0400 @@ -7,3 +7,5 @@ #include "digest_common.h" extern int LDAPArguments(int argc, char **argv); extern void LDAPHHA1(RequestData * requestData); +extern int GetFIPSEnabled(void); + diff -aur squid-3.2.0.16-orig/helpers/digest_auth/file/text_backend.cc squid-3.2.0.16/helpers/digest_auth/file/text_backend.cc --- squid-3.2.0.16-orig/helpers/digest_auth/file/text_backend.cc 2012-03-06 21:42:55.000000000 -0500 +++ squid-3.2.0.16/helpers/digest_auth/file/text_backend.cc 2012-06-19 18:47:05.981122479 -0400 @@ -38,6 +38,8 @@ static int ha1mode = 0; static time_t change_time = 0; +extern int GetFIPSEnabled(void); + typedef struct _user_data { hash_link hash; char *passwd; @@ -177,6 +179,11 @@ xstrncpy(requestData->HHA1, u->ha1, sizeof(requestData->HHA1)); } else { HASH HA1; +#ifdef LINUX + if (GetFIPSEnabled()) { + requestData->error = -1; /* not allowed to do user auth with md5 in fips */ + } else +#endif DigestCalcHA1("md5", requestData->user, requestData->realm, u->passwd, NULL, NULL, HA1, requestData->HHA1); } } diff -aur squid-3.2.0.16-orig/helpers/digest_auth/file/text_backend.h squid-3.2.0.16/helpers/digest_auth/file/text_backend.h --- squid-3.2.0.16-orig/helpers/digest_auth/file/text_backend.h 2012-03-06 21:42:55.000000000 -0500 +++ squid-3.2.0.16/helpers/digest_auth/file/text_backend.h 2012-06-19 18:47:05.981122479 -0400 @@ -23,3 +23,5 @@ extern void TextArguments(int argc, char **argv); extern void TextHHA1(RequestData * requestData); +extern int GetFIPSEnabled(void); + diff -aur squid-3.2.0.16-orig/helpers/digest_auth/LDAP/ldap_backend.cc squid-3.2.0.16/helpers/digest_auth/LDAP/ldap_backend.cc --- squid-3.2.0.16-orig/helpers/digest_auth/LDAP/ldap_backend.cc 2012-03-06 21:42:55.000000000 -0500 +++ squid-3.2.0.16/helpers/digest_auth/LDAP/ldap_backend.cc 2012-06-19 18:47:05.981122479 -0400 @@ -644,6 +644,11 @@ xstrncpy(requestData->HHA1, password, sizeof(requestData->HHA1)); else { HASH HA1; +#ifdef LINUX + if (GetFIPSEnabled()) { + requestData->error = -1; /* not allowed to do user auth with md5 in fips */ + } else +#endif DigestCalcHA1("md5", requestData->user, requestData->realm, password, NULL, NULL, HA1, requestData->HHA1); } free(password); diff -aur squid-3.2.0.16-orig/helpers/digest_auth/LDAP/ldap_backend.h squid-3.2.0.16/helpers/digest_auth/LDAP/ldap_backend.h --- squid-3.2.0.16-orig/helpers/digest_auth/LDAP/ldap_backend.h 2012-03-06 21:42:55.000000000 -0500 +++ squid-3.2.0.16/helpers/digest_auth/LDAP/ldap_backend.h 2012-06-19 18:47:05.982122491 -0400 @@ -7,3 +7,4 @@ #include "digest_common.h" extern int LDAPArguments(int argc, char **argv); extern void LDAPHHA1(RequestData * requestData); +extern int GetFIPSEnabled(void); diff -aur squid-3.2.0.16-orig/include/md5.h squid-3.2.0.16/include/md5.h --- squid-3.2.0.16-orig/include/md5.h 2012-03-06 21:42:55.000000000 -0500 +++ squid-3.2.0.16/include/md5.h 2012-06-19 18:47:05.982122491 -0400 @@ -38,7 +38,9 @@ uint32_t in[16]; } SquidMD5_CTX; +SQUIDCEXTERN int GetFIPSEnabled(void); SQUIDCEXTERN void SquidMD5Init(struct SquidMD5Context *context); +SQUIDCEXTERN void SquidMD5InitFipsOK(struct SquidMD5Context *context); SQUIDCEXTERN void SquidMD5Update(struct SquidMD5Context *context, const void *buf, unsigned len); SQUIDCEXTERN void SquidMD5Final(uint8_t digest[16], struct SquidMD5Context *context); SQUIDCEXTERN void SquidMD5Transform(uint32_t buf[4], uint32_t const in[16]); diff -aur squid-3.2.0.16-orig/lib/md5.c squid-3.2.0.16/lib/md5.c --- squid-3.2.0.16-orig/lib/md5.c 2012-03-06 21:42:55.000000000 -0500 +++ squid-3.2.0.16/lib/md5.c 2012-06-19 18:47:05.982122491 -0400 @@ -55,13 +55,60 @@ #define byteSwap(buf,words) #endif +/* + * Return 1 if we are in fips mode and we should disallow MD5 for any + * network and authentication use + * Quick and dirty way to test functionality in fips mode: + * echo "1" > /tmp/fips_enabled + * sudo mount --bind /tmp/fips_enabled /proc/sys/crypto/fips_enabled + * and umount to disable fips again + */ +int +GetFIPSEnabled(void) +{ +#ifdef LINUX + FILE *f; + char d; + size_t size; + + f = fopen("/proc/sys/crypto/fips_enabled", "r"); + if (!f) + return 0; + + size = fread(&d, 1, 1, f); + fclose(f); + if (size != 1) + return 0; + if (d != '1') + return 0; + + return 1; +#else + return 0; /* we only know about Linux fips mode */ +#endif +} + /* - * Start MD5 accumulation. Set bit count to 0 and buffer to mysterious - * initialization constants. + * since these functions can't return success/failure, we assert and + * hope people will fix the code */ void SquidMD5Init(struct SquidMD5Context *ctx) { + if(GetFIPSEnabled()){ + printf("ERROR, MD5 call in fips mode is not allowed!\n"); + assert(GetFIPSEnabled() == 1); + } + SquidMD5InitFipsOK(ctx); +} + +void +SquidMD5InitFipsOK(struct SquidMD5Context *ctx) +{ +/* + * Start MD5 accumulation. Set bit count to 0 and buffer to mysterious + * initialization constants. + */ ctx->buf[0] = 0x67452301; ctx->buf[1] = 0xefcdab89; ctx->buf[2] = 0x98badcfe; diff -aur squid-3.2.0.16-orig/lib/rfc2617.c squid-3.2.0.16/lib/rfc2617.c --- squid-3.2.0.16-orig/lib/rfc2617.c 2012-03-06 21:42:55.000000000 -0500 +++ squid-3.2.0.16/lib/rfc2617.c 2012-06-19 18:47:05.983122503 -0400 @@ -51,6 +51,8 @@ #include "rfc2617.h" #include "md5.h" +extern int GetFIPSEnabled(void); + void CvtHex(const HASH Bin, HASHHEX Hex) { @@ -120,6 +122,11 @@ { SquidMD5_CTX Md5Ctx; + if(GetFIPSEnabled() && strncmp(pszAlg, "md5", 3) == 0) { + fprintf(stderr,"We should not call DigestCalcHA1() with md5 in fips mode\n"); + assert(GetFIPSEnabled() == 1); + } + if (pszUserName) { SquidMD5Init(&Md5Ctx); SquidMD5Update(&Md5Ctx, pszUserName, strlen(pszUserName)); diff -aur squid-3.2.0.16-orig/src/MemObject.cc squid-3.2.0.16/src/MemObject.cc --- squid-3.2.0.16-orig/src/MemObject.cc 2012-03-06 21:42:55.000000000 -0500 +++ squid-3.2.0.16/src/MemObject.cc 2012-06-19 18:47:05.983122503 -0400 @@ -55,7 +55,7 @@ unsigned int ck; SquidMD5_CTX M; static unsigned char digest[16]; - SquidMD5Init(&M); + SquidMD5InitFipsOK(&M); /* override fips check, this md5 use is ok */ SquidMD5Update(&M, (unsigned char *) url, strlen(url)); SquidMD5Final(digest, &M); memcpy(&ck, digest, sizeof(ck)); diff -aur squid-3.2.0.16-orig/src/store_key_md5.cc squid-3.2.0.16/src/store_key_md5.cc --- squid-3.2.0.16-orig/src/store_key_md5.cc 2012-03-06 21:42:55.000000000 -0500 +++ squid-3.2.0.16/src/store_key_md5.cc 2012-06-19 18:47:05.983122503 -0400 @@ -105,7 +105,7 @@ SquidMD5_CTX M; assert(id > 0); debugs(20, 3, "storeKeyPrivate: " << RequestMethodStr(method) << " " << url); - SquidMD5Init(&M); + SquidMD5InitFipsOK(&M); /* override fips check, this md5 use is ok */ SquidMD5Update(&M, (unsigned char *) &id, sizeof(id)); SquidMD5Update(&M, (unsigned char *) &method, sizeof(method)); SquidMD5Update(&M, (unsigned char *) url, strlen(url)); @@ -119,7 +119,7 @@ static cache_key digest[SQUID_MD5_DIGEST_LENGTH]; unsigned char m = (unsigned char) method.id(); SquidMD5_CTX M; - SquidMD5Init(&M); + SquidMD5InitFipsOK(&M); /* override fips check, this md5 use is ok */ SquidMD5Update(&M, &m, sizeof(m)); SquidMD5Update(&M, (unsigned char *) url, strlen(url)); SquidMD5Final(digest, &M); @@ -139,7 +139,7 @@ unsigned char m = (unsigned char) method.id(); const char *url = urlCanonical(request); SquidMD5_CTX M; - SquidMD5Init(&M); + SquidMD5InitFipsOK(&M); /* override fips check, this md5 use is ok */ SquidMD5Update(&M, &m, sizeof(m)); SquidMD5Update(&M, (unsigned char *) url, strlen(url)); diff -aur squid-3.2.0.16-orig/src/wccp2.cc squid-3.2.0.16/src/wccp2.cc --- squid-3.2.0.16-orig/src/wccp2.cc 2012-03-06 21:42:55.000000000 -0500 +++ squid-3.2.0.16/src/wccp2.cc 2012-06-19 18:47:05.984122515 -0400 @@ -52,6 +52,8 @@ #define WCCP_RESPONSE_SIZE 12448 #define WCCP_BUCKETS 256 +extern int GetFIPSEnabled(void); + static int theWccp2Connection = -1; static int wccp2_connected = 0; @@ -587,6 +589,13 @@ debugs(80, 5, "wccp2_update_md5_security: called"); +#ifdef LINUX + if(GetFIPSEnabled()) { + debugs(80, 5, "wccp2_update_md5_security: not allowed in fips mode, abort"); + return 0; + } +#endif + /* The password field, for the MD5 hash, needs to be 8 bytes and NUL padded. */ memset(pwd, 0, sizeof(pwd)); strncpy(pwd, password, sizeof(pwd)); @@ -655,6 +664,13 @@ /* If execution makes it here then we have an MD5 security */ +#ifdef LINUX + if(GetFIPSEnabled()) { + debugs(80, 1, "wccp2_check_security: not allowed in fips mode, abort"); + return 0; + } +#endif + /* The password field, for the MD5 hash, needs to be 8 bytes and NUL padded. */ memset(pwd, 0, sizeof(pwd));