Hi, while testing LDAP authentication with FreeIPA, I noticed PAP complained about no "known good" password found for the user. After looking into it, I saw that the Password.With-Header attribute has an unknown header. PAP rewrote it to Cleartext and obviously failed to verify it. Below is a patch to make authentication work. The additional password type was necessary because password_process_header in password.c always strips the header it finds, which sucks for OpenLDAP and 389ds passwords because they encode the hash algorithm there. I would really appreciate it if somebody could provide some feedback. Since it's such a massive codebase, chances are that I'm doing it (very) wrong. Thanks, Oliver --- .../dictionary.freeradius.internal.password | 2 + src/lib/server/password.c | 18 ++++++- src/modules/rlm_pap/rlm_pap.c | 49 +++++++++++++++++++ src/tests/modules/pap/pbkdf2_389ds_sha1.attrs | 11 +++++ .../modules/pap/pbkdf2_389ds_sha1.unlang | 17 +++++++ .../modules/pap/pbkdf2_389ds_sha256.attrs | 11 +++++ .../modules/pap/pbkdf2_389ds_sha256.unlang | 17 +++++++ .../modules/pap/pbkdf2_389ds_sha512.attrs | 11 +++++ .../modules/pap/pbkdf2_389ds_sha512.unlang | 17 +++++++ 9 files changed, 151 insertions(+), 2 deletions(-) create mode 100644 src/tests/modules/pap/pbkdf2_389ds_sha1.attrs create mode 100644 src/tests/modules/pap/pbkdf2_389ds_sha1.unlang create mode 100644 src/tests/modules/pap/pbkdf2_389ds_sha256.attrs create mode 100644 src/tests/modules/pap/pbkdf2_389ds_sha256.unlang create mode 100644 src/tests/modules/pap/pbkdf2_389ds_sha512.attrs create mode 100644 src/tests/modules/pap/pbkdf2_389ds_sha512.unlang diff --git a/share/dictionary/freeradius/dictionary.freeradius.internal.password b/share/dictionary/freeradius/dictionary.freeradius.internal.password index 2d184710ae..4e9ed86539 100644 --- a/share/dictionary/freeradius/dictionary.freeradius.internal.password +++ b/share/dictionary/freeradius/dictionary.freeradius.internal.password @@ -64,6 +64,8 @@ ATTRIBUTE SSHA3-256 28 octets ATTRIBUTE SSHA3-384 29 octets ATTRIBUTE SSHA3-512 30 octets +ATTRIBUTE PBKDF2-389DS 31 octets + END-TLV Password # TOTP passwords and secrets diff --git a/src/lib/server/password.c b/src/lib/server/password.c index 669c815efe..fe42a76890 100644 --- a/src/lib/server/password.c +++ b/src/lib/server/password.c @@ -73,6 +73,8 @@ typedef struct { ///< attribute using the hex/base64 decoders. bool always_allow; //!< Always allow processing of this attribute, irrespective ///< of what the caller says. + bool retain_header; //!< Do not strip header from password string before + ///< passing it to the authentication module } password_info_t; static fr_dict_t const *dict_freeradius = NULL; @@ -112,6 +114,7 @@ static fr_dict_attr_t const *attr_ssha3_384; static fr_dict_attr_t const *attr_ssha3_512; static fr_dict_attr_t const *attr_pbkdf2; +static fr_dict_attr_t const *attr_pbkdf2_389ds; static fr_dict_attr_t const *attr_lm; static fr_dict_attr_t const *attr_nt; static fr_dict_attr_t const *attr_ns_mta_md5; @@ -160,6 +163,7 @@ fr_dict_attr_autoload_t password_dict_attr[] = { { .out = &attr_ssha3_512, .name = "Password.SSHA3-512", .type = FR_TYPE_OCTETS, .dict = &dict_freeradius }, { .out = &attr_pbkdf2, .name = "Password.PBKDF2", .type = FR_TYPE_OCTETS, .dict = &dict_freeradius }, + { .out = &attr_pbkdf2_389ds, .name = "Password.PBKDF2-389DS", .type = FR_TYPE_OCTETS, .dict = &dict_freeradius }, { .out = &attr_lm, .name = "Password.LM", .type = FR_TYPE_OCTETS, .dict = &dict_freeradius }, { .out = &attr_nt, .name = "Password.NT", .type = FR_TYPE_OCTETS, .dict = &dict_freeradius }, { .out = &attr_ns_mta_md5, .name = "Password.NS-MTA-MD5", .type = FR_TYPE_STRING, .dict = &dict_freeradius }, @@ -205,6 +209,9 @@ static fr_table_num_sorted_t const password_header_table[] = { { L("{ns-mta-md5}"), FR_NS_MTA_MD5 }, { L("{nt}"), FR_NT }, { L("{nthash}"), FR_NT }, + { L("{pbkdf2-sha1}"), FR_PBKDF2_389DS }, + { L("{pbkdf2-sha256}"), FR_PBKDF2_389DS }, + { L("{pbkdf2-sha512}"), FR_PBKDF2_389DS }, #ifdef HAVE_OPENSSL_EVP_H { L("{sha224}"), FR_SHA2 }, @@ -285,6 +292,11 @@ static password_info_t password_info[] = { .type = PASSWORD_HASH_VARIABLE, .da = &attr_pbkdf2 }, + [FR_PBKDF2_389DS] = { + .type = PASSWORD_HASH_VARIABLE, + .da = &attr_pbkdf2_389ds, + .retain_header = true + }, [FR_SHA1] = { .type = PASSWORD_HASH, .da = &attr_sha1, @@ -681,11 +693,13 @@ do_header: goto bad_header; } - p = q + 1; - if (!fr_cond_assert(known_good->da->attr < NUM_ELEMENTS(password_info))) return NULL; info = &password_info[attr]; + if (!info->retain_header) { + p = q + 1; + } + MEM(new = fr_pair_afrom_da(ctx, *(info->da))); switch ((*(info->da))->type) { case FR_TYPE_OCTETS: diff --git a/src/modules/rlm_pap/rlm_pap.c b/src/modules/rlm_pap/rlm_pap.c index 67a363fb7d..216c68ddf5 100644 --- a/src/modules/rlm_pap/rlm_pap.c +++ b/src/modules/rlm_pap/rlm_pap.c @@ -682,6 +682,54 @@ finish: RETURN_MODULE_RCODE(rcode); } +static inline unlang_action_t CC_HINT(nonnull) pap_auth_pbkdf2_389ds(rlm_rcode_t *p_result, + UNUSED rlm_pap_t const *inst, + request_t *request, + fr_pair_t const *known_good, + fr_value_box_t const *password) +{ + uint8_t const *p = known_good->vp_octets, *q, *end = p + known_good->vp_length; + + /* + * OpenLDAP and 389ds carry algorithm information in + * the userPassword header, so make sure it's there. + * + * Message format: + * OpenLDAP: {PBKDF2-<digest>}<rounds>$<alt_b64_salt>$<alt_b64_hash> + * 389ds: {PBKDF2-<digest>}<rounds>$<b64_salt>$<b64_hash> + * + * The format is almost identical to Python's passlib. + * If we advance the pointer a bit and use '}' as scheme separator, + * pap_auth_pbkdf2_parse will do the rest for us + */ + + if (end - p < 2) { + REDEBUG("Password.PBKDF2-389DS too short"); + RETURN_MODULE_INVALID; + } + + if (*p != '{') { + REDEBUG("Password.PBKDF2-389DS has invalid header"); + RETURN_MODULE_INVALID; + } + + q = memchr(p, '}', end - p); + if (!q || q - p < 2) { + REDEBUG("Password.PBKDF2-389DS has invalid header"); + RETURN_MODULE_INVALID; + } + + if ((size_t)(q - p) > sizeof("{PBKDF2-") && strncasecmp((char const *) p, "{PBKDF2-", 8) == 0) { + p += sizeof("{PBKDF2-") - 1; + return pap_auth_pbkdf2_parse(p_result, request, p, end - p, + pbkdf2_passlib_names, pbkdf2_passlib_names_len, + '}', '$', '$', false, password); + } + + REDEBUG("Can't determine format of Password.PBKDF2-389DS"); + RETURN_MODULE_INVALID; +} + static inline unlang_action_t CC_HINT(nonnull) pap_auth_pbkdf2(rlm_rcode_t *p_result, UNUSED rlm_pap_t const *inst, request_t *request, @@ -870,6 +918,7 @@ static const pap_auth_func_t auth_func_table[] = { #ifdef HAVE_OPENSSL_EVP_H [FR_PBKDF2] = pap_auth_pbkdf2, + [FR_PBKDF2_389DS] = pap_auth_pbkdf2_389ds, [FR_SHA2] = pap_auth_dummy, [FR_SHA2_224] = pap_auth_sha2_224, [FR_SHA2_256] = pap_auth_sha2_256, diff --git a/src/tests/modules/pap/pbkdf2_389ds_sha1.attrs b/src/tests/modules/pap/pbkdf2_389ds_sha1.attrs new file mode 100644 index 0000000000..74119f6a78 --- /dev/null +++ b/src/tests/modules/pap/pbkdf2_389ds_sha1.attrs @@ -0,0 +1,11 @@ +# +# Input packet +# +Packet-Type = Access-Request +User-Name = 'pbkdf2_389ds_sha1' +User-Password = 'password' + +# +# Expected answer +# +Packet-Type == Access-Accept diff --git a/src/tests/modules/pap/pbkdf2_389ds_sha1.unlang b/src/tests/modules/pap/pbkdf2_389ds_sha1.unlang new file mode 100644 index 0000000000..22fd74c7bc --- /dev/null +++ b/src/tests/modules/pap/pbkdf2_389ds_sha1.unlang @@ -0,0 +1,17 @@ +if ("${feature.tls}" == no) { + test_pass + return +} + +if (&User-Name == 'pbkdf2_389ds_sha1') { + &control.Password.With-Header := '{PBKDF2-SHA1}10000$kX7ZFznyliHoBZ+7ZIIAI1yH+V9yZg6j$35n7xsdaw763OdDgKGSMsJUWz8s=' + + pap.authorize + pap.authenticate + + if (ok) { + test_pass + } else { + test_fail + } +} diff --git a/src/tests/modules/pap/pbkdf2_389ds_sha256.attrs b/src/tests/modules/pap/pbkdf2_389ds_sha256.attrs new file mode 100644 index 0000000000..f1a566a94c --- /dev/null +++ b/src/tests/modules/pap/pbkdf2_389ds_sha256.attrs @@ -0,0 +1,11 @@ +# +# Input packet +# +Packet-Type = Access-Request +User-Name = 'pbkdf2_389ds_sha256' +User-Password = 'password' + +# +# Expected answer +# +Packet-Type == Access-Accept diff --git a/src/tests/modules/pap/pbkdf2_389ds_sha256.unlang b/src/tests/modules/pap/pbkdf2_389ds_sha256.unlang new file mode 100644 index 0000000000..b9cf455ad9 --- /dev/null +++ b/src/tests/modules/pap/pbkdf2_389ds_sha256.unlang @@ -0,0 +1,17 @@ +if ("${feature.tls}" == no) { + test_pass + return +} + +if (&User-Name == 'pbkdf2_389ds_sha256') { + &control.Password.With-Header := '{PBKDF2-SHA256}10000$kX7ZFznyliHoBZ+7ZIIAI1yH+V9yZg6j$kH2xGFdC+rS5+tjPnTXj/pGK0eFXTez2QqF2QZhunBM=' + + pap.authorize + pap.authenticate + + if (ok) { + test_pass + } else { + test_fail + } +} diff --git a/src/tests/modules/pap/pbkdf2_389ds_sha512.attrs b/src/tests/modules/pap/pbkdf2_389ds_sha512.attrs new file mode 100644 index 0000000000..15cbd64dc3 --- /dev/null +++ b/src/tests/modules/pap/pbkdf2_389ds_sha512.attrs @@ -0,0 +1,11 @@ +# +# Input packet +# +Packet-Type = Access-Request +User-Name = 'pbkdf2_389ds_sha512' +User-Password = 'password' + +# +# Expected answer +# +Packet-Type == Access-Accept diff --git a/src/tests/modules/pap/pbkdf2_389ds_sha512.unlang b/src/tests/modules/pap/pbkdf2_389ds_sha512.unlang new file mode 100644 index 0000000000..9444d690ea --- /dev/null +++ b/src/tests/modules/pap/pbkdf2_389ds_sha512.unlang @@ -0,0 +1,17 @@ +if ("${feature.tls}" == no) { + test_pass + return +} + +if (&User-Name == 'pbkdf2_389ds_sha512') { + &control.Password.With-Header := '{PBKDF2-SHA512}10000$kX7ZFznyliHoBZ+7ZIIAI1yH+V9yZg6j$kyzz433C/pdvdkzPTjwi8wr6cHNTFD0dCKuUkKVMmSgilGBDQjNxfDQwZzFBiPgChQJTFAm8oThm65xrLGFnAg==' + + pap.authorize + pap.authenticate + + if (ok) { + test_pass + } else { + test_fail + } +} --