From: Alan T. DeKok Date: Sat, 8 Aug 2015 15:44:27 +0000 (+0200) Subject: More tests for tunnel password encryption X-Git-Tag: release_3_0_10~256 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d1cdce1b07d4630832f4e834d22ddc5d06557c5a;p=thirdparty%2Ffreeradius-server.git More tests for tunnel password encryption --- diff --git a/src/lib/radius.c b/src/lib/radius.c index a7de70c5c69..c45c4cc22db 100644 --- a/src/lib/radius.c +++ b/src/lib/radius.c @@ -595,62 +595,59 @@ static void make_passwd(uint8_t *output, ssize_t *outlen, memcpy(output, passwd, len); } + static void make_tunnel_passwd(uint8_t *output, ssize_t *outlen, uint8_t const *input, size_t inlen, size_t room, char const *secret, uint8_t const *vector) { FR_MD5_CTX context, old; uint8_t digest[AUTH_VECTOR_LEN]; - uint8_t passwd[MAX_STRING_LEN + AUTH_VECTOR_LEN]; - int i, n; - int len; + size_t i, n; + size_t encrypted_len; /* - * Be paranoid. + * The password gets encoded with a 1-byte "length" + * field. Ensure that it doesn't overflow. */ if (room > 253) room = 253; /* - * Account for 2 bytes of the salt, and round the room - * available down to the nearest multiple of 16. Then, - * subtract one from that to account for the length byte, - * and the resulting number is the upper bound on the data - * to copy. - * - * We could short-cut this calculation just be forcing - * inlen to be no more than 239. It would work for all - * VSA's, as we don't pack multiple VSA's into one - * attribute. - * - * However, this calculation is more general, if a little - * complex. And it will work in the future for all possible - * kinds of weird attribute packing. + * Limit the maximum size of the input password. 2 bytes + * are taken up by the salt, and one by the encoded + * "length" field. Note that if we have a tag, the + * "room" will be 252 octets, not 253 octets. */ - room -= 2; - room -= (room & 0x0f); - room--; - - if (inlen > room) inlen = room; + if (inlen > (room - 3)) inlen = room - 3; /* - * Length of the encrypted data is password length plus - * one byte for the length of the password. + * Length of the encrypted data is the clear-text + * password length plus one byte which encodes the length + * of the password. We round up to the nearest encoding + * block. Note that this can result in the encoding + * length being more than 253 octets. */ - len = inlen + 1; - if ((len & 0x0f) != 0) { - len += 0x0f; - len &= ~0x0f; + encrypted_len = inlen + 1; + if ((encrypted_len & 0x0f) != 0) { + encrypted_len += 0x0f; + encrypted_len &= ~0x0f; } - *outlen = len + 2; /* account for the salt */ /* - * Copy the password over. + * We need 2 octets for the salt, followed by the actual + * encrypted data. */ - memcpy(passwd + 3, input, inlen); - memset(passwd + 3 + inlen, 0, sizeof(passwd) - 3 - inlen); + if (encrypted_len > (room - 2)) encrypted_len = room - 2; + + *outlen = encrypted_len + 2; /* account for the salt */ /* - * Generate salt. The RFC's say: + * Copy the password over, and zero-fill the remainder. + */ + memcpy(output + 3, input, inlen); + memset(output + 3 + inlen, 0, *outlen - 3 - inlen); + + /* + * Generate salt. The RFCs say: * * The high bit of salt[0] must be set, each salt in a * packet should be unique, and they should be random @@ -658,33 +655,40 @@ static void make_tunnel_passwd(uint8_t *output, ssize_t *outlen, * So, we set the high bit, add in a counter, and then * add in some CSPRNG data. should be OK.. */ - passwd[0] = (0x80 | ( ((salt_offset++) & 0x0f) << 3) | + output[0] = (0x80 | ( ((salt_offset++) & 0x0f) << 3) | (fr_rand() & 0x07)); - passwd[1] = fr_rand(); - passwd[2] = inlen; /* length of the password string */ + output[1] = fr_rand(); + output[2] = inlen; /* length of the password string */ fr_md5_init(&context); fr_md5_update(&context, (uint8_t const *) secret, strlen(secret)); old = context; fr_md5_update(&context, vector, AUTH_VECTOR_LEN); - fr_md5_update(&context, &passwd[0], 2); + fr_md5_update(&context, &output[0], 2); + + for (n = 0; n < encrypted_len; n += AUTH_PASS_LEN) { + size_t block_len; - for (n = 0; n < len; n += AUTH_PASS_LEN) { if (n > 0) { context = old; fr_md5_update(&context, - passwd + 2 + n - AUTH_PASS_LEN, + output + 2 + n - AUTH_PASS_LEN, AUTH_PASS_LEN); } fr_md5_final(digest, &context); - for (i = 0; i < AUTH_PASS_LEN; i++) { - passwd[i + 2 + n] ^= digest[i]; + if ((2 + n + AUTH_PASS_LEN) < room) { + block_len = AUTH_PASS_LEN; + } else { + block_len = room - 2 - n; + } + + for (i = 0; i < block_len; i++) { + output[i + 2 + n] ^= digest[i]; } } - memcpy(output, passwd, len + 2); } static int do_next_tlv(VALUE_PAIR const *vp, VALUE_PAIR const *next, int nest) @@ -4270,14 +4274,15 @@ int rad_tunnel_pwencode(char *passwd, size_t *pwlen, char const *secret, if (len > 127) len = 127; /* - * Shift the password 3 positions right to place a salt and original - * length, tag will be added automatically on packet send + * Shift the password 3 positions right to place a salt and original + * length, tag will be added automatically on packet send. */ - for (n=len ; n>=0 ; n--) passwd[n+3] = passwd[n]; + for (n = len ; n >= 0 ; n--) passwd[n + 3] = passwd[n]; salt = passwd; passwd += 2; + /* - * save original password length as first password character; + * save original password length as first password character; */ *passwd = len; len += 1; @@ -4344,14 +4349,14 @@ int rad_tunnel_pwdecode(uint8_t *passwd, size_t *pwlen, char const *secret, FR_MD5_CTX context, old; uint8_t digest[AUTH_VECTOR_LEN]; int secretlen; - unsigned i, n, len, reallen; + size_t i, n, encrypted_len, reallen; - len = *pwlen; + encrypted_len = *pwlen; /* * We need at least a salt. */ - if (len < 2) { + if (encrypted_len < 2) { fr_strerror_printf("tunnel password is too short"); return -1; } @@ -4366,13 +4371,13 @@ int rad_tunnel_pwdecode(uint8_t *passwd, size_t *pwlen, char const *secret, * more data. So the 'data_len' field may be wrong, * but that's ok... */ - if (len <= 3) { + if (encrypted_len <= 3) { passwd[0] = 0; *pwlen = 0; return 0; } - len -= 2; /* discount the salt */ + encrypted_len -= 2; /* discount the salt */ /* * Use the secret to setup the decryption digest @@ -4392,10 +4397,20 @@ int rad_tunnel_pwdecode(uint8_t *passwd, size_t *pwlen, char const *secret, fr_md5_update(&context, passwd, 2); reallen = 0; - for (n = 0; n < len; n += AUTH_PASS_LEN) { - int base = 0; + for (n = 0; n < encrypted_len; n += AUTH_PASS_LEN) { + size_t base; + size_t block_len = AUTH_PASS_LEN; + + /* + * Ensure we don't overflow the input on MD5 + */ + if ((n + 2 + AUTH_PASS_LEN) > *pwlen) { + block_len = *pwlen - n - 2; + } if (n == 0) { + base = 1; + fr_md5_final(digest, &context); context = old; @@ -4406,31 +4421,27 @@ int rad_tunnel_pwdecode(uint8_t *passwd, size_t *pwlen, char const *secret, * 'data_len' field. Ensure it's sane. */ reallen = passwd[2] ^ digest[0]; - if (reallen >= len) { + if (reallen > encrypted_len) { fr_strerror_printf("tunnel password is too long for the attribute"); return -1; } - fr_md5_update(&context, passwd + 2, AUTH_PASS_LEN); + fr_md5_update(&context, passwd + 2, block_len); - base = 1; } else { + base = 0; + fr_md5_final(digest, &context); context = old; - fr_md5_update(&context, passwd + n + 2, AUTH_PASS_LEN); + fr_md5_update(&context, passwd + n + 2, block_len); } - for (i = base; i < AUTH_PASS_LEN; i++) { + for (i = base; i < block_len; i++) { passwd[n + i - 1] = passwd[n + i + 2] ^ digest[i]; } } - /* - * See make_tunnel_password, above. - */ - if (reallen > 239) reallen = 239; - *pwlen = reallen; passwd[reallen] = 0; diff --git a/src/tests/unit/tunnel.txt b/src/tests/unit/tunnel.txt index 6b8cc2533b7..da0cb314844 100644 --- a/src/tests/unit/tunnel.txt +++ b/src/tests/unit/tunnel.txt @@ -69,3 +69,19 @@ data Tunnel-Password:0 = "0123456789abcde" encode Tunnel-Password:0 = "0123456789abcdef" decode - data Tunnel-Password:0 = "0123456789abcdef" + +# +# We can't look at the data here, because the encoded Tunnel-Password has a 2 byte +# random salt +# +encode Tunnel-Password:0 := "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" +decode - +data Tunnel-Password:0 = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + +# +# 1 octet for the tag. 2 octets for salt. One octet for encrypted length. +# 249 octets left for real data. +# +encode Tunnel-Password:0 = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx123456789ab" +decode - +data Tunnel-Password:0 = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx123456789"