[PATCH] skip rule on error instead of failing completely - sssd-devel - Fedora Mailing-Lists

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

[PATCH] skip rule on error instead of failing completely

[PATCH] fix GSSAPI integrity...

[PATCH] SSH: Ensure that...

Pavel Březina

Friday, 26 July 2013 Fri, 26 Jul '13

5:34 a.m.

https://fedorahosted.org/sssd/ticket/2031

Attachments:

0002-sudo-print-better-debug-message-when-a-rule-has-mult.patch (text/x-patch — 1.2 KB)
0001-sudo-skip-rule-on-error-instead-of-failing-completel.patch (text/x-patch — 1.1 KB)

Reply

Show replies by date

Jakub Hrozek

Friday, 26 July Fri, 26 Jul

6:31 a.m.

On Fri, Jul 26, 2013 at 12:34:47PM +0200, Pavel Březina wrote:

https://fedorahosted.org/sssd/ticket/2031

> From fecd2799c21dc78fcb098e786b1c7e879e943c5b Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com> > Date: Fri, 26 Jul 2013 12:25:01 +0200 > Subject: [PATCH 1/2] sudo: skip rule on error instead of failing completely >

https://fedorahosted.org/sssd/ticket/2031

> --- > src/providers/ldap/sdap_sudo_cache.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/providers/ldap/sdap_sudo_cache.c b/src/providers/ldap/sdap_sudo_cache.c > index 3c438b9307c310cd4eec030ad628ce28a66ba726..9a2d326e0e11cf90279d8327afaf6a0ab76cac85 100644 > --- a/src/providers/ldap/sdap_sudo_cache.c > +++ b/src/providers/ldap/sdap_sudo_cache.c > @@ -135,7 +135,9 @@ sdap_save_native_sudorule_list(TALLOC_CTX *mem_ctx, > domain, map, replies[i], > cache_timeout, now, &usn_value); > if (ret != EOK) { > - goto fail; > + DEBUG(SSSDBG_OP_FAILURE, ("Failed to save sudo rule, " > + "will continue with next...\n")); > + continue; > } > > /* find highest usn */ > -- > 1.7.11.7 >

From 09546d945e2b4932550c9d267c2146ac4c901e59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com> Date: Fri, 26 Jul 2013 12:29:37 +0200 Subject: [PATCH 2/2] sudo: print better debug message when a rule has multiple cn values --- src/providers/ldap/sdap_sudo_cache.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/providers/ldap/sdap_sudo_cache.c b/src/providers/ldap/sdap_sudo_cache.c index 9a2d326e0e11cf90279d8327afaf6a0ab76cac85..aaaa1e2aa545864c01acd160c211ecfffce874e2 100644 --- a/src/providers/ldap/sdap_sudo_cache.c +++ b/src/providers/ldap/sdap_sudo_cache.c @@ -69,7 +69,11 @@ sdap_save_native_sudorule(TALLOC_CTX *mem_ctx, ret = sysdb_attrs_get_string(attrs, map[SDAP_AT_SUDO_NAME].sys_name, &rule_name); - if (ret != EOK) { + if (ret == ERANGE) { + DEBUG(SSSDBG_OP_FAILURE, ("Warning: found rule that contains none " + "or multiple CN values. It will be skipped.\n")); + return ret; + } else if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("Could not get rule name [%d]: %s\n", ret, strerror(ret))); return ret; -- 1.7.11.7

Can you make either of those DEBUG messages more verbose? I think we should make it clear that we have not processed all the rules. Also I wonder if we should support multiple names by looking at RDN value and picking the one that matches the RDN? That's what we do for users and groups anyway. What is the sudo rule name used for (except for cn=defaults) ?

Reply

Pavel Březina

6:46 a.m.

On 07/26/2013 01:31 PM, Jakub Hrozek wrote:

On Fri, Jul 26, 2013 at 12:34:47PM +0200, Pavel Březina wrote: > https://fedorahosted.org/sssd/ticket/2031 > From fecd2799c21dc78fcb098e786b1c7e879e943c5b Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com> > Date: Fri, 26 Jul 2013 12:25:01 +0200 > Subject: [PATCH 1/2] sudo: skip rule on error instead of failing completely > > https://fedorahosted.org/sssd/ticket/2031 > --- > src/providers/ldap/sdap_sudo_cache.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/providers/ldap/sdap_sudo_cache.c b/src/providers/ldap/sdap_sudo_cache.c > index 3c438b9307c310cd4eec030ad628ce28a66ba726..9a2d326e0e11cf90279d8327afaf6a0ab76cac85 100644 > --- a/src/providers/ldap/sdap_sudo_cache.c > +++ b/src/providers/ldap/sdap_sudo_cache.c > @@ -135,7 +135,9 @@ sdap_save_native_sudorule_list(TALLOC_CTX *mem_ctx, > domain, map, replies[i], > cache_timeout, now, &usn_value); > if (ret != EOK) { > - goto fail; > + DEBUG(SSSDBG_OP_FAILURE, ("Failed to save sudo rule, " > + "will continue with next...\n")); > + continue; > } > > /* find highest usn */ > -- > 1.7.11.7 > > From 09546d945e2b4932550c9d267c2146ac4c901e59 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com> > Date: Fri, 26 Jul 2013 12:29:37 +0200 > Subject: [PATCH 2/2] sudo: print better debug message when a rule has > multiple cn values > > --- > src/providers/ldap/sdap_sudo_cache.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/providers/ldap/sdap_sudo_cache.c b/src/providers/ldap/sdap_sudo_cache.c > index 9a2d326e0e11cf90279d8327afaf6a0ab76cac85..aaaa1e2aa545864c01acd160c211ecfffce874e2 100644 > --- a/src/providers/ldap/sdap_sudo_cache.c > +++ b/src/providers/ldap/sdap_sudo_cache.c > @@ -69,7 +69,11 @@ sdap_save_native_sudorule(TALLOC_CTX *mem_ctx, > > ret = sysdb_attrs_get_string(attrs, map[SDAP_AT_SUDO_NAME].sys_name, > &rule_name); > - if (ret != EOK) { > + if (ret == ERANGE) { > + DEBUG(SSSDBG_OP_FAILURE, ("Warning: found rule that contains none " > + "or multiple CN values. It will be skipped.\n")); > + return ret; > + } else if (ret != EOK) { > DEBUG(SSSDBG_OP_FAILURE, ("Could not get rule name [%d]: %s\n", > ret, strerror(ret))); > return ret; > -- > 1.7.11.7 > Can you make either of those DEBUG messages more verbose? I think we should make it clear that we have not processed all the rules.

New patches are attached.

Also I wonder if we should support multiple names by looking at RDN value and picking the one that matches the RDN? That's what we do for users and groups anyway.

Yes, ctrianta will file a bug/rfe and we'll do that as separate patch.

What is the sudo rule name used for (except for cn=defaults) ?

Only administrative purpose.

Reply

Jakub Hrozek

Monday, 5 August Mon, 5 Aug

9:55 a.m.

On Fri, Jul 26, 2013 at 01:46:04PM +0200, Pavel Březina wrote:

On 07/26/2013 01:31 PM, Jakub Hrozek wrote: >On Fri, Jul 26, 2013 at 12:34:47PM +0200, Pavel Březina wrote: >>https://fedorahosted.org/sssd/ticket/2031 > >> From fecd2799c21dc78fcb098e786b1c7e879e943c5b Mon Sep 17 00:00:00 2001 >>From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com> >>Date: Fri, 26 Jul 2013 12:25:01 +0200 >>Subject: [PATCH 1/2] sudo: skip rule on error instead of failing completely >> >>https://fedorahosted.org/sssd/ticket/2031 >>--- >> src/providers/ldap/sdap_sudo_cache.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >>diff --git a/src/providers/ldap/sdap_sudo_cache.c b/src/providers/ldap/sdap_sudo_cache.c >>index 3c438b9307c310cd4eec030ad628ce28a66ba726..9a2d326e0e11cf90279d8327afaf6a0ab76cac85 100644 >>--- a/src/providers/ldap/sdap_sudo_cache.c >>+++ b/src/providers/ldap/sdap_sudo_cache.c >>@@ -135,7 +135,9 @@ sdap_save_native_sudorule_list(TALLOC_CTX *mem_ctx, >> domain, map, replies[i], >> cache_timeout, now, &usn_value); >> if (ret != EOK) { >>- goto fail; >>+ DEBUG(SSSDBG_OP_FAILURE, ("Failed to save sudo rule, " >>+ "will continue with next...\n")); >>+ continue; >> } >> >> /* find highest usn */ >>-- >>1.7.11.7 >> > >> From 09546d945e2b4932550c9d267c2146ac4c901e59 Mon Sep 17 00:00:00 2001 >>From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com> >>Date: Fri, 26 Jul 2013 12:29:37 +0200 >>Subject: [PATCH 2/2] sudo: print better debug message when a rule has >> multiple cn values >> >>--- >> src/providers/ldap/sdap_sudo_cache.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >>diff --git a/src/providers/ldap/sdap_sudo_cache.c b/src/providers/ldap/sdap_sudo_cache.c >>index 9a2d326e0e11cf90279d8327afaf6a0ab76cac85..aaaa1e2aa545864c01acd160c211ecfffce874e2 100644 >>--- a/src/providers/ldap/sdap_sudo_cache.c >>+++ b/src/providers/ldap/sdap_sudo_cache.c >>@@ -69,7 +69,11 @@ sdap_save_native_sudorule(TALLOC_CTX *mem_ctx, >> >> ret = sysdb_attrs_get_string(attrs, map[SDAP_AT_SUDO_NAME].sys_name, >> &rule_name); >>- if (ret != EOK) { >>+ if (ret == ERANGE) { >>+ DEBUG(SSSDBG_OP_FAILURE, ("Warning: found rule that contains none " >>+ "or multiple CN values. It will be skipped.\n")); >>+ return ret; >>+ } else if (ret != EOK) { >> DEBUG(SSSDBG_OP_FAILURE, ("Could not get rule name [%d]: %s\n", >> ret, strerror(ret))); >> return ret; >>-- >>1.7.11.7 >> > >Can you make either of those DEBUG messages more verbose? I think we >should make it clear that we have not processed all the rules. New patches are attached. >Also I wonder if we should support multiple names by looking at RDN >value and picking the one that matches the RDN? That's what we do for >users and groups anyway. Yes, ctrianta will file a bug/rfe and we'll do that as separate patch. >What is the sudo rule name used for (except for cn=defaults) ? Only administrative purpose.

Looks good to me now. Ack!

Reply

Jakub Hrozek

9:57 a.m.

On Mon, Aug 05, 2013 at 04:55:39PM +0200, Jakub Hrozek wrote:

On Fri, Jul 26, 2013 at 01:46:04PM +0200, Pavel Březina wrote: > On 07/26/2013 01:31 PM, Jakub Hrozek wrote: > >On Fri, Jul 26, 2013 at 12:34:47PM +0200, Pavel Březina wrote: > >>https://fedorahosted.org/sssd/ticket/2031 > > > >> From fecd2799c21dc78fcb098e786b1c7e879e943c5b Mon Sep 17 00:00:00 2001 > >>From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com> > >>Date: Fri, 26 Jul 2013 12:25:01 +0200 > >>Subject: [PATCH 1/2] sudo: skip rule on error instead of failing completely > >> > >>https://fedorahosted.org/sssd/ticket/2031 > >>--- > >> src/providers/ldap/sdap_sudo_cache.c | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > >>diff --git a/src/providers/ldap/sdap_sudo_cache.c b/src/providers/ldap/sdap_sudo_cache.c > >>index 3c438b9307c310cd4eec030ad628ce28a66ba726..9a2d326e0e11cf90279d8327afaf6a0ab76cac85 100644 > >>--- a/src/providers/ldap/sdap_sudo_cache.c > >>+++ b/src/providers/ldap/sdap_sudo_cache.c > >>@@ -135,7 +135,9 @@ sdap_save_native_sudorule_list(TALLOC_CTX *mem_ctx, > >> domain, map, replies[i], > >> cache_timeout, now, &usn_value); > >> if (ret != EOK) { > >>- goto fail; > >>+ DEBUG(SSSDBG_OP_FAILURE, ("Failed to save sudo rule, " > >>+ "will continue with next...\n")); > >>+ continue; > >> } > >> > >> /* find highest usn */ > >>-- > >>1.7.11.7 > >> > > > >> From 09546d945e2b4932550c9d267c2146ac4c901e59 Mon Sep 17 00:00:00 2001 > >>From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com> > >>Date: Fri, 26 Jul 2013 12:29:37 +0200 > >>Subject: [PATCH 2/2] sudo: print better debug message when a rule has > >> multiple cn values > >> > >>--- > >> src/providers/ldap/sdap_sudo_cache.c | 6 +++++- > >> 1 file changed, 5 insertions(+), 1 deletion(-) > >> > >>diff --git a/src/providers/ldap/sdap_sudo_cache.c b/src/providers/ldap/sdap_sudo_cache.c > >>index 9a2d326e0e11cf90279d8327afaf6a0ab76cac85..aaaa1e2aa545864c01acd160c211ecfffce874e2 100644 > >>--- a/src/providers/ldap/sdap_sudo_cache.c > >>+++ b/src/providers/ldap/sdap_sudo_cache.c > >>@@ -69,7 +69,11 @@ sdap_save_native_sudorule(TALLOC_CTX *mem_ctx, > >> > >> ret = sysdb_attrs_get_string(attrs, map[SDAP_AT_SUDO_NAME].sys_name, > >> &rule_name); > >>- if (ret != EOK) { > >>+ if (ret == ERANGE) { > >>+ DEBUG(SSSDBG_OP_FAILURE, ("Warning: found rule that contains none " > >>+ "or multiple CN values. It will be skipped.\n")); > >>+ return ret; > >>+ } else if (ret != EOK) { > >> DEBUG(SSSDBG_OP_FAILURE, ("Could not get rule name [%d]: %s\n", > >> ret, strerror(ret))); > >> return ret; > >>-- > >>1.7.11.7 > >> > > > >Can you make either of those DEBUG messages more verbose? I think we > >should make it clear that we have not processed all the rules. > > New patches are attached. > > >Also I wonder if we should support multiple names by looking at RDN > >value and picking the one that matches the RDN? That's what we do for > >users and groups anyway. > > Yes, ctrianta will file a bug/rfe and we'll do that as separate patch. > > >What is the sudo rule name used for (except for cn=defaults) ? > > Only administrative purpose. > Looks good to me now. Ack!

Pushed to master and sssd-1-10

Reply

3921

days inactive

3931

days old

sssd-devel@lists.fedorahosted.org

Manage subscription

4 comments

2 participants

tags (0)

participants (2)

Jakub Hrozek
Pavel Březina