Skip to content

Commit 5e676b3

Browse files
committed
fix aci bypass, mem leak, url validation
1 parent 4c8b148 commit 5e676b3

4 files changed

Lines changed: 67 additions & 36 deletions

File tree

ldap/servers/slapd/hibp_client.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,7 @@ hibp_query_api(const char *prefix, const char *api_url, HIBPResponse *response,
358358
if (curl_easy_setopt(curl, CURLOPT_URL, url) != CURLE_OK ||
359359
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, hibp_write_callback) != CURLE_OK ||
360360
curl_easy_setopt(curl, CURLOPT_WRITEDATA, response) != CURLE_OK ||
361+
curl_easy_setopt(curl, CURLOPT_PROTOCOLS, CURLPROTO_HTTPS) != CURLE_OK ||
361362
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 1L) != CURLE_OK ||
362363
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 2L) != CURLE_OK) {
363364
slapi_log_err(SLAPI_LOG_ERR, "hibp_query_api",

ldap/servers/slapd/libglobs.c

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3829,9 +3829,14 @@ config_set_pw_breach_check(const char *attrname, char *value, char *errorbuf, in
38293829
slapdFrontendConfig_t *slapdFrontendConfig = getFrontendConfig();
38303830

38313831
#ifndef ENABLE_HIBP
3832-
if (apply && value && strcasecmp(value, "on") == 0) {
3833-
slapi_log_err(SLAPI_LOG_WARNING, "config_set_pw_breach_check",
3834-
"HIBP breached password checking not enabled - passwordBreachCheck has no effect\n");
3832+
if (value && strcasecmp(value, "on") == 0) {
3833+
slapi_create_errormsg(errorbuf, SLAPI_DSE_RETURNTEXT_SIZE,
3834+
"%s: HIBP breached password checking is not available. "
3835+
"Rebuild with --enable-hibp to enable this feature.", attrname);
3836+
slapi_log_err(SLAPI_LOG_ERR, "config_set_pw_breach_check",
3837+
"HIBP breached password checking is not available - "
3838+
"rebuild with --enable-hibp to enable this feature\n");
3839+
return LDAP_UNWILLING_TO_PERFORM;
38353840
}
38363841
#endif
38373842

@@ -3849,18 +3854,37 @@ config_set_pw_breach_url(const char *attrname, char *value, char *errorbuf, int
38493854
{
38503855
int32_t retVal = LDAP_SUCCESS;
38513856
slapdFrontendConfig_t *slapdFrontendConfig = getFrontendConfig();
3857+
size_t len;
38523858

38533859
#ifndef ENABLE_HIBP
38543860
if (apply && value && strlen(value) > 0) {
38553861
slapi_log_err(SLAPI_LOG_WARNING, "config_set_pw_breach_url",
3856-
"HIBP breached password checking not enabled - passwordBreachDbUrl has no effect\n");
3862+
"HIBP breached password checking not enabled - passwordBreachDbUrl has no effect\n");
38573863
}
38583864
#endif
38593865

38603866
if (config_value_is_null(attrname, value, errorbuf, 0)) {
38613867
value = NULL;
38623868
}
38633869

3870+
/* Validate URL if provided */
3871+
if (value && strlen(value) > 0) {
3872+
/* Require https:// endpoint for security */
3873+
if (strncasecmp(value, "https://", 8) != 0) {
3874+
slapi_create_errormsg(errorbuf, SLAPI_DSE_RETURNTEXT_SIZE,
3875+
"%s: URL must use https://", attrname);
3876+
return LDAP_UNWILLING_TO_PERFORM;
3877+
}
3878+
/* Require trailing slash for correct URL construction */
3879+
len = strlen(value);
3880+
if (value[len - 1] != '/') {
3881+
slapi_create_errormsg(errorbuf, SLAPI_DSE_RETURNTEXT_SIZE,
3882+
"%s: URL must end with a trailing slash (e.g., https://api.pwnedpasswords.com/range/)",
3883+
attrname);
3884+
return LDAP_UNWILLING_TO_PERFORM;
3885+
}
3886+
}
3887+
38643888
if (apply) {
38653889
CFG_LOCK_WRITE(slapdFrontendConfig);
38663890
slapi_ch_free_string(&slapdFrontendConfig->pw_policy.pw_breach_db_url);

ldap/servers/slapd/modify.c

Lines changed: 37 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,38 +1306,6 @@ op_shared_allow_pw_change(Slapi_PBlock *pb, LDAPMod *mod, char **old_pw, Slapi_M
13061306
pwpolicy = new_passwdPolicy(pb, (char *)slapi_sdn_get_ndn(&sdn));
13071307
internal_op = operation_is_flag_set(operation, OP_FLAG_INTERNAL);
13081308

1309-
#ifdef ENABLE_HIBP
1310-
/* Check all passwords against breach database before any other checks */
1311-
if (pwpolicy->pw_check_breach && mod->mod_bvalues) {
1312-
Slapi_Value **breach_vals = NULL;
1313-
valuearray_init_bervalarray(mod->mod_bvalues, &breach_vals);
1314-
if (breach_vals) {
1315-
for (size_t i = 0; breach_vals[i] != NULL; i++) {
1316-
const char *pwd = slapi_value_get_string(breach_vals[i]);
1317-
if (pwd && !slapi_is_encoded((char *)pwd)) {
1318-
int breach_count = hibp_check_password(pwd, pwpolicy);
1319-
if (breach_count > 0) {
1320-
slapi_log_err(SLAPI_LOG_WARNING, "op_shared_allow_pw_change",
1321-
"Rejecting password for %s - found in breach database (%d occurrences)\n",
1322-
dn, breach_count);
1323-
if (pwresponse_req == 1) {
1324-
slapi_pwpolicy_make_response_control(pb, -1, -1, LDAP_PWPOLICY_INVALIDPWDSYNTAX);
1325-
}
1326-
send_ldap_result(pb, LDAP_CONSTRAINT_VIOLATION, NULL,
1327-
"Password found in breach database - choose a different password", 0, NULL);
1328-
valuearray_free(&breach_vals);
1329-
rc = -1;
1330-
goto done;
1331-
} else if (breach_count < 0) {
1332-
slapi_log_err(SLAPI_LOG_WARNING, "op_shared_allow_pw_change",
1333-
"Failed to check password against breach database for %s\n", dn);
1334-
}
1335-
}
1336-
}
1337-
valuearray_free(&breach_vals);
1338-
}
1339-
}
1340-
#endif
13411309
/* internal operation has root permissions for subtrees it is allowed to access */
13421310
if (!internal_op) {
13431311
/* slapi_acl_check_mods needs an array of LDAPMods, but
@@ -1392,6 +1360,43 @@ op_shared_allow_pw_change(Slapi_PBlock *pb, LDAPMod *mod, char **old_pw, Slapi_M
13921360
/* done with slapi entry e */
13931361
slapi_search_get_entry_done(&entry_pb);
13941362

1363+
#ifdef ENABLE_HIBP
1364+
/*
1365+
* Check password against breach database after ACI validation.
1366+
*/
1367+
if (!SLAPI_IS_MOD_DELETE(mod->mod_op) &&
1368+
!pw_is_pwp_admin(pb, pwpolicy, PWP_ADMIN_OR_ROOTDN) &&
1369+
pwpolicy->pw_check_breach && mod->mod_bvalues) {
1370+
Slapi_Value **breach_vals = NULL;
1371+
valuearray_init_bervalarray(mod->mod_bvalues, &breach_vals);
1372+
if (breach_vals) {
1373+
for (size_t i = 0; breach_vals[i] != NULL; i++) {
1374+
const char *pwd = slapi_value_get_string(breach_vals[i]);
1375+
if (pwd && !slapi_is_encoded((char *)pwd)) {
1376+
int breach_count = hibp_check_password(pwd, pwpolicy);
1377+
if (breach_count > 0) {
1378+
slapi_log_err(SLAPI_LOG_PWDPOLICY, PWDPOLICY_DEBUG,
1379+
"Rejecting password for %s - found in breach database (%d occurrences)\n",
1380+
dn, breach_count);
1381+
if (pwresponse_req == 1) {
1382+
slapi_pwpolicy_make_response_control(pb, -1, -1, LDAP_PWPOLICY_INVALIDPWDSYNTAX);
1383+
}
1384+
send_ldap_result(pb, LDAP_CONSTRAINT_VIOLATION, NULL,
1385+
"Password found in breach database - choose a different password", 0, NULL);
1386+
valuearray_free(&breach_vals);
1387+
rc = -1;
1388+
goto done;
1389+
} else if (breach_count < 0) {
1390+
slapi_log_err(SLAPI_LOG_WARNING, "op_shared_allow_pw_change",
1391+
"Failed to check password against breach database for %s\n", dn);
1392+
}
1393+
}
1394+
}
1395+
valuearray_free(&breach_vals);
1396+
}
1397+
}
1398+
#endif
1399+
13951400
/*
13961401
* If this mod is being performed by a password administrator/rootDN,
13971402
* just return success.

ldap/servers/slapd/pw.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2513,6 +2513,7 @@ delete_passwdPolicy(passwdPolicy **pwpolicy)
25132513
slapi_ch_free_string(&(*(*pwpolicy)).pw_bad_words);
25142514
slapi_ch_array_free((*(*pwpolicy)).pw_cmp_attrs_array);
25152515
slapi_ch_free_string(&(*(*pwpolicy)).pw_cmp_attrs);
2516+
slapi_ch_free_string(&(*(*pwpolicy)).pw_breach_db_url);
25162517
}
25172518
slapi_ch_free_string(&(*(*pwpolicy)).pw_local_dn);
25182519
slapi_ch_free((void **)pwpolicy);

0 commit comments

Comments
 (0)