On Wed, Nov 14, 2012 at 11:02:14AM +0100, Sumit Bose wrote:
On Tue, Nov 13, 2012 at 03:53:43PM +0100, Pavel Březina wrote:
> On 11/12/2012 02:33 PM, Pavel Březina wrote:
> >On 11/09/2012 03:18 PM, Simo Sorce wrote:
> >>On Fri, 2012-11-09 at 14:28 +0100, Pavel Březina wrote:
> >>> From 724ebcafa2d4e54df048ca2a578b03281fb7fe6f Mon Sep 17 00:00:00 2001
> >>>From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
> >>>Date: Fri, 9 Nov 2012 13:31:52 +0100
> >>>Subject: [PATCH 1/6] sudo: fix missing parameter in two debug messages
> >>>
> >>>---
> >>> src/responder/sudo/sudosrv_get_sudorules.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>>diff --git a/src/responder/sudo/sudosrv_get_sudorules.c
> >>>b/src/responder/sudo/sudosrv_get_sudorules.c
> >>>index
>
>>>b6e016c73cba6c86b9f20af63357c6fcc2b5508b..0e0437622e94a234b1597bcd272633f6944d4762
> >>>100644
> >>>--- a/src/responder/sudo/sudosrv_get_sudorules.c
> >>>+++ b/src/responder/sudo/sudosrv_get_sudorules.c
> >>>@@ -369,7 +369,7 @@ errno_t sudosrv_get_rules(struct sudo_cmd_ctx
> >>>*cmd_ctx)
> >>> NULL, &groupnames);
> >>> if (ret != EOK) {
> >>> DEBUG(SSSDBG_CRIT_FAILURE,
> >>>- ("Unable to retrieve user info [%d]: %s\n",
> >>>strerror(ret)));
> >>>+ ("Unable to retrieve user info [%d]: %s\n",
ret,
> >>>strerror(ret)));
> >>> goto done;
> >>> }
> >>
> >>The comment says 2 debug messages, but the patch only has one, which is
> >>wrong ?
> >>
> >>Simo.
> >>
> >
> >Hi,
> >thank you. Looks like I forgot to modify the second message. New patches
> >are attached.
>
> I'm attaching a new set of patches base on IRC discussion with Simo
> and Jakub.
>
> Domain name is no longer sent to sudo. This was done without
> changing the protocol.
>
I just checked the handling of subdomain users in patch 0003 and it
looks good to me. Rules for subdomain users are always searched in the
parent domain.
bye,
Sumit
I tested the functionality and the patches seem to work fine. Here is a
full review:
Patch 1/7: sudo: fix missing parameter in two debug messages
Ack
Patch 2/7: use tmp_ctx in sudosrv_get_sudorules_from_cache()
Ack
Patch 3/7: sudo: support users from subdomains
Ack
Patch 4/7: sudo: do not send domain name with username
Ack but please file a ticket to change the protocol
Patch 5/7: libsss_sudo: bump version to 2:1:1 (revision)
I think it would be better to get rid of the version completely and just
build with -module -avoid-version. The API is defined on the consumer
side only anyway.
Patch 6/7: sudo: print message if old protocol is used
I don't really agree with this change. Why should we allow higher
protocol numbers and only disallow 0?
Patch 7/7: sudo: print how many rules we are refreshing or returning
Ack