Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Ensure that ipa-otpd bind authentications validate an OTP
Before this patch, if the user was configured for either OTP or password
it was possible to do a 1FA authentication through ipa-otpd if the user
was configured for optional 2FA. Because this correctly respected the
configuration, it is not a security error.

However, once we begin to insert authentication indicators into the
Kerberos tickets, we cannot allow 1FA authentications through this
code path. Otherwise the ticket would contain a 2FA indicator when
only 1FA was actually performed.

To solve this problem, we have ipa-otpd send a critical control during
the bind operation which informs the LDAP server that it *MUST* validate
an OTP token for authentication to be successful. Next, we implement
support for this control in the ipa-pwd-extop plugin. The end result is
that the bind operation will always fail if the control is present and
no OTP is validated.
  • Loading branch information
npmccallum committed Feb 23, 2016
1 parent d1252cf commit a78191e
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 20 deletions.
5 changes: 4 additions & 1 deletion daemons/ipa-otpd/bind.c
Expand Up @@ -26,9 +26,12 @@
*/

#include "internal.h"
#include "../ipa-slapi-plugins/ipa-pwd-extop/otpctrl.h"

static void on_bind_writable(verto_ctx *vctx, verto_ev *ev)
{
LDAPControl control = { OTP_REQUIRED_OID, {}, true };
LDAPControl *ctrls[] = { &control, NULL };
struct otpd_queue *push = &ctx.stdio.responses;
const krb5_data *data;
struct berval cred;
Expand All @@ -55,7 +58,7 @@ static void on_bind_writable(verto_ctx *vctx, verto_ev *ev)
cred.bv_val = data->data;
cred.bv_len = data->length;
i = ldap_sasl_bind(verto_get_private(ev), item->user.dn, LDAP_SASL_SIMPLE,
&cred, NULL, NULL, &item->msgid);
&cred, ctrls, NULL, &item->msgid);
if (i != LDAP_SUCCESS) {
otpd_log_err(errno, "Unable to initiate bind: %s", ldap_err2string(i));
verto_break(ctx.vctx);
Expand Down
2 changes: 1 addition & 1 deletion daemons/ipa-slapi-plugins/ipa-pwd-extop/Makefile.am
Expand Up @@ -47,7 +47,7 @@ libipa_pwd_extop_la_SOURCES = \
encoding.c \
prepost.c \
ipa_pwd_extop.c \
syncreq.c \
otpctrl.c \
$(KRB5_UTIL_SRCS) \
$(NULL)

Expand Down
Expand Up @@ -38,19 +38,19 @@
* END COPYRIGHT BLOCK **/

#include "../libotp/otp_token.h"
#include "syncreq.h"
#include "otpctrl.h"

bool sync_request_present(Slapi_PBlock *pb)
bool otpctrl_present(Slapi_PBlock *pb, const char *oid)
{
LDAPControl **controls = NULL;

if (slapi_pblock_get(pb, SLAPI_REQCONTROLS, &controls) != 0)
return false;

return ldap_control_find(OTP_SYNC_REQUEST_OID, controls, NULL) != NULL;
return ldap_control_find(oid, controls, NULL) != NULL;
}

bool sync_request_handle(const struct otp_config *cfg, Slapi_PBlock *pb,
bool otpctrl_sync_handle(const struct otp_config *cfg, Slapi_PBlock *pb,
const char *user_dn)
{
struct otp_token **tokens = NULL;
Expand Down
Expand Up @@ -37,9 +37,7 @@
* All rights reserved.
* END COPYRIGHT BLOCK **/


#ifndef SYNCREQ_H_
#define SYNCREQ_H_
#pragma once

#include "../libotp/otp_config.h"
#include <stdbool.h>
Expand All @@ -55,9 +53,10 @@
*/
#define OTP_SYNC_REQUEST_OID "2.16.840.1.113730.3.8.10.6"

bool sync_request_present(Slapi_PBlock *pb);
/* This control has no data. */
#define OTP_REQUIRED_OID "2.16.840.1.113730.3.8.10.7"

bool sync_request_handle(const struct otp_config *cfg, Slapi_PBlock *pb,
const char *user_dn);
bool otpctrl_present(Slapi_PBlock *pb, const char *oid);

#endif /* SYNCREQ_H_ */
bool otpctrl_sync_handle(const struct otp_config *cfg, Slapi_PBlock *pb,
const char *user_dn);
20 changes: 13 additions & 7 deletions daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
Expand Up @@ -62,7 +62,7 @@

#include "ipapwd.h"
#include "util.h"
#include "syncreq.h"
#include "otpctrl.h"

#define IPAPWD_OP_NULL 0
#define IPAPWD_OP_ADD 1
Expand Down Expand Up @@ -1171,7 +1171,9 @@ static int ipapwd_post_modadd(Slapi_PBlock *pb)
* value at the end. This leaves only the password in creds for later
* validation.
*/
static bool ipapwd_pre_bind_otp(const char *bind_dn, Slapi_Entry *entry,
static bool ipapwd_pre_bind_otp(Slapi_PBlock *pb,
Slapi_Entry *entry,
const char *bind_dn,
struct berval *creds)
{
uint32_t auth_types;
Expand Down Expand Up @@ -1207,7 +1209,7 @@ static bool ipapwd_pre_bind_otp(const char *bind_dn, Slapi_Entry *entry,
/* If the user has no active tokens, succeed. */
if (tokens[0] == NULL) {
otp_token_free_array(tokens);
return true;
return !otpctrl_present(pb, OTP_REQUIRED_OID);
}

if (otp_token_validate_berval(tokens, creds, NULL)) {
Expand All @@ -1218,7 +1220,10 @@ static bool ipapwd_pre_bind_otp(const char *bind_dn, Slapi_Entry *entry,
otp_token_free_array(tokens);
}

return auth_types & OTP_CONFIG_AUTH_TYPE_PASSWORD;
if (auth_types & OTP_CONFIG_AUTH_TYPE_PASSWORD)
return !otpctrl_present(pb, OTP_REQUIRED_OID);

return false;
}

static int ipapwd_authenticate(const char *dn, Slapi_Entry *entry,
Expand Down Expand Up @@ -1450,8 +1455,8 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
}

/* Try to do OTP first. */
syncreq = sync_request_present(pb);
if (!syncreq && !ipapwd_pre_bind_otp(dn, entry, credentials))
syncreq = otpctrl_present(pb, OTP_SYNC_REQUEST_OID);
if (!syncreq && !ipapwd_pre_bind_otp(pb, entry, dn, credentials))
goto invalid_creds;

/* Ensure that there is a password. */
Expand All @@ -1466,7 +1471,7 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
}

/* Attempt to handle a token synchronization request. */
if (syncreq && !sync_request_handle(otp_config, pb, dn))
if (syncreq && !otpctrl_sync_handle(otp_config, pb, dn))
goto invalid_creds;

/* Attempt to write out kerberos keys for the user. */
Expand All @@ -1488,6 +1493,7 @@ int ipapwd_pre_init(Slapi_PBlock *pb)
int ret;

slapi_register_supported_control(OTP_SYNC_REQUEST_OID, SLAPI_OPERATION_BIND);
slapi_register_supported_control(OTP_REQUIRED_OID, SLAPI_OPERATION_BIND);

ret = slapi_pblock_set(pb, SLAPI_PLUGIN_VERSION, SLAPI_PLUGIN_VERSION_01);
if (!ret) ret = slapi_pblock_set(pb, SLAPI_PLUGIN_DESCRIPTION, (void *)&ipapwd_plugin_desc);
Expand Down

0 comments on commit a78191e

Please sign in to comment.