Project

General

Profile

Bug #12236

getmembers_DN doesn't properly handle errors from __ns_ldap_dn2uid

Added by Jason King 11 months ago. Updated 10 months ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
lib - userland libraries
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

Helping sjorge on irc with some ldap group issues (where groups weren't showing up), we discovered the following:

  3  90236          __ns_ldap_dn2uid:return                 2
  3  90229             getmembers_DN:return                 2
  3  90245       _nss_ldap_group2str:return                 2

Note that __ns_ldap_dn2uid returns a value from enum ns_ldap_return_code. In this case 2 is equal to NS_LDAP_NOTFOUND which seems reasonable. However, getmembers_DN should return one of: NSS_STR_PARSE_SUCCESS, NSS_STR_PARSE_PARSE, NSS_STR_PARSE_ERANGE. There seems to be two issues here:

  1. The return value of __ns_ldap_dn2uid should be mapped to one of the NSS_STR_ values before getmembers_DN returns
  2. It seems likely that since the existing code in getmembers_DN appears to just ignore errors from __ns_ldap_dn2uid that at least some errors that are ignored should also reset the value of nss_result. Possibly, getmembers_DN might also instead be a void function if no errors are intended to be returned.

Related issues

Related to illumos gate - Bug #12240: nss_ldap does not properly look up group members by distinguished nameClosed

Actions
#1

Updated by Jorge Schrauwen 11 months ago

Tiny bit more background, I am trying to use rfc2307bis with groups using the member attribute holding a DN of the user. (as per the rfc)

For groups with 1 or more member attribute, the group does not show up in getent group, possibly due to nss not returning properly. Due to the issue found above.

#2

Updated by Jason King 11 months ago

A proposed fix:

diff --git a/usr/src/lib/nsswitch/ldap/common/getgrent.c b/usr/src/lib/nsswitch/ldap/common/getgrent.c
index 184891c9d3..615b67a4e5 100644
--- a/usr/src/lib/nsswitch/ldap/common/getgrent.c
+++ b/usr/src/lib/nsswitch/ldap/common/getgrent.c
@@ -23,6 +23,7 @@
  * Use is subject to license terms.
  *
  * Copyright 2017 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright 2020 Joyent, Inc.
  */

 #include <grp.h>
@@ -239,8 +240,9 @@ getmembers_DN(char **bufpp, int *lenp, ns_ldap_attr_t *members)
        char    *buffer;
        int     buflen;
        int     i, len;
-       int     nss_result = 0;
+       int     nss_result = 0; /* used by TEST_AND_ADJUST macro */
        int     firsttime;
+       ns_ldap_return_code dn2uid_rc;

        buffer = *bufpp;
        buflen = *lenp;
@@ -263,9 +265,9 @@ getmembers_DN(char **bufpp, int *lenp, ns_ldap_attr_t *members)
                if (member_dn[0] == '\0')
                        continue;

-               nss_result = __ns_ldap_dn2uid(member_dn,
+               dn2uid_rc = __ns_ldap_dn2uid(member_dn,
                    &member_uid, NULL, &error);
-               if (nss_result != NS_LDAP_SUCCESS) {
+               if (dn2uid_rc != NS_LDAP_SUCCESS) {
                        (void) __ns_ldap_freeError(&error);
                        error = NULL;
                        continue;
#3

Updated by Jason King 11 months ago

Slightly more compact version:

diff --git a/usr/src/lib/nsswitch/ldap/common/getgrent.c b/usr/src/lib/nsswitch/ldap/common/getgrent.c
index 184891c9d3..765059e86a 100644
--- a/usr/src/lib/nsswitch/ldap/common/getgrent.c
+++ b/usr/src/lib/nsswitch/ldap/common/getgrent.c
@@ -23,6 +23,7 @@
  * Use is subject to license terms.
  *
  * Copyright 2017 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright 2020 Joyent, Inc.
  */

 #include <grp.h>
@@ -239,7 +240,7 @@ getmembers_DN(char **bufpp, int *lenp, ns_ldap_attr_t *members)
        char    *buffer;
        int     buflen;
        int     i, len;
-       int     nss_result = 0;
+       int     nss_result = 0; /* used by TEST_AND_ADJUST macro */
        int     firsttime;

        buffer = *bufpp;
@@ -263,9 +264,8 @@ getmembers_DN(char **bufpp, int *lenp, ns_ldap_attr_t *members)
                if (member_dn[0] == '\0')
                        continue;

-               nss_result = __ns_ldap_dn2uid(member_dn,
-                   &member_uid, NULL, &error);
-               if (nss_result != NS_LDAP_SUCCESS) {
+               if (__ns_ldap_dn2uid(member_dn,
+                   &member_uid, NULL, &error) != NS_LDAP_SUCCESS) {
                        (void) __ns_ldap_freeError(&error);
                        error = NULL;
                        continue;
#4

Updated by Jorge Schrauwen 11 months ago

I applied patch v2 and build a new PI.

I now see the admins group! (but it is still empty tough), so I asume getmembers_DN still fails... but the failure does not result in a missing group. I'm cursious why it can't fine the DN as it does clearly exist.

[root@freeradius ~]# getent group admins
admins::10002:
[root@freeradius ~]# getent passwd sjorge
sjorge:x:10001:10001:Jorge Schrauwen:/home/sjorge:/opt/local/bin/zsh
# users, groups, acheron.be
dn: cn=users,ou=groups,dc=acheron,dc=be
cn: users
gidNumber: 10001
description: Users
objectClass: posixGroup
objectClass: groupOfMembers

# admins, groups, acheron.be
dn: cn=admins,ou=groups,dc=acheron,dc=be
cn: admins
gidNumber: 10002
description: System Administrators
objectClass: posixGroup
objectClass: groupOfMembers
member: uid=sjorge,ou=accounts,dc=acheron,dc=be

# sjorge, accounts, acheron.be
dn: uid=sjorge,ou=accounts,dc=acheron,dc=be
cn: sjorge
uid: sjorge
uidNumber: 10001
gidNumber: 10001
homeDirectory: /home/sjorge
loginShell: /opt/local/bin/zsh
userPassword:: <REMOVED>
gecos: Jorge Schrauwen
sn: Schrauwen
givenName: Jorge
SolarisAttrKeyValue: type=normal;profiles=Primary Administrator
sshPublicKey: <REMOVED>
objectClass: posixAccount
objectClass: shadowAccount
objectClass: inetOrgPerson
objectClass: SolarisUserAttr
objectClass: ldapPublicKey
#5

Updated by Jason King 11 months ago

  • Related to Bug #12240: nss_ldap does not properly look up group members by distinguished name added
#6

Updated by Jason King 10 months ago

Note that the missing members is addressed by #12240. In addition to the testing Jorge did (the 2nd patch being what was ultimately reviewed), I also setup a small ldap server and loaded it with test data. As part of the test data, 3 groups were added to LDAP, 2 of which had a test user as a secondary member. Without the fix, the groups with the secondary members by DN did not show up. With the fix they appeared. (This was tested in conjunction with the fix for #12240).

#7

Updated by Electric Monk 10 months ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit d7ab8532a7a0f65d1c2b7bc3f45072f665860b20

commit  d7ab8532a7a0f65d1c2b7bc3f45072f665860b20
Author: Jason King <jason.king@joyent.com>
Date:   2020-02-15T23:23:54.000Z

    12236 getmembers_DN doesn't properly handle errors from __ns_ldap_dn2uid
    12240 nss_ldap does not properly look up group members by distinguished name
    Reviewed by: Jorge Schrauwen <jorge@blackdot.be>
    Reviewed by: Gordon Ross <gordon.w.ross@gmail.com>
    Reviewed by: Andy Fiddaman <omnios@citrus-it.co.uk>
    Reviewed by: Matt Barden <matt.barden@nexenta.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Also available in: Atom PDF