Version-Release number of selected component (if applicable): pam_krb5-2.3.1-1 This patch adds a readme file regarding configuration options within the krb5.conf, it adds a --with-ldap configuration option, and adds remote ldap/active directory uid/gid mapping.
Created attachment 357189 [details] This will allow ldap/active directory uid/gid mapping I have only tested this against the pam_krb5-2.3.1-1 release
Thanks for the patch. Sorry about not getting to this sooner. I'm not sure that this functionality would best be added directly to pam_krb5, as it's not directly related to its function, and I could easily see wanting to use this in combination with other modules (for example, pam_ldap, if for whatever reason an admin didn't want to use nss_ldap, nss-ldapd, or sssd). Putting it into a different module (that would then return PAM_IGNORE for every function that it provided) would let it be called before all other modules, which I think would work better because we don't always call pam_krb5 first, and this is the sort of function I'd want to have called before other plugins get a chance to do anything (for example, pam_succeed_if, which can be used to allow or deny access based on account attributes which the patch retrieves). As written, the functionality added by the patch is enabled at compile time, which could be a problem because if I'm reading it right, leaving the new options unset would leave them set to NULL in the options structure. I expect that this'll trigger a segfault in _check_ldap_options when it passes one of the values to strlen(). I think there are some memory leaks in there -- strings added to the options structure don't get freed in _pam_krb5_options_free(), and the uidNumber, gidNumber, and loginShell values appear to be leaked from _ldap_search(), though the homeDirectory list is freed. I don't really understand what _ldap_search() is doing when there's more than one value for the home directory attribute -- could you describe the intention there? If ldap_get_values() returns NULL, for example if the located object doesn't have any values for the requested attribute, there might also be a crash. The shell value added to the userinfo structure in _ldap_search() also not freed -- it should probably be allocated here (either as a copy of the default value or of the value we got back from the directory) and always freed by _pam_krb5_user_info_free(), which would allow the loginShell value list to be freed. At the top of _ldap_search(), options->schema needs to be compared with "ad" using strcmp() or strcasecmp(), which compare contents, rather than ==, which compares the pointer values directly. The _ldap_search() function attempts to return either 1 (a number) on failure, or the userinfo structure (a pointer value) on success -- returning NULL would make more sense to me. When _check_file() is passed a descriptor, the patch attempts to use 0 to signal that the descriptor isn't connected to a file -- I think it'd be better to use -1 as the value there, because -1 is the error value returned by either open(), socket(), accept(), or other functions that create descriptors (so it can never be a descriptor), while 0 is actually a valid descriptor value. Forcing the use of "cn" is probably an unnecessary limitation -- if it's pointed at a server that's holding entries the way Unix boxes expect, it'd likely be expected to use "uid", for example. The _chk_local_account() function added by the patch uses getpwnam(), when _get_pw_nam() is already trying to avoid calling the non-reentrant version of the function. If _get_pw_name() skipped writing to the pointers it's passed if they're NULL, it would work as well. My big concern is that the locking scheme you're using to avoid damaging the /etc/passwd file is not same as the one used by utilities such as useradd, which could lead to some file corruption. My personal preference would be to use fork() and exec() to call useradd, since it also knows (or can be expected to know) how to get nscd to flush its cache -- negative caching (caching the knowledge that there is no such user) is a potential problem if this is used with nscd. The readme file seems to be missing from the patch, but that's solvable. There's no need to patch config.h.in if AC_DEFINE is called with three arguments -- autoheader'll take care of it. Thanks for taking the time to file the patch, hopefully you found this helpful.
I came across this report because it was assigned against to FC2. Normally I would change it to the correct version, however, after reading the comments I am wondering if this should be closed?
I am in the process of updating this patch to include the features/fixes Mr. Dahyabhai suggested. I should have it re-submitted within the month.
My apologies on this being open so long. I have made the changes and created a new patch. I do understand the concern in perhaps making this a separate module but with the afs support as being an internal function I thought having the option for the administrator to decide if he wishes to utilize an active directory or openldap backend to service the UID/GID verification this might be an easier way to do so without the need to configure the nss_ldap or pam_ldap within the stack. In any event if you have feedback about the patch, I do appreciate it.
Created attachment 382024 [details] A patch reflecting the changes as suggested. I have also integrated a feature to dynamically add the users to a comma separated list of groups to bring more functionality to the patch and features.
This message is a reminder that Fedora 12 is nearing its end of life. Approximately 30 (thirty) days from now Fedora will stop maintaining and issuing updates for Fedora 12. It is Fedora's policy to close all bug reports from releases that are no longer maintained. At that time this bug will be closed as WONTFIX if it remains open with a Fedora 'version' of '12'. Package Maintainer: If you wish for this bug to remain open because you plan to fix it in a currently maintained version, simply change the 'version' to a later Fedora version prior to Fedora 12's end of life. Bug Reporter: Thank you for reporting this issue and we are sorry that we may not be able to fix it before Fedora 12 is end of life. If you would still like to see this bug fixed and are able to reproduce it against a later version of Fedora please change the 'version' of this bug to the applicable version. If you are unable to change the version, please add a comment here and someone will do it for you. Although we aim to fix as many bugs as possible during every release's lifetime, sometimes those efforts are overtaken by events. Often a more recent Fedora release includes newer upstream software that fixes bugs or makes them obsolete. The process we are following is described here: http://fedoraproject.org/wiki/BugZappers/HouseKeeping
Fedora 12 changed to end-of-life (EOL) status on 2010-12-02. Fedora 12 is no longer maintained, which means that it will not receive any further security or bug fix updates. As a result we are closing this bug. If you can reproduce this bug against a currently maintained version of Fedora please feel free to reopen this bug against that version. Thank you for reporting this bug and we are sorry it could not be fixed.