Skip Menu |
 

From: "Machin, Glenn D" <GMachin@sandia.gov>
To: "krb5-bugs@mit.edu" <krb5-bugs@mit.edu>
Subject: Issue setting require_auth attribute with ldap backend with release 1.17
Date: Sat, 22 Feb 2020 22:28:28 +0000
Download (untitled) / with headers
text/plain 2.5KiB

After setting an authentication indicator on a service. You cannot use delstr to remove it.

 

    kadmin.local:  setstr host/hostname.domain@realm require_auth LOA2

    Attribute set for principal " host/hostname.domain@realm "

 

    kadmin.local:  getstrs host/hostname.domain 

    require_auth: LOA2

    

    kadmin.local:  delstr host/hostname.domain require_auth

    Attribute removed from principal "host/hostname.domain@realm".

    

    kadmin.local:   getstrs host/hostname.domain 

    require_auth: LOA2

 

 

krb5-1.17/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c nevers checks to see if krbPrincipalAuthInd exists, in the case where it’s not being set.

 

/* Parse the "require_auth" string for auth indicators, adding them to the

 * krbPrincipalAuthInd attribute. */

static krb5_error_code

update_ldap_mod_auth_ind(krb5_context context, krb5_db_entry *entry,

                         LDAPMod ***mods)

{

    int i = 0;

    krb5_error_code ret;

    char *auth_ind = NULL;

    char *strval[10] = { 0 };

    char *ai, *ai_save = NULL;

    int sv_num = sizeof(strval) / sizeof(*strval);

 

    ret = krb5_dbe_get_string(context, entry, KRB5_KDB_SK_REQUIRE_AUTH,

                              &auth_ind);

    if (ret || auth_ind == NULL)

 

        goto cleanup;

 

    ai = strtok_r(auth_ind, " ", &ai_save);

    while (ai != NULL && i < sv_num) {

        strval[i++] = ai;

        ai = strtok_r(NULL, " ", &ai_save);

    }

 

    ret = krb5_add_str_mem_ldap_mod(mods, "krbPrincipalAuthInd",

                                    LDAP_MOD_REPLACE, strval);

 

cleanup:

    krb5_dbe_free_string(context, auth_ind);

    return ret;

}

 

Change above to :

int attr_mask = 0;

                krb5_boolean has_AuthInd;

 

   if (ret || auth_ind == NULL)

    {

        /* No krbPrincipalAuthInd to be set - lets check and see if current */

        /* settings in ldap has it set. If so then we need to delete it */

        ret = krb5_get_attributes_mask(context, entry, &attr_mask);

        if (ret == 0){

            /* If current ldap entry has krbPrincipalAuthInd set we need to delete it */

            has_AuthInd = ((attr_mask & KDB_AUTH_IND_ATTR ) != 0);

            if (has_AuthInd) {

                ret = krb5_add_str_mem_ldap_mod(mods, "krbPrincipalAuthInd",

                                               LDAP_MOD_DELETE,

                                               NULL );

            }

        }

        goto cleanup;

    }

 

 

My notes:

* Normalization of the "require_auth" string attribute to the krbPrincipalAuthInd LDAP attribute was added in release 1.15 (ticket 8379), while auth indicator support itself was added in release 1.14 (ticket 8157).

* This is currently the only normalization of a string attribute, although several tl-data attributes (like KRB5_TL_LAST_PWD_CHANGE) are normalized.

* Unlike other normalizations, the require_auth attribute is also represented redundantly in krbExtraData (within KRB5_TL_STRING_ATTRS).  This is at least partially because krb5 1.14 could have written entries to LDAP without normalization.

* On load, if any krbPrincipalAuthInd attributes are present, they are re-encoded as a string attribute and set or replace the value of the require_auth string attribute in the tl-data.  This is so that modifications to krbPrincipalAuthInd via LDAP will have the intended effect on the principal entry.

* There is an ancillary bug that removing all krbPrincipalAuthInd attributes via LDAP won't have the effect of removing the require_auth string attribute.  There is currently no way to distinguish this state from an entry stored without normalization (by krb5 1.14).

* The attribute mask returned by krb5_get_attributes_mask() is synthesized on principal entry load by populate_krb5_db_entry().  It it stored in memory in the tl-data under the type KDB_TL_USER_INFO, encoded under the subtype KDB_TL_MASK.  KDB_TL_USER_INFO tl-data is not written out to LDAP.

* Using the attribute mask to fix the primary bug would miss the case where the caller writes out an entry from scratch, rather than reading and entry and modifying it.  That might be an acceptable blind spot, compared to the cost of alternative fixes.

* To fix the ancillary bug, we would probably have to change the normalization strategy so that the require_auth string attribute is no longer stored within krbExtraData (but can still be loaded from krbExtraData).  That change would mean that krb5 1.14 KDCs would no longer be able to see require_auth string attributes on entries written out by current KDCs.
Subject: git commit
From: ghudson@mit.edu
Download (untitled) / with headers
text/plain 1.2KiB

Allow deletion of require_auth with LDAP KDB

In update_ldap_mod_auth_ind(), if there is no string attribute value
for require_auth, check for krbPrincipalAuthInd attributes that might
need to be removed. (This will only work if the entry is loaded and
then modified, but that is the normal case for an existing entry.)

Move the update_ldap_mod_auth_ind() call inside the tl-data
conditional (which should perhaps be a check for KADM5_TL_DATA in the
mask instead). A modification which did not intend to update tl-data
should not remove the krbPrincipalAuthInd attributes.

Change get_int_from_tl_data() to to zero its output so that it can't
leave a garbage value behind if it returns 0 (as it does if no
KDB_TL_USER_INFO tl-data is present).

Based on a patch by Glenn Machin.

https://github.com/krb5/krb5/commit/6d9da7bb216f96cbdd731aa894714bd84213a9d0
Author: Greg Hudson <ghudson@mit.edu>
Commit: 6d9da7bb216f96cbdd731aa894714bd84213a9d0
Branch: master
src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.c | 2 +
src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c | 31 +++++++++++++-------
src/tests/t_kdb.py | 26 ++++++++++++++++-
3 files changed, 47 insertions(+), 12 deletions(-)
From: ghudson@mit.edu
Subject: git commit
Download (untitled) / with headers
text/plain 1.2KiB

Allow deletion of require_auth with LDAP KDB

In update_ldap_mod_auth_ind(), if there is no string attribute value
for require_auth, check for krbPrincipalAuthInd attributes that might
need to be removed. (This will only work if the entry is loaded and
then modified, but that is the normal case for an existing entry.)

Move the update_ldap_mod_auth_ind() call inside the tl-data
conditional (which should perhaps be a check for KADM5_TL_DATA in the
mask instead). A modification which did not intend to update tl-data
should not remove the krbPrincipalAuthInd attributes.

Change get_int_from_tl_data() to to zero its output so that it can't
leave a garbage value behind if it returns 0 (as it does if no
KDB_TL_USER_INFO tl-data is present).

Based on a patch by Glenn Machin.

(cherry picked from commit 6d9da7bb216f96cbdd731aa894714bd84213a9d0)

https://github.com/krb5/krb5/commit/0badbf05a8cc7981980b20f2d3aa05989232f0b0
Author: Greg Hudson <ghudson@mit.edu>
Commit: 0badbf05a8cc7981980b20f2d3aa05989232f0b0
Branch: krb5-1.18
src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.c | 2 +
src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c | 31 +++++++++++++-------
src/tests/t_kdb.py | 26 ++++++++++++++++-
3 files changed, 47 insertions(+), 12 deletions(-)
Subject: git commit
From: ghudson@mit.edu
Download (untitled) / with headers
text/plain 1.2KiB

Allow deletion of require_auth with LDAP KDB

In update_ldap_mod_auth_ind(), if there is no string attribute value
for require_auth, check for krbPrincipalAuthInd attributes that might
need to be removed. (This will only work if the entry is loaded and
then modified, but that is the normal case for an existing entry.)

Move the update_ldap_mod_auth_ind() call inside the tl-data
conditional (which should perhaps be a check for KADM5_TL_DATA in the
mask instead). A modification which did not intend to update tl-data
should not remove the krbPrincipalAuthInd attributes.

Change get_int_from_tl_data() to to zero its output so that it can't
leave a garbage value behind if it returns 0 (as it does if no
KDB_TL_USER_INFO tl-data is present).

Based on a patch by Glenn Machin.

(cherry picked from commit 6d9da7bb216f96cbdd731aa894714bd84213a9d0)

https://github.com/krb5/krb5/commit/03cc033ece30c515a6d7e72c4b37c9b7ca746acd
Author: Greg Hudson <ghudson@mit.edu>
Commit: 03cc033ece30c515a6d7e72c4b37c9b7ca746acd
Branch: krb5-1.17
src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.c | 2 +
src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c | 31 +++++++++++++-------
src/tests/t_kdb.py | 26 ++++++++++++++++-
3 files changed, 47 insertions(+), 12 deletions(-)