From a26fe47cb0cefded3fec7df975e068e2631062c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Esser?= Date: Thu, 19 Dec 2024 23:29:32 +0100 Subject: [PATCH] libnss_tcb: Match interfaces with NSS documentation. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit According to [1] the interfaces provided are of return-type 'enum nss_status', of which the _nss_database_getXXX_r() interfaces are mandated to take four parameters: a pointer to the result, a pointer to an additional buffer, the size of the buffer supplied, and a pointer to an integer to report error values (errnop). The returned status values are meant to be accompanied by a corresponding error value [2] passed through the errnop pointer. [1] https://www.gnu.org/software/libc/manual/html_node/NSS-Module-Function-Internals.html [2] https://www.gnu.org/software/libc/manual/html_node/NSS-Modules-Interface.html Signed-off-by: Björn Esser --- ChangeLog | 18 +++++++++++++++ libs/nss.c | 68 ++++++++++++++++++++++++++++++++++++++---------------- 2 files changed, 66 insertions(+), 20 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5c8dba9..15f4162 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,23 @@ 2024-12-20 Björn Esser + libnss_tcb: Match interfaces with NSS documentation. + According to [1] the interfaces provided are of return-type + 'enum nss_status', of which the _nss_database_getXXX_r() interfaces + are mandated to take four parameters: a pointer to the result, + a pointer to an additional buffer, the size of the buffer supplied, + and a pointer to an integer to report error values (errnop). The + returned status values are meant to be accompanied by a corresponding + error value [2] passed through the errnop pointer. + [1] https://www.gnu.org/software/libc/manual/html_node/NSS-Module-Function-Internals.html + [2] https://www.gnu.org/software/libc/manual/html_node/NSS-Modules-Interface.html + * libs/nss.c (_nss_tcb_setspent): Adapt return-type and use return + values from documented macro. + (_nss_tcb_endspent): Likewise. + (_nss_tcb_getspnam_r): Adapt return-type, change fourth parameter + to be 'int *errnop', use return values from documented macros, and + set proper errno in errnop before returning. + (_nss_tcb_getspent_r): Likewise. + libnss_tcb: Disallow potentially-malicious user names in getspnam(3). IEEE Std 1003.1-2001 allows only the following characters to appear in group- and usernames: letters, digits, underscores, periods, diff --git a/libs/nss.c b/libs/nss.c index bf4b9c1..c855bf5 100644 --- a/libs/nss.c +++ b/libs/nss.c @@ -13,27 +13,27 @@ static __thread DIR *tcbdir = NULL; -int _nss_tcb_setspent(void) +enum nss_status _nss_tcb_setspent(void) { if (!tcbdir) { tcbdir = opendir(TCB_DIR); if (!tcbdir) return NSS_STATUS_UNAVAIL; - return 1; + return NSS_STATUS_SUCCESS; } rewinddir(tcbdir); - return 1; + return NSS_STATUS_SUCCESS; } -int _nss_tcb_endspent(void) +enum nss_status _nss_tcb_endspent(void) { if (tcbdir) { closedir(tcbdir); tcbdir = NULL; } - return 1; + return NSS_STATUS_SUCCESS; } /****************************************************************************** @@ -91,8 +91,8 @@ static FILE *tcb_safe_open(const char *file, const char *name) return f; } -int _nss_tcb_getspnam_r(const char *name, struct spwd *__result_buf, - char *__buffer, size_t __buflen, struct spwd **__result) +enum nss_status _nss_tcb_getspnam_r(const char *name, + struct spwd *__result_buf, char *__buffer, size_t __buflen, int *__errnop) { FILE *f; char *file; @@ -100,48 +100,71 @@ int _nss_tcb_getspnam_r(const char *name, struct spwd *__result_buf, /* Disallow potentially-malicious user names */ if (!is_valid_username(name)) { - errno = ENOENT; + /* we don't serve an entry here */ + *__errnop = ENOENT; return NSS_STATUS_NOTFOUND; } - if (asprintf(&file, TCB_FMT, name) < 0) + if (asprintf(&file, TCB_FMT, name) < 0) { + /* retry, as malloc or another resource has failed */ + *__errnop = EAGAIN; return NSS_STATUS_TRYAGAIN; + } + f = tcb_safe_open(file, name); free(file); - if (!f) - return errno == ENOENT ? NSS_STATUS_NOTFOUND : NSS_STATUS_UNAVAIL; + if (!f) { + /* $user/shadow not existing nor readable */ + *__errnop = ENOENT; + return NSS_STATUS_UNAVAIL; + } - retval = fgetspent_r(f, __result_buf, __buffer, __buflen, __result); + retval = fgetspent_r(f, __result_buf, __buffer, + __buflen, &__result_buf); saved_errno = errno; fclose(f); errno = saved_errno; - if (!retval) - return NSS_STATUS_SUCCESS; - switch (saved_errno) { + /* real error number is retval from fgetspent_r(), + by NSS spec errnop *MUST NOT* be set to 0 */ + if (retval) + *__errnop = retval; + + switch (retval) { case 0: + /* no error, entry found */ return NSS_STATUS_SUCCESS; case ENOENT: + /* if the file would not exist nor be readable, we would + have already bailed out with ENOENT/NSS_STATUS_UNAVAIL + immediately after the call to tcb_safe_open() */ return NSS_STATUS_NOTFOUND; + case EAGAIN: + /* ressources are temporary not available */ + return NSS_STATUS_TRYAGAIN; + case ERANGE: + /* buffer too small */ return NSS_STATUS_TRYAGAIN; default: + /* something else, e.g. parser error, but we can't help it */ return NSS_STATUS_UNAVAIL; } } -int _nss_tcb_getspent_r(struct spwd *__result_buf, - char *__buffer, size_t __buflen, struct spwd **__result) +enum nss_status _nss_tcb_getspent_r(struct spwd *__result_buf, + char *__buffer, size_t __buflen, int *__errnop) { struct dirent *result; off_t currpos; int retval, saved_errno; if (!tcbdir) { - errno = ENOENT; + /* tcbdir does not exist */ + *__errnop = ENOENT; return NSS_STATUS_UNAVAIL; } @@ -154,12 +177,16 @@ int _nss_tcb_getspent_r(struct spwd *__result_buf, closedir(tcbdir); errno = saved_errno; tcbdir = NULL; + /* cannot iterate tcbdir */ + *__errnop = ENOENT; return NSS_STATUS_UNAVAIL; } if (!result) { closedir(tcbdir); - errno = ENOENT; + errno = saved_errno; tcbdir = NULL; + /* we have no more entries in tcbdir */ + *__errnop = ENOENT; return NSS_STATUS_NOTFOUND; } errno = saved_errno; @@ -167,8 +194,9 @@ int _nss_tcb_getspent_r(struct spwd *__result_buf, !strcmp(result->d_name, "..") || result->d_name[0] == ':'); retval = _nss_tcb_getspnam_r(result->d_name, __result_buf, __buffer, - __buflen, __result); + __buflen, __errnop); + /* errnop has already been set by _nss_tcb_getspnam_r() */ switch (retval) { case NSS_STATUS_SUCCESS: return NSS_STATUS_SUCCESS;