keytabs: Modularize setkeytab operation

In preparation of adding another function to avoid code duplication.

Related:
https://fedorahosted.org/freeipa/ticket/3859

Reviewed-By: Nathaniel McCallum <npmccallum@redhat.com>

1 files changed, 611 insertions, 477 deletions

diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
index d8af39153..0b084a4d1 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
@@ -93,8 +93,8 @@ void *ipapwd_get_plugin_id(void)
     return ipapwd_plugin_id;
 }
 
-static int filter_keys(struct ipapwd_krbcfg *krbcfg,
-                       struct ipapwd_keyset *kset)
+static void filter_keys(struct ipapwd_krbcfg *krbcfg,
+                        struct ipapwd_keyset *kset)
 {
     int i, j;
 
@@ -123,8 +123,6 @@ static int filter_keys(struct ipapwd_krbcfg *krbcfg,
             i--;
         }
     }
-
-    return 0;
 }
 
 static int ipapwd_to_ldap_pwpolicy_error(int ipapwderr)
@@ -562,541 +560,686 @@ free_and_return:
 
 }
 
-/* Password Modify Extended operation plugin function */
-static int ipapwd_setkeytab(Slapi_PBlock *pb, struct ipapwd_krbcfg *krbcfg)
+static char *check_service_name(krb5_context krbctx, char *svc)
 {
-	char *bindDN = NULL;
-	char *serviceName = NULL;
-	char *errMesg = NULL;
-	int ret=0, rc=0, is_root=0;
-	struct berval *extop_value = NULL;
-	BerElement *ber = NULL;
-	Slapi_PBlock *pbte = NULL;
-	Slapi_Entry *targetEntry=NULL;
-	struct berval *bval = NULL;
-	Slapi_Value **svals = NULL;
-	Slapi_Value **evals = NULL;
-	const char *bdn;
-	const Slapi_DN *bsdn;
-	Slapi_DN *sdn;
-	Slapi_Backend *be;
-	Slapi_Entry **es = NULL;
-	int scope, res;
-	char *filter;
-	char *attrlist[] = {"krbPrincipalKey", "krbLastPwdChange", "userPassword", "krbPrincipalName", "enrolledBy", NULL };
-	krb5_context krbctx = NULL;
-	krb5_principal krbname = NULL;
-	krb5_error_code krberr;
-	int i, kvno;
-	Slapi_Mods *smods;
-	ber_tag_t rtag, ttmp;
-	ber_int_t tint;
-	ber_len_t tlen;
-	struct ipapwd_keyset *kset = NULL;
-	struct tm utctime;
-	char timestr[GENERALIZED_TIME_LENGTH+1];
-	time_t time_now = time(NULL);
-	char *pw = NULL;
-	Slapi_Value *objectclass;
+    krb5_principal krbname = NULL;
+    krb5_error_code krberr;
+    char *name = NULL;
 
-	svals = (Slapi_Value **)calloc(2, sizeof(Slapi_Value *));
-	if (!svals) {
-		LOG_OOM();
-		rc = LDAP_OPERATIONS_ERROR;
-		goto free_and_return;
-	}
+    krberr = krb5_parse_name(krbctx, svc, &krbname);
+    if (krberr) {
+        LOG_FATAL("krb5_parse_name failed\n");
+    } else {
+        /* invert so that we get the canonical form (add REALM if not present
+         * for example) */
+        krberr = krb5_unparse_name(krbctx, krbname, &name);
+        if (krberr) {
+            LOG_FATAL("krb5_unparse_name failed\n");
+        }
+    }
 
-	krberr = krb5_init_context(&krbctx);
-	if (krberr) {
-		LOG_FATAL("krb5_init_context failed\n");
-		rc = LDAP_OPERATIONS_ERROR;
-		goto free_and_return;
-	}
+    krb5_free_principal(krbctx, krbname);
+    return name;
+}
 
-	/* Get Bind DN */
-	slapi_pblock_get(pb, SLAPI_CONN_DN, &bindDN);
+static Slapi_Backend *get_realm_backend(void)
+{
+    Slapi_Backend *be;
+    Slapi_DN *sdn;
+
+    sdn = slapi_sdn_new_dn_byval(ipa_realm_dn);
+    if (!sdn) return NULL;
+    be = slapi_be_select(sdn);
+    slapi_sdn_free(&sdn);
+    return be;
+}
 
-	 /* If the connection is bound anonymously, we must refuse to process
-	 * this operation. */
-	if (bindDN == NULL || *bindDN == '\0') {
-	 	/* Refuse the operation because they're bound anonymously */
-		errMesg = "Anonymous Binds are not allowed.\n";
-		rc = LDAP_INSUFFICIENT_ACCESS;
-		goto free_and_return;
-	}
+static const char *get_realm_base_dn(void)
+{
+    const Slapi_DN *bsdn;
+    Slapi_Backend *be;
 
-	/* Get the ber value of the extended operation */
-	slapi_pblock_get(pb, SLAPI_EXT_OP_REQ_VALUE, &extop_value);
+    /* Find ancestor base DN */
+    be = get_realm_backend();
+    if (!be) return NULL;
 
-	if ((ber = ber_init(extop_value)) == NULL)
-	{
-		errMesg = "KeytabGet Request decode failed.\n";
-		rc = LDAP_PROTOCOL_ERROR;
-		goto free_and_return;
-	}
+    bsdn = slapi_be_getsuffix(be, 0);
+    if (!bsdn) return NULL;
 
-	/* Format of request to parse
-	 *
-	 * KeytabGetRequest ::= SEQUENCE {
-	 *     serviceIdentity OCTET STRING
-	 *     keys            SEQUENCE OF KrbKey,
-	 *     ...
-	 * }
-	 *
-	 * KrbKey ::= SEQUENCE {
-	 *     key       [0] EncryptionKey,
-	 *     salt      [1] KrbSalt OPTIONAL,
-	 *     s2kparams [2] OCTET STRING OPTIONAL,
-	 *     ...
-	 * }
-	 *
-	 * EncryptionKey ::= SEQUENCE {
-	 *     keytype   [0] Int32,
-	 *     keyvalue  [1] OCTET STRING
-	 * }
-	 *
-	 * KrbSalt ::= SEQUENCE {
-	 *     type      [0] Int32,
-	 *     salt      [1] OCTET STRING OPTIONAL
-	 * }
-	 */
+    return slapi_sdn_get_dn(bsdn);
+}
 
-	/* ber parse code */
-	rtag = ber_scanf(ber, "{a{", &serviceName);
-	if (rtag == LBER_ERROR) {
-		LOG_FATAL("ber_scanf failed\n");
-		errMesg = "Invalid payload, failed to decode.\n";
-		rc = LDAP_PROTOCOL_ERROR;
-		goto free_and_return;
-	}
+static Slapi_Entry *get_entry_by_principal(const char *principal)
+{
+    const char *bdn;
+    char *filter = NULL;
+    Slapi_PBlock *pb = NULL;
+    char *attrlist[] = { "krbPrincipalKey", "krbLastPwdChange",
+                         "userPassword", "krbPrincipalName",
+                         "enrolledBy", NULL };
+    Slapi_Entry **es = NULL;
+    int res, ret, i;
+    Slapi_Entry *entry = NULL;
+
+    /* Find ancestor base DN */
+    bdn = get_realm_base_dn();
+    if (!bdn) {
+        LOG_TRACE("Search for Base DN failed\n");
+        goto free_and_return;
+    }
 
-	/* make sure it is a valid name */
-	krberr = krb5_parse_name(krbctx, serviceName, &krbname);
-	if (krberr) {
-		slapi_ch_free_string(&serviceName);
-		LOG_FATAL("krb5_parse_name failed\n");
-		rc = LDAP_OPERATIONS_ERROR;
-		goto free_and_return;
-	} else {
-		/* invert so that we get the canonical form
-		 * (add REALM if not present for example) */
-		char *canonname;
-		krberr = krb5_unparse_name(krbctx, krbname, &canonname);
-		if (krberr) {
-			slapi_ch_free_string(&serviceName);
-			LOG_FATAL("krb5_unparse_name failed\n");
-			rc = LDAP_OPERATIONS_ERROR;
-			goto free_and_return;
-		}
-		slapi_ch_free_string(&serviceName);
-		serviceName = canonname;
-	}
+    filter = slapi_ch_smprintf("(krbPrincipalName=%s)", principal);
+    if (!filter) {
+        LOG_TRACE("Building filter failed\n");
+        goto free_and_return;
+    }
 
-	/* check entry before doing any other decoding */
+    pb = slapi_pblock_new();
+    slapi_search_internal_set_pb(pb, bdn, LDAP_SCOPE_SUBTREE, filter,
+                                 attrlist, 0,
+                                 NULL, /* Controls */ NULL, /* UniqueID */
+                                 ipapwd_plugin_id, 0); /* Flags */
+
+    /* do search the tree */
+    ret = slapi_search_internal_pb(pb);
+    slapi_pblock_get(pb, SLAPI_PLUGIN_INTOP_RESULT, &res);
+    if (ret == -1 || res != LDAP_SUCCESS) {
+        LOG_TRACE("Search for Principal failed, err (%d)\n", res ? res : ret);
+        goto free_and_return;
+    }
 
-	/* Find ancestor base DN */
-	sdn = slapi_sdn_new_dn_byval(ipa_realm_dn);
-	be = slapi_be_select(sdn);
-	slapi_sdn_free(&sdn);
-	bsdn = slapi_be_getsuffix(be, 0);
-	if (bsdn == NULL) {
-		LOG_TRACE("Search for Base DN failed\n");
-		errMesg = "PrincipalName not found (search for Base DN failed).\n";
-		rc = LDAP_NO_SUCH_OBJECT;
-		goto free_and_return;
-	}
-	bdn = slapi_sdn_get_dn(bsdn);
-	scope = LDAP_SCOPE_SUBTREE;
+    /* get entries */
+    slapi_pblock_get(pb, SLAPI_PLUGIN_INTOP_SEARCH_ENTRIES, &es);
+    if (!es) {
+        LOG_TRACE("No entries ?!");
+        goto free_and_return;
+    }
 
-	/* get Entry by krbPrincipalName */
-	filter = slapi_ch_smprintf("(krbPrincipalName=%s)", serviceName);
-
-	pbte = slapi_pblock_new();
-	slapi_search_internal_set_pb(pbte,
-		bdn, scope, filter, attrlist, 0,
-		NULL, /* Controls */
-		NULL, /* UniqueID */
-		ipapwd_plugin_id,
-		0); /* Flags */
-
-	/* do search the tree */
-	ret = slapi_search_internal_pb(pbte);
-	slapi_pblock_get(pbte, SLAPI_PLUGIN_INTOP_RESULT, &res);
-	if (ret == -1 || res != LDAP_SUCCESS) {
-		LOG_TRACE("Search for Principal failed, err (%d)\n",
-			  res ? res : ret);
-		errMesg = "PrincipalName not found (search failed).\n";
-		rc = LDAP_NO_SUCH_OBJECT;
-		goto free_and_return;
-	}
+    /* count entries */
+    for (i = 0; es[i]; i++) /* count */ ;
 
-	/* get entries */
-	slapi_pblock_get(pbte, SLAPI_PLUGIN_INTOP_SEARCH_ENTRIES, &es);
-	if (!es) {
-		LOG_TRACE("No entries ?!");
-		errMesg = "PrincipalName not found (no result returned).\n";
-		rc = LDAP_NO_SUCH_OBJECT;
-		goto free_and_return;
-	}
+    /* if there is none or more than one, freak out */
+    if (i != 1) {
+        LOG_TRACE("Too many entries, or entry no found (%d)", i);
+        goto free_and_return;
+    }
+    entry = slapi_entry_dup(es[0]);
 
-	/* count entries */
-	for (i = 0; es[i]; i++) /* count */ ;
+free_and_return:
+    if (pb) {
+        slapi_free_search_results_internal(pb);
+        slapi_pblock_destroy(pb);
+    }
+    if (filter) slapi_ch_free_string(&filter);
+    return entry;
+}
 
-	/* if there is none or more than one, freak out */
-	if (i != 1) {
-		LOG_TRACE("Too many entries, or entry no found (%d)", i);
-		if (i == 0)
-			errMesg = "PrincipalName not found.\n";
-		else
-			errMesg = "PrincipalName not found (too many entries).\n";
-		rc = LDAP_NO_SUCH_OBJECT;
-		goto free_and_return;
-	}
-	targetEntry = es[0];
+static bool is_allowed_to_access_attr(Slapi_PBlock *pb, char *bindDN,
+                                      Slapi_Entry *targetEntry,
+                                      const char *attrname,
+                                      struct berval *value,
+                                      int access)
+{
+    Slapi_Backend *be;
+    int is_root = 0;
+    int ret;
 
-	/* First thing to do is to ask access control if the bound identity has
-	 * rights to modify the userpassword attribute on this entry. If not,
-	 * then we fail immediately with insufficient access. This means that
-	 * we don't leak any useful information to the client such as current
-	 * password wrong, etc.
-	 */
+    is_root = slapi_dn_isroot(bindDN);
+    if (slapi_pblock_set(pb, SLAPI_REQUESTOR_ISROOT, &is_root)) {
+        LOG_FATAL("slapi_pblock_set failed!\n");
+        return false;
+    }
 
-	is_root = slapi_dn_isroot(bindDN);
-	if (slapi_pblock_set(pb, SLAPI_REQUESTOR_ISROOT, &is_root)) {
-		LOG_FATAL("slapi_pblock_set failed!\n");
-		rc = LDAP_OPERATIONS_ERROR;
-		goto free_and_return;
-	}
+    /* In order to perform the access control check, we need to select a
+     * backend (even though we don't actually need it otherwise).
+     */
+    be = get_realm_backend();
+    if (!be) {
+        LOG_FATAL("Could not fetch REALM backend!");
+        return false;
+    }
+    if (slapi_pblock_set(pb, SLAPI_BACKEND, be)) {
+        LOG_FATAL("slapi_pblock_set failed!\n");
+        return false;
+    }
 
-	/* In order to perform the access control check,
-	 * we need to select a backend (even though
-	 * we don't actually need it otherwise).
-	 */
-	if (slapi_pblock_set(pb, SLAPI_BACKEND, be)) {
-		LOG_FATAL("slapi_pblock_set failed!\n");
-		rc = LDAP_OPERATIONS_ERROR;
-		goto free_and_return;
-	}
+    ret = slapi_access_allowed(pb, targetEntry, discard_const(attrname),
+                               value, access);
+    if (ret != LDAP_SUCCESS) {
+        LOG_FATAL("slapi_access_allowed does not allow %s to %s%s!\n",
+                  (access == SLAPI_ACL_WRITE)?"WRITE":"READ",
+                  attrname, value?"(value specified)":"");
+        return false;
+    }
 
-	/* Access Strategy:
-	 * If the user has WRITE-ONLY access, a new keytab is set on the entry.
-	 */
+    return true;
+}
 
-	ret = slapi_access_allowed(pb, targetEntry, "krbPrincipalKey", NULL, SLAPI_ACL_WRITE);
-	if (ret != LDAP_SUCCESS) {
-		errMesg = "Insufficient access rights\n";
-		rc = LDAP_INSUFFICIENT_ACCESS;
-		goto free_and_return;
-	}
+static int set_krbLastPwdChange(Slapi_Mods *smods, time_t now)
+{
+    char tstr[GENERALIZED_TIME_LENGTH + 1];
+    struct tm utctime;
 
-	/* increment kvno (will be 1 if this is a new entry) */
-	kvno = ipapwd_get_cur_kvno(targetEntry) + 1;
+    /* change Last Password Change field with the current date */
+    if (!gmtime_r(&now, &utctime)) {
+        LOG_FATAL("failed to retrieve current date (buggy gmtime_r ?)\n");
+        return LDAP_OPERATIONS_ERROR;
+    }
+    strftime(tstr, GENERALIZED_TIME_LENGTH + 1, "%Y%m%d%H%M%SZ", &utctime);
+    slapi_mods_add_string(smods, LDAP_MOD_REPLACE, "krbLastPwdChange", tstr);
+    return LDAP_SUCCESS;
+}
+
+static void remove_user_password(Slapi_Mods *smods,
+                                 Slapi_Entry *targetEntry, char *bindDN)
+{
+    Slapi_Value *objectclass = NULL;
+    char *krbLastPwdChange = NULL;
+    char *enrolledBy = NULL;
+    char *pw = NULL;
+    int ret;
 
-	/* ok access allowed, init kset and continue to parse ber buffer */
+    objectclass = slapi_value_new_string("ipaHost");
+    pw = slapi_entry_attr_get_charptr(targetEntry, "userPassword");
+    ret = slapi_entry_attr_has_syntax_value(targetEntry,
+                                            SLAPI_ATTR_OBJECTCLASS,
+                                            objectclass);
+    if (ret == 1) {
+        krbLastPwdChange = slapi_entry_attr_get_charptr(targetEntry,
+                                                        "krbLastPwdChange");
+        enrolledBy = slapi_entry_attr_get_charptr(targetEntry, "enrolledBy");
+        if (!enrolledBy) {
+            slapi_mods_add_string(smods, LDAP_MOD_ADD, "enrolledBy", bindDN);
+        }
+        if ((NULL != pw) && (NULL == krbLastPwdChange)) {
+            slapi_mods_add_mod_values(smods, LDAP_MOD_DELETE,
+                                      "userPassword", NULL);
+            LOG_TRACE("Removing userPassword from host entry\n");
+        }
+    }
+    if (krbLastPwdChange) slapi_ch_free_string(&krbLastPwdChange);
+    if (enrolledBy) slapi_ch_free_string(&enrolledBy);
+    if (pw) slapi_ch_free_string(&pw);
+    if (objectclass) slapi_value_free(&objectclass);
+}
 
-	errMesg = "Unable to set key\n";
-	rc = LDAP_OPERATIONS_ERROR;
+static int store_new_keys(Slapi_Entry *target, char *svcname, char *bind_dn,
+                          Slapi_Value **svals, char **_err_msg)
+{
+    int rc = LDAP_OPERATIONS_ERROR;
+    char *err_msg = NULL;
+    Slapi_Mods *smods = NULL;
+    time_t time_now = time(NULL);
+
+    smods = slapi_mods_new();
+    slapi_mods_add_mod_values(smods, LDAP_MOD_REPLACE,
+                              "krbPrincipalKey", svals);
+    rc = set_krbLastPwdChange(smods, time_now);
+    if (rc) {
+        rc = LDAP_OPERATIONS_ERROR;
+        LOG_FATAL("Failed to set krbLastPwdChange");
+        err_msg = "Internal error while storing keytab data\n";
+        goto done;
+    }
 
-	kset = malloc(sizeof(struct ipapwd_keyset));
-	if (!kset) {
-		LOG_OOM();
-		rc = LDAP_OPERATIONS_ERROR;
-		goto free_and_return;
-	}
+    /* If we are creating a keytab for a host service, attempt to remove
+     * the userPassword attribute if it exists
+     */
+    remove_user_password(smods, target, bind_dn);
 
-	/* this encoding assumes all keys have the same kvno */
-	/* major-vno = 1 and minor-vno = 1 */
-	kset->major_vno = 1;
-	kset->minor_vno = 1;
-	kset->mkvno = krbcfg->mkvno;
-
-	kset->keys = NULL;
-	kset->num_keys = 0;
-
-	rtag = ber_peek_tag(ber, &tlen);
-	while (rtag == LBER_SEQUENCE) {
-		krb5_key_data *newset;
-		krb5_data plain;
-		krb5_enc_data cipher;
-		struct berval tval;
-		krb5_octet *kdata;
-                krb5_int16 t;
-		size_t klen;
-
-		i = kset->num_keys;
-
-		newset = realloc(kset->keys, sizeof(krb5_key_data) * (i + 1));
-		if (!newset) {
-			LOG_OOM();
-			goto free_and_return;
-		}
-		kset->keys = newset;
+    /* commit changes */
+    rc = ipapwd_apply_mods(slapi_entry_get_dn_const(target), smods);
+    if (rc != LDAP_SUCCESS) {
+        rc = LDAP_OPERATIONS_ERROR;
+        LOG_FATAL("Failed to apply mods");
+        err_msg = "Internal error while saving keys\n";
+        goto done;
+    }
 
-		kset->num_keys += 1;
+    rc = ipapwd_set_extradata(slapi_entry_get_dn_const(target),
+                              svcname, time_now);
+    if (rc != LDAP_SUCCESS) {
+        rc = LDAP_OPERATIONS_ERROR;
+        LOG_FATAL("Failed to set extradata");
+        err_msg = "Internal error while saving keytab extradata\n";
+        goto done;
+    }
 
-		memset(&kset->keys[i], 0, sizeof(krb5_key_data));
-		kset->keys[i].key_data_ver = 1;
-		kset->keys[i].key_data_kvno = kvno;
+    rc = LDAP_SUCCESS;
 
-		/* EncryptionKey */
-		rtag = ber_scanf(ber, "{t[{t[i]t[o]}]", &ttmp, &ttmp, &tint, &ttmp, &tval);
-		if (rtag == LBER_ERROR) {
-			LOG_FATAL("ber_scanf failed\n");
-			errMesg = "Invalid payload, failed to decode EncryptionKey.\n";
-			rc = LDAP_PROTOCOL_ERROR;
-			goto free_and_return;
-		}
+done:
+    if (smods) slapi_mods_free(&smods);
+    *_err_msg = err_msg;
+    return rc;
+}
 
-		kset->keys[i].key_data_type[0] = tint;
+/* Format of request to parse
+ *
+ * KeytabSetRequest ::= SEQUENCE {
+ *     serviceIdentity OCTET STRING
+ *     keys            SEQUENCE OF KrbKey,
+ *     ...
+ * }
+ *
+ * KrbKey ::= SEQUENCE {
+ *     key       [0] EncryptionKey,
+ *     salt      [1] KrbSalt OPTIONAL,
+ *     s2kparams [2] OCTET STRING OPTIONAL,
+ *     ...
+ * }
+ *
+ * EncryptionKey ::= SEQUENCE {
+ *     keytype   [0] Int32,
+ *     keyvalue  [1] OCTET STRING
+ * }
+ *
+ * KrbSalt ::= SEQUENCE {
+ *     type      [0] Int32,
+ *     salt      [1] OCTET STRING OPTIONAL
+ * }
+ */
 
-		plain.length = tval.bv_len;
-		plain.data = tval.bv_val;
+#define SKREQ_SALT_TAG (LBER_CLASS_CONTEXT | LBER_CONSTRUCTED | 1)
+#define SKREQ_SALTVALUE_TAG (LBER_CLASS_CONTEXT | LBER_CONSTRUCTED | 1)
+#define SKREQ_S2KPARAMS_TAG (LBER_CLASS_CONTEXT | LBER_CONSTRUCTED | 2)
+
+/* The returned krb5_key_data kvno is set to 0 for all keys, the caller,
+ * is responsible for fixing it up if necessary before using the data */
+static int decode_setkeytab_request(krb5_context krbctx,
+                                    krb5_keyblock *kmkey, int mkvno,
+                                    struct berval *extop, char **_svcname,
+                                    struct ipapwd_keyset **_kset,
+                                    char **_err_msg) {
+    int rc = LDAP_OPERATIONS_ERROR;
+    char *err_msg = NULL;
+    BerElement *ber = NULL;
+    char *svcname = NULL;
+    ber_tag_t rtag;
+    ber_len_t tlen;
+    struct ipapwd_keyset *kset = NULL;
+
+    ber = ber_init(extop);
+    if (ber == NULL) {
+        rc = LDAP_PROTOCOL_ERROR;
+        err_msg = "KeytabSet Request decode failed.\n";
+        goto done;
+    }
 
-		krberr = krb5_c_encrypt_length(krbctx, krbcfg->kmkey->enctype, plain.length, &klen);
-		if (krberr) {
-			free(tval.bv_val);
-			LOG_FATAL("krb encryption failed!\n");
-			goto free_and_return;
-		}
+    /* ber parse code */
+    rtag = ber_scanf(ber, "{a{", &svcname);
+    if (rtag == LBER_ERROR) {
+        rc = LDAP_PROTOCOL_ERROR;
+        LOG_FATAL("ber_scanf failed to fecth service name\n");
+        err_msg = "Invalid payload.\n";
+        goto done;
+    }
 
-		kdata = malloc(2 + klen);
-		if (!kdata) {
-			free(tval.bv_val);
-			LOG_OOM();
-			goto free_and_return;
-		}
-		t = htole16(plain.length);
-		memcpy(kdata, &t, 2);
+    kset = calloc(1, sizeof(struct ipapwd_keyset));
+    if (!kset) {
+        rc = LDAP_OPERATIONS_ERROR;
+        LOG_OOM();
+        err_msg = "Internal error.\n";
+        goto done;
+    }
 
-		kset->keys[i].key_data_length[0] = 2 + klen;
-		kset->keys[i].key_data_contents[0] = (krb5_octet *)kdata;
+    /* this encoding assumes all keys have the same kvno */
+    /* major-vno = 1 and minor-vno = 1 */
+    kset->major_vno = 1;
+    kset->minor_vno = 1;
+    kset->mkvno = mkvno;
+
+    rtag = ber_peek_tag(ber, &tlen);
+    for (int i = 0; rtag == LBER_SEQUENCE; i++) {
+        krb5_key_data *newset;
+        ber_tag_t ctag;
+        ber_int_t type;
+        krb5_data plain;
+        krb5_enc_data cipher;
+        struct berval tval;
+        krb5_octet *kdata;
+        krb5_int16 le_len;
+        size_t klen;
+
+        newset = realloc(kset->keys, sizeof(krb5_key_data) * (i + 1));
+        if (!newset) {
+            rc = LDAP_OPERATIONS_ERROR;
+            LOG_OOM();
+            err_msg = "Internal error.\n";
+            goto done;
+        }
+        kset->keys = newset;
+        kset->num_keys = i + 1;
+
+        memset(&kset->keys[i], 0, sizeof(krb5_key_data));
+        kset->keys[i].key_data_ver = 1;
+        kset->keys[i].key_data_kvno = 0;
+
+        /* EncryptionKey */
+        rtag = ber_scanf(ber, "{t[{t[i]t[o]}]",
+                         &ctag, &ctag, &type, &ctag, &tval);
+        if (rtag == LBER_ERROR) {
+            rc = LDAP_PROTOCOL_ERROR;
+            LOG_FATAL("ber_scanf failed fetching key\n");
+            err_msg = "Invalid payload.\n";
+            goto done;
+        }
 
-		cipher.ciphertext.length = klen;
-		cipher.ciphertext.data = (char *)kdata + 2;
+        kset->keys[i].key_data_type[0] = type;
+        plain.length = tval.bv_len;
+        plain.data = tval.bv_val;
 
-		krberr = krb5_c_encrypt(krbctx, krbcfg->kmkey, 0, 0, &plain, &cipher);
-		if (krberr) {
-			free(tval.bv_val);
-			LOG_FATAL("krb encryption failed!\n");
-			goto free_and_return;
-		}
+        rc = krb5_c_encrypt_length(krbctx, kmkey->enctype,
+                                   plain.length, &klen);
+        if (rc) {
+            ber_memfree(tval.bv_val);
+            rc = LDAP_OPERATIONS_ERROR;
+            LOG_FATAL("krb5_c_encrypt_length failed!\n");
+            err_msg = "Internal error.\n";
+            goto done;
+        }
+        kdata = malloc(2 + klen);
+        if (!kdata) {
+            ber_memfree(tval.bv_val);
+            rc = LDAP_OPERATIONS_ERROR;
+            LOG_OOM();
+            err_msg = "Internal error.\n";
+            goto done;
+        }
+        le_len = htole16(plain.length);
+        memcpy(kdata, &le_len, 2);
 
-		ber_memfree(tval.bv_val);
+        kset->keys[i].key_data_length[0] = 2 + klen;
+        kset->keys[i].key_data_contents[0] = kdata;
 
-		rtag = ber_peek_tag(ber, &tlen);
+        cipher.ciphertext.length = klen;
+        cipher.ciphertext.data = (char *)kdata + 2;
 
-		/* KrbSalt */
-		if (rtag == (ber_tag_t)(LBER_CONSTRUCTED | LBER_CLASS_CONTEXT | 1)) {
+        rc = krb5_c_encrypt(krbctx, kmkey, 0, 0, &plain, &cipher);
+        if (rc) {
+            ber_memfree(tval.bv_val);
+            rc = LDAP_OPERATIONS_ERROR;
+            LOG_FATAL("krb5_c_encrypt failed!\n");
+            err_msg = "Internal error.\n";
+            goto done;
+        }
 
-			rtag = ber_scanf(ber, "t[{t[i]", &ttmp, &ttmp, &tint);
-			if (rtag == LBER_ERROR) {
-				LOG_FATAL("ber_scanf failed\n");
-				errMesg = "Invalid payload, failed to decode KrbSalt type.\n";
-				rc = LDAP_PROTOCOL_ERROR;
-				goto free_and_return;
-			}
+        ber_memfree(tval.bv_val);
+
+        rtag = ber_peek_tag(ber, &tlen);
+        /* KrbSalt */
+        if (rtag == SKREQ_SALT_TAG) {
+            rtag = ber_scanf(ber, "t[{t[i]", &ctag, &ctag, &type);
+            if (rtag == LBER_ERROR) {
+                rc = LDAP_PROTOCOL_ERROR;
+                LOG_FATAL("ber_scanf failed fetching salt\n");
+                err_msg = "Invalid payload.\n";
+                goto done;
+            }
 
-			kset->keys[i].key_data_ver = 2; /* we have a salt */
-			kset->keys[i].key_data_type[1] = tint;
-
-			rtag = ber_peek_tag(ber, &tlen);
-			if (rtag == (ber_tag_t)(LBER_CONSTRUCTED | LBER_CLASS_CONTEXT | 1)) {
-
-				rtag = ber_scanf(ber, "t[o]}]", &ttmp, &tval);
-				if (rtag == LBER_ERROR) {
-					LOG_FATAL("ber_scanf failed\n");
-					errMesg = "Invalid payload, failed to decode KrbSalt contents.\n";
-					rc = LDAP_PROTOCOL_ERROR;
-					goto free_and_return;
-				}
-
-				kset->keys[i].key_data_length[1] = tval.bv_len;
-				kset->keys[i].key_data_contents[1] = malloc(tval.bv_len);
-				if (!kset->keys[i].key_data_contents[1]) {
-				    LOG_OOM();
-				    rc = LDAP_OPERATIONS_ERROR;
-				    goto free_and_return;
-				}
-				memcpy(kset->keys[i].key_data_contents[1],
-				       tval.bv_val, tval.bv_len);
-				ber_memfree(tval.bv_val);
-
-				rtag = ber_peek_tag(ber, &tlen);
-			}
-		}
+            kset->keys[i].key_data_ver = 2; /* we have a salt */
+            kset->keys[i].key_data_type[1] = type;
+
+            rtag = ber_peek_tag(ber, &tlen);
+            if (rtag == SKREQ_SALTVALUE_TAG) {
+                rtag = ber_scanf(ber, "t[o]}]", &ctag, &tval);
+                if (rtag == LBER_ERROR) {
+                    rc = LDAP_PROTOCOL_ERROR;
+                    LOG_FATAL("ber_scanf failed fetching salt value\n");
+                    err_msg = "Invalid payload.\n";
+                    goto done;
+                }
+
+                kset->keys[i].key_data_length[1] = tval.bv_len;
+                kset->keys[i].key_data_contents[1] = malloc(tval.bv_len);
+                if (!kset->keys[i].key_data_contents[1]) {
+                    ber_memfree(tval.bv_val);
+                    rc = LDAP_OPERATIONS_ERROR;
+                    LOG_OOM();
+                    err_msg = "Internal error.\n";
+                    goto done;
+                }
+                memcpy(kset->keys[i].key_data_contents[1],
+                       tval.bv_val, tval.bv_len);
+                ber_memfree(tval.bv_val);
+
+                rtag = ber_peek_tag(ber, &tlen);
+            }
+        }
 
-		/* FIXME: s2kparams - NOT implemented yet */
-		if (rtag == (ber_tag_t)(LBER_CONSTRUCTED | LBER_CLASS_CONTEXT | 2)) {
-			rtag = ber_scanf(ber, "t[x]}", &ttmp);
-		} else {
-			rtag = ber_scanf(ber, "}", &ttmp);
-		}
-		if (rtag == LBER_ERROR) {
-			LOG_FATAL("ber_scanf failed\n");
-			errMesg = "Invalid payload, failed to decode s2kparams.\n";
-			rc = LDAP_PROTOCOL_ERROR;
-			goto free_and_return;
-		}
+        /* FIXME: s2kparams - NOT implemented yet */
+        if (rtag == SKREQ_S2KPARAMS_TAG) {
+            rtag = ber_scanf(ber, "t[x]}", &ctag);
+        } else {
+            rtag = ber_scanf(ber, "}", &ctag);
+        }
+        if (rtag == LBER_ERROR) {
+            rc = LDAP_PROTOCOL_ERROR;
+            LOG_FATAL("ber_scanf failed to read key data termination\n");
+            err_msg = "Invalid payload.\n";
+            goto done;
+        }
 
-		rtag = ber_peek_tag(ber, &tlen);
-	}
+        rtag = ber_peek_tag(ber, &tlen);
+    }
 
-	ber_free(ber, 1);
-	ber = NULL;
+    rc = LDAP_SUCCESS;
 
-	/* filter un-supported encodings */
-	ret = filter_keys(krbcfg, kset);
-	if (ret) {
-		LOG_FATAL("keyset filtering failed\n");
-		goto free_and_return;
-	}
+done:
+    if (rc != LDAP_SUCCESS) {
+        if (kset) ipapwd_keyset_free(&kset);
+        free(svcname);
+        *_err_msg = err_msg;
+    } else {
+        *_svcname = svcname;
+        *_kset = kset;
+    }
+    if (ber) ber_free(ber, 1);
+    return rc;
+}
 
-	/* check if we have any left */
-	if (kset->num_keys == 0) {
-		LOG_FATAL("keyset filtering rejected all proposed keys\n");
-		errMesg = "All enctypes provided are unsupported";
-		rc = LDAP_UNWILLING_TO_PERFORM;
-		goto free_and_return;
-	}
+/* Format of response
+ *
+ * KeytabGetRequest ::= SEQUENCE {
+ * 	new_kvno	Int32
+ * 	SEQUENCE OF	KeyTypes
+ * }
+ *
+ * * List of accepted enctypes *
+ * KeyTypes ::= SEQUENCE {
+ * 	enctype		Int32
+ * }
+ */
 
-	smods = slapi_mods_new();
+static int encode_setkeytab_reply(struct ipapwd_keyset *kset,
+                                  struct berval **_bvp)
+{
+    int rc = LDAP_OPERATIONS_ERROR;
+    struct berval *bvp = NULL;
+    BerElement *ber = NULL;
 
-	/* change Last Password Change field with the current date */
-	if (!gmtime_r(&(time_now), &utctime)) {
-		LOG_FATAL("failed to retrieve current date (buggy gmtime_r ?)\n");
-		slapi_mods_free(&smods);
-		goto free_and_return;
-	}
-	strftime(timestr, GENERALIZED_TIME_LENGTH+1, "%Y%m%d%H%M%SZ", &utctime);
-	slapi_mods_add_string(smods, LDAP_MOD_REPLACE, "krbLastPwdChange", timestr);
-
-	/* FIXME: set Password Expiration date ? */
-#if 0
-	if (!gmtime_r(&(data->expireTime), &utctime)) {
-		LOG_FATAL("failed to convert expiration date\n");
-		slapi_ch_free_string(&randPasswd);
-		slapi_mods_free(&smods);
+    ber = ber_alloc();
+    if (!ber) {
+        rc = LDAP_OPERATIONS_ERROR;
+        LOG_OOM();
+        goto done;
+    }
+
+    rc = ber_printf(ber, "{i{", (ber_int_t)kset->keys[0].key_data_kvno);
+    if (rc == -1) {
+        rc = LDAP_OPERATIONS_ERROR;
+        LOG_FATAL("Failed to ber_printf the kvno");
+        goto done;
+    }
+
+    for (int i = 0; i < kset->num_keys; i++) {
+        rc = ber_printf(ber, "{i}", (ber_int_t)kset->keys[i].key_data_type[0]);
+    if (rc == -1) {
+            rc = LDAP_OPERATIONS_ERROR;
+            LOG_FATAL("Failed to ber_printf the enctype");
+            goto done;
+        }
+    }
+    rc = ber_printf(ber, "}}");
+    if (rc == -1) {
+        rc = LDAP_OPERATIONS_ERROR;
+        LOG_FATAL("Failed to ber_printf the termination");
+        goto done;
+    }
+
+    rc = ber_flatten(ber, &bvp);
+    if (rc == -1) {
+        rc = LDAP_OPERATIONS_ERROR;
+        LOG_FATAL("Failed to ber_flatten the buffer");
+        goto done;
+    }
+
+    rc = LDAP_SUCCESS;
+
+done:
+    if (rc != LDAP_SUCCESS) {
+        if (bvp) ber_bvfree(bvp);
+    } else {
+        *_bvp = bvp;
+    }
+    if (ber) ber_free(ber, 1);
+    return rc;
+}
+
+/* Password Modify Extended operation plugin function */
+static int ipapwd_setkeytab(Slapi_PBlock *pb, struct ipapwd_krbcfg *krbcfg)
+{
+	char *bindDN = NULL;
+	char *serviceName = NULL;
+	char *errMesg = NULL;
+	struct berval *extop_value = NULL;
+	Slapi_Entry *targetEntry=NULL;
+	struct berval *bval = NULL;
+	Slapi_Value **svals = NULL;
+	krb5_context krbctx = NULL;
+	krb5_error_code krberr;
+	struct ipapwd_keyset *kset = NULL;
+    int rc;
+    int kvno;
+    char *svcname;
+    bool allowed_access = false;
+    struct berval *bvp = NULL;
+    LDAPControl new_ctrl;
+
+	krberr = krb5_init_context(&krbctx);
+	if (krberr) {
+		LOG_FATAL("krb5_init_context failed\n");
 		rc = LDAP_OPERATIONS_ERROR;
 		goto free_and_return;
 	}
-	strftime(timestr, GENERALIZED_TIME_LENGTH+1, "%Y%m%d%H%M%SZ", &utctime);
-	slapi_mods_add_string(smods, LDAP_MOD_REPLACE, "krbPasswordExpiration", timestr);
-#endif
 
-	ret = ber_encode_krb5_key_data(kset->keys, kset->num_keys,
-                                       kset->mkvno, &bval);
-	if (ret != 0) {
-		LOG_FATAL("encoding krb5_key_data failed\n");
-		slapi_mods_free(&smods);
-		goto free_and_return;
-	}
+	/* Get Bind DN */
+	slapi_pblock_get(pb, SLAPI_CONN_DN, &bindDN);
 
-	svals[0] = slapi_value_new_berval(bval);
-	if (!svals[0]) {
-		LOG_FATAL("Converting berval to Slapi_Value\n");
-		slapi_mods_free(&smods);
+	 /* If the connection is bound anonymously, we must refuse to process
+	 * this operation. */
+	if (bindDN == NULL || *bindDN == '\0') {
+	 	/* Refuse the operation because they're bound anonymously */
+		errMesg = "Anonymous Binds are not allowed.\n";
+		rc = LDAP_INSUFFICIENT_ACCESS;
 		goto free_and_return;
 	}
 
-	slapi_mods_add_mod_values(smods, LDAP_MOD_REPLACE, "krbPrincipalKey", svals);
-
-	/* If we are creating a keytab for a host service attempt to remove
-	 * the userPassword attribute if it exists
-	*/
-	pw = slapi_entry_attr_get_charptr(targetEntry, "userPassword");
-	objectclass = slapi_value_new_string("ipaHost");
-	if ((slapi_entry_attr_has_syntax_value(targetEntry, SLAPI_ATTR_OBJECTCLASS, objectclass)) == 1)
-	{
-		char * krbLastPwdChange = slapi_entry_attr_get_charptr(targetEntry, "krbLastPwdChange");
-		char * enrolledBy = slapi_entry_attr_get_charptr(targetEntry, "enrolledBy");
-		if (NULL == enrolledBy) {
-			evals = (Slapi_Value **)calloc(2, sizeof(Slapi_Value *));
-
-			if (!evals) {
-				LOG_OOM();
-				slapi_mods_free(&smods);
-				goto free_and_return;
-			}
-
-			evals[0] = slapi_value_new_string(bindDN);
-			slapi_mods_add_mod_values(smods, LDAP_MOD_ADD, "enrolledBy", evals);
-		} else {
-			slapi_ch_free_string(&enrolledBy);
-		}
-		if ((NULL != pw) && (NULL == krbLastPwdChange)) {
-			slapi_mods_add_mod_values(smods, LDAP_MOD_DELETE, "userPassword", NULL);
-			LOG_TRACE("Removing userPassword from host entry\n");
-			slapi_ch_free_string(&pw);
-		}
-		slapi_value_free(&objectclass);
-	}
-	slapi_value_free(&objectclass);
+	/* Get the ber value of the extended operation */
+	slapi_pblock_get(pb, SLAPI_EXT_OP_REQ_VALUE, &extop_value);
 
-	/* commit changes */
-	ret = ipapwd_apply_mods(slapi_entry_get_dn_const(targetEntry), smods);
+    rc = decode_setkeytab_request(krbctx, krbcfg->kmkey, krbcfg->mkvno,
+                                  extop_value, &serviceName, &kset, &errMesg);
+    if (rc) {
+        goto free_and_return;
+    }
 
-	if (ret != LDAP_SUCCESS) {
-		slapi_mods_free(&smods);
-		goto free_and_return;
+    /* make sure it is a valid name */
+    svcname = check_service_name(krbctx, serviceName);
+    if (!svcname) {
+        rc = LDAP_OPERATIONS_ERROR;
+        goto free_and_return;
+    }
+    slapi_ch_free_string(&serviceName);
+    serviceName = svcname;
 
-	}
-	slapi_mods_free(&smods);
+	/* check entry before doing any other decoding */
 
-    ipapwd_set_extradata(slapi_entry_get_dn_const(targetEntry),
-                         serviceName, time_now);
+	/* get Entry by krbPrincipalName */
+    targetEntry = get_entry_by_principal(serviceName);
+    if (!targetEntry) {
+        errMesg = "PrincipalName not found.\n";
+        rc = LDAP_NO_SUCH_OBJECT;
+        goto free_and_return;
+    }
 
-	/* Format of response
-	 *
-	 * KeytabGetRequest ::= SEQUENCE {
-	 * 	new_kvno	Int32
-	 * 	SEQUENCE OF	KeyTypes
-	 * }
-	 *
-	 * * List of accepted enctypes *
-	 * KeyTypes ::= SEQUENCE {
-	 * 	enctype		Int32
-	 * }
-	 */
+    /* Accesseck strategy:
+     * If the user has WRITE access, a new keytab can be set on the entry.
+     * If not, then we fail immediately with insufficient access. This
+     * means that we don't leak any useful information to the client such
+     * as current password wrong, etc.
+     */
+    allowed_access = is_allowed_to_access_attr(pb, bindDN, targetEntry,
+                                               "krbPrincipalKey", NULL,
+                                               SLAPI_ACL_WRITE);
+    if (!allowed_access) {
+        LOG_FATAL("Access not allowed to set keytab on [%s]!\n",
+                  serviceName);
+        errMesg = "Insufficient access rights\n";
+        rc = LDAP_INSUFFICIENT_ACCESS;
+        goto free_and_return;
+    }
 
-	errMesg = "Internal Error\n";
-	rc = LDAP_OPERATIONS_ERROR;
+    /* get next kvno for entry (will be 1 if this is new) and fix keyset */
+    kvno = ipapwd_get_cur_kvno(targetEntry) + 1;
+    for (int i = 0; i < kset->num_keys; i++) {
+        kset->keys[i].key_data_kvno = kvno;
+    }
+    filter_keys(krbcfg, kset);
 
-	ber = ber_alloc();
-	if (!ber) {
+	/* check if we have any left */
+	if (kset->num_keys == 0) {
+		LOG_FATAL("keyset filtering rejected all proposed keys\n");
+		errMesg = "All enctypes provided are unsupported";
+		rc = LDAP_UNWILLING_TO_PERFORM;
 		goto free_and_return;
 	}
 
-	ret = ber_printf(ber, "{i{", (ber_int_t)kvno);
-	if (ret == -1) {
+	rc = ber_encode_krb5_key_data(kset->keys, kset->num_keys,
+                                       kset->mkvno, &bval);
+	if (rc != 0) {
+		LOG_FATAL("encoding krb5_key_data failed\n");
+		rc = LDAP_OPERATIONS_ERROR;
 		goto free_and_return;
 	}
 
-	for (i = 0; i < kset->num_keys; i++) {
-		ret = ber_printf(ber, "{i}", (ber_int_t)kset->keys[i].key_data_type[0]);
-		if (ret == -1) {
-			goto free_and_return;
-		}
+	svals = (Slapi_Value **)calloc(2, sizeof(Slapi_Value *));
+	if (!svals) {
+		LOG_OOM();
+		rc = LDAP_OPERATIONS_ERROR;
+		goto free_and_return;
 	}
-	ret = ber_printf(ber, "}}");
-	if (ret == -1) {
+
+	svals[0] = slapi_value_new_berval(bval);
+	if (!svals[0]) {
+		LOG_FATAL("Converting berval to Slapi_Value\n");
 		goto free_and_return;
 	}
 
-	if (ret != -1) {
-		struct berval *bvp;
-		LDAPControl new_ctrl;
+    rc = store_new_keys(targetEntry, serviceName, bindDN, svals, &errMesg);
+    if (rc) {
+        goto free_and_return;
+    }
 
-		ret = ber_flatten(ber, &bvp);
-		if (ret == -1) {
-			goto free_and_return;
-		}
+    rc = encode_setkeytab_reply(kset, &bvp);
+    if (rc) {
+        errMesg = "Internal Error.\n";
+        goto free_and_return;
+    }
 
-		new_ctrl.ldctl_oid = KEYTAB_RET_OID;
-		new_ctrl.ldctl_value = *bvp;
-		new_ctrl.ldctl_iscritical = 0;
-		rc= slapi_pblock_set(pb, SLAPI_ADD_RESCONTROL, &new_ctrl);
-		ber_bvfree(bvp);
-	}
+    new_ctrl.ldctl_oid = KEYTAB_RET_OID;
+    new_ctrl.ldctl_value = *bvp;
+    new_ctrl.ldctl_iscritical = 0;
+    rc = slapi_pblock_set(pb, SLAPI_ADD_RESCONTROL, &new_ctrl);
 
 	/* Free anything that we allocated above */
 free_and_return:
@@ -1104,26 +1247,17 @@ free_and_return:
 	if (kset) ipapwd_keyset_free(&kset);
 
 	if (bval) ber_bvfree(bval);
-	if (ber) ber_free(ber, 1);
+	if (bvp) ber_bvfree(bvp);
+
+    if (targetEntry) slapi_entry_free(targetEntry);
 
-	if (pbte) {
-		slapi_free_search_results_internal(pbte);
-		slapi_pblock_destroy(pbte);
-	}
 	if (svals) {
-		for (i = 0; svals[i]; i++) {
+		for (int i = 0; svals[i]; i++) {
 			slapi_value_free(&svals[i]);
 		}
 		free(svals);
 	}
-	if (evals) {
-		for (i = 0; evals[i]; i++) {
-			slapi_value_free(&evals[i]);
-		}
-		free(evals);
-	}
 
-	if (krbname) krb5_free_principal(krbctx, krbname);
 	if (krbctx) krb5_free_context(krbctx);
 
         if (rc == LDAP_SUCCESS)