Add initgroups support #26
Conversation
src/handlers.rs
Outdated
| let user = User::from_name(key.to_str()?)?; | ||
| debug!(log, "got user"; "user" => ?user); | ||
| let groups = if let Some(user) = user { | ||
| getgrouplist(key, user.gid)? |
There was a problem hiding this comment.
the comment above says to always return success, but this line implies it bails early if something goes wrong. looks like nix adds a too-many-groups error condition, even though libc is never going to return an error.
There was a problem hiding this comment.
might be less confusing to pull this if/else into serialize_initgroups and change than fn so it takes the username and primary gid
There was a problem hiding this comment.
- I changed this specific line to log that error condition and return an empty list, and I expanded the comment to explain why we return an empty list.
- There are still error conditions in
serialize_initgroupsthat we don't handle. It's potentially important that we don't drop the connection there. However, that's a problem for all of theserialize_functions, i.e, I think we should consider teachingmain()not to drop the connection but instead to provide some perfunctory reply and log what's going on, because all of those cases can cause glibc to decide that there isn't a working NSCD. I think that's for another PR, and we shouldn't special-caseserialize_initgroupshere. (I also think that several of these cases can be statically proven not to be problems, if we try harder, since they're integer conversions.)
There was a problem hiding this comment.
However, that's a problem for all of the serialize_ functions, i.e, I think we should consider teaching main() not to drop the connection but instead to provide some perfunctory reply and log what's going on, because all of those cases can cause glibc to decide that there isn't a working NSCD. I think that's for another PR, and we shouldn't special-case serialize_initgroups here.
+1
Apart from its own inherent value in rounding out user/group support, glibc uses the same variable to cache whether the current NSCD implementation handles group (GETGRBYNAME/GETGRBYGID) and INITGROUPS requests, so we need to have a working implementation of it.
Apart from its own inherent value in rounding out user/group support,
glibc uses the same variable to cache whether the current NSCD
implementation handles group (GETGRBYNAME/GETGRBYGID) and INITGROUPS
requests, so we need to have a working implementation of it.