[SSSD] Announcing SSSD 1.14.0

[Jakub Hrozek]

 === SSSD 1.14.0 ===

The SSSD team is proud to announce the release of version 1.14.0 of
the System Security Services Daemon.

As always, the source is available from https://fedorahosted.org/sssd

RPM packages will be made available for Fedora shortly.

== Feedback ==
Please provide comments, bugs and other feedback via the sssd-devel
or sssd-users mailing lists:
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
https://lists.fedorahosted.org/mailman/listinfo/sssd-users

== Highlights ==
This section includes all changes since the last stable release, even
those included in the Alpha and Beta releases.

* Smart Card related enhancements
* The IPA provider allows looking up users from trusted Active Directory
  domains by certificates that are included in the IPA ID-views. Please
  note that this functionality requires a recent IPA server.
* The AD provider is now able to look up users from Active Directory
  domains by certificate. This change enables logins for Active Directory
  users with the help of a smart card.
* The sss_override tool is now able to add certificates as local
  overrides in the SSSD cache. Please note that the certificate overrides
  are stored in the local cache, so removing the cache also removes all
  the local certificates!
* Invalid certificates are skipped instead of aborting the whole
  operation when logging in with a smart card using SSH.
* This version allows several OCSP-related options such as the OCSP
  responder to be configured during smart card authentication
* SSSD is now able to determine the name of the user who logs in from
  the inserted smart card without having to type in the username. Please
  note that this functionality must be enabled with the allow_missing_name
  pam_sss option.

* Enhancements for easier administration
* The sss_cache command line tool is now able to invalidate SUDO rules
  with its new -r/-R switches. Please note that the sudo rules are not
  refreshed with the sss_cache tool immediately. Refer to the sssd-sudo
  man page for the existing refresh timeouts.
* SSSD is able to merge configuration file snippets from an include
  directory. This functionality requires the latest libini release 1.3.0.
* The GPO evaluator is able to skip malformed INI files. This feature
  is also only available with libini release 1.3.0 or newer.
* SSSD is able to validate configuration files against a built-in
  schema. To retain backwards-compatibility with configuration files
  that would otherwise not validate, the validator only warns about
  errors in the config file in this version.
* A new command line tool, called sssctl was added. This tool allows
  the administrator to observe the status of SSSD. In this version,
  the tool is able to:
- list SSSD domains and subdomains, including their online and offline 
status
- print information about objects stored in the cache
- backup or remove the local databases
- check the validity of the configuration file
- help truncate SSSD logs 

* Two-factor authentication improvements
* With a recent IPA server (4.4.x), it is now possible to authenticate
  either with OTP or with password. When a user has OTP authentication
  enabled and hits enter during the password prompt, the authentication
  will proceed with single factor only. The authentication method is
  stored in the ticket and depending on configuration of Kerberized
  services, the user will be able to only access selected service unless
  he authenticates with the second factor as well.

* Performance improvements
* Several systemtap probes were added across the SSSD codebase as well
  as example systemtap scripts that use these probes. The scripts allow
  the administrator to observe the performance of some operations such
  as saving a group or the 'id' command with systemtap.
* SSSD's cache performance was improved. SSSD now stores operational
  attributes of cache entries to a separate database with asynchronous
  writes mode, which results in substantially faster cache update times
  in most cases. Note that the performance of the initial cache write
  with an empty cache does not improve, only subsequent updates.

* Miscellanous improvements
* A new option local_negative_timeout was added. This option allows
  the admin to specify the time during which lookups for users that
  are not handled by SSSD but are present on the system (typically in
  /etc/passwd and /etc/group) and prevents repeated lookups of local
  users on the remote server during initgroups operation.
* The AD provider as well as the IPA provider part that handles AD
  users is able to use the PAC blob attached to the Kerberos ticket to
  resolve group memberships for a user if 

[SSSD] Re: [PATCH] sssctl: manual page

[Jakub Hrozek]

On Thu, Jul 07, 2016 at 07:18:55PM +0200, Michal Židek wrote:
> On 07/07/2016 07:12 PM, Michal Židek wrote:
> > On 07/07/2016 06:45 PM, Michal Židek wrote:
> > > The man page looks good to me with exception
> > > for one detail (see inline)
> > > 
> > > On 07/04/2016 12:45 PM, Pavel Březina wrote:
> > > > +
> > > > +COMMON OPTIONS
> > > > +
> > > > +Those options are available with all commands.
> > > > +
> > > > +
> > > > +
> > > > +
> > > > +--debug
> > > > +LEVEL
> > > > +
> > > > + > > > xmlns:xi="http://www.w3.org/2001/XInclude;
> > > > href="include/debug_levels.xml" />
> > > 
> > > This include contains some irrelevant information for sssctl
> > > that could be confusing for users. But I do not think we
> > > should block the patch, I will create a ticket to track this
> > > as a man page bug.
> > > 
> > > > +
> > > > +
> > > > +
> > > > +
> > > > +http://www.w3.org/2001/XInclude;
> > > > href="include/seealso.xml" />
> > > > +
> > > 
> > > The attached patch is the same as Pavel's, with some
> > > whitespaces removed (git complaint about them).
> > > 
> > > Otherwise LGTM ( == ACK, because we do not have much
> > > time and I think it is better than nothing).
> > > 
> > > Michal
> > 
> > The mailman is very slow today for some mails.
> > 
> > Sending again with the hope it will arrive sooner.
> > 
> > Michal
> 
> Third time...

Because mailman has issues today, Michal sent me the patch in a direct
mail and I pushed it:
dc6dd1ef6a70a0a07017d362e13c7680e83c4fc8
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] sssctl: manual page

[Michal Židek]

On 07/07/2016 07:12 PM, Michal Židek wrote:

On 07/07/2016 06:45 PM, Michal Židek wrote:

The man page looks good to me with exception
for one detail (see inline)

On 07/04/2016 12:45 PM, Pavel Březina wrote:

+
+COMMON OPTIONS
+
+Those options are available with all commands.
+
+
+
+
+--debug
+LEVEL
+
+http://www.w3.org/2001/XInclude;
href="include/debug_levels.xml" />


This include contains some irrelevant information for sssctl
that could be confusing for users. But I do not think we
should block the patch, I will create a ticket to track this
as a man page bug.


+
+
+
+
+http://www.w3.org/2001/XInclude;
href="include/seealso.xml" />
+


The attached patch is the same as Pavel's, with some
whitespaces removed (git complaint about them).

Otherwise LGTM ( == ACK, because we do not have much
time and I think it is better than nothing).

Michal


The mailman is very slow today for some mails.

Sending again with the hope it will arrive sooner.

Michal


Third time...
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] SSH-CERT: always initialize cert_verify_opts

[Jakub Hrozek]

On Thu, Jul 07, 2016 at 06:13:04PM +0200, Jakub Hrozek wrote:
> On Fri, Jun 17, 2016 at 02:50:36PM +0200, Sumit Bose wrote:
> > Hi,
> > 
> > please find attached two small patches related to the conversion of the
> > public key in a certificate to a public ssh-key.
> > 
> > The first fixes an issue which should only happen in master. The second
> > might be useful for already released versions as well, but I would wait
> > until we get a report that the old scheme fails with some certificates.
> > 
> > bye,
> > Sumit
> 
> ACK to both, sorry for not finding the first issue when I reviewed the
> original patch.
> 
> I also only tested the happy path with the second patch, not the issue
> that is actually fixed.

CI: http://sssd-ci.duckdns.org/logs/job/47/48/summary.html

The failure on rawhide is unrelated.

* master:
* ecd48ae244dbb6490989752fba99b58d84babfa6
* 8b2bd0587af6ed6bbd7eab7a332ec88de6b7c36c
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] SSH-CERT: always initialize cert_verify_opts

[Jakub Hrozek]

On Fri, Jun 17, 2016 at 02:50:36PM +0200, Sumit Bose wrote:
> Hi,
> 
> please find attached two small patches related to the conversion of the
> public key in a certificate to a public ssh-key.
> 
> The first fixes an issue which should only happen in master. The second
> might be useful for already released versions as well, but I would wait
> until we get a report that the old scheme fails with some certificates.
> 
> bye,
> Sumit

ACK to both, sorry for not finding the first issue when I reviewed the
original patch.

I also only tested the happy path with the second patch, not the issue
that is actually fixed.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] CONFIGURE: Inform about optional build dependencies

[Jakub Hrozek]

On Thu, Jul 07, 2016 at 06:06:26PM +0200, Jakub Hrozek wrote:
> On Thu, Jul 07, 2016 at 01:05:39PM +0200, Lukas Slebodnik wrote:
> > On (05/07/16 07:59), Jakub Hrozek wrote:
> > >On Fri, Jul 01, 2016 at 08:45:53AM +0200, Lukas Slebodnik wrote:
> > >> ehlo,
> > >>
> > >> We usually inform about optional build dependencies at configure time
> > >> and which option can disable checking of this dependency.
> > >>
> > >> LS
> > >
> > >I agree with the intent, but I would just say we need such and such
> > >library, instead of a header file. This is also what we say when the
> > >samba libraries are not found.
> > Detection of http_parser and libjansson is slit into 3 parts.
> > a) try detect with pkg-config (http_parser doesn' have it)
> > b) detect header file
> > c) check for specific function if header file is available.
> > 
> > I modified error mesasge for the 2nd step.
> > Error message for library with wrong function is different.
> >   AC_MSG_ERROR([libhttp_parser missing http_parser_init])
> > I didn't change it because hint should be added
> > to the 2nd step and not the 3rd step.
> > 
> > I choose to inform about header file because the name of package
> > is not the same on all distributions
> >  (debian:libhttp-parser-dev fedora:http-parser-devel)
> > 
> > But feel free to propose better sentences.
> > 
> > LS
> 
> I really have issues replying to sssd-devel lately, maybe third time is
> the charm -- ACK, no point in arguing about error message in configure.

* master: c5a47e4a809aca39669e26d6136f8056952efd74
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] CONFIGURE: Inform about optional build dependencies

[Jakub Hrozek]

On Thu, Jul 07, 2016 at 01:05:39PM +0200, Lukas Slebodnik wrote:
> On (05/07/16 07:59), Jakub Hrozek wrote:
> >On Fri, Jul 01, 2016 at 08:45:53AM +0200, Lukas Slebodnik wrote:
> >> ehlo,
> >>
> >> We usually inform about optional build dependencies at configure time
> >> and which option can disable checking of this dependency.
> >>
> >> LS
> >
> >I agree with the intent, but I would just say we need such and such
> >library, instead of a header file. This is also what we say when the
> >samba libraries are not found.
> Detection of http_parser and libjansson is slit into 3 parts.
> a) try detect with pkg-config (http_parser doesn' have it)
> b) detect header file
> c) check for specific function if header file is available.
> 
> I modified error mesasge for the 2nd step.
> Error message for library with wrong function is different.
>   AC_MSG_ERROR([libhttp_parser missing http_parser_init])
> I didn't change it because hint should be added
> to the 2nd step and not the 3rd step.
> 
> I choose to inform about header file because the name of package
> is not the same on all distributions
>  (debian:libhttp-parser-dev fedora:http-parser-devel)
> 
> But feel free to propose better sentences.
> 
> LS

I really have issues replying to sssd-devel lately, maybe third time is
the charm -- ACK, no point in arguing about error message in configure.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Fwd: Re: [PATCH] LDAP: Lookup services by all protocols unless a protocol is specified

[Jakub Hrozek]

sssd-devel seems to be eating our mails lately, resending

- Forwarded Message -
From: "Jakub Hrozek" 
To: sssd-devel@lists.fedorahosted.org
Sent: Thursday, July 7, 2016 4:44:30 PM
Subject: Re: [SSSD] Re: [PATCH] LDAP: Lookup services by all protocols unless a 
protocol is specified

On Thu, Jul 07, 2016 at 02:46:46PM +0200, Sumit Bose wrote:
> Looks like my mail in between got delayed, CI passed
> http://sssd-ci.duckdns.org/logs/job/47/43/summary.html
> 
> ACK

* master: aa58e216c1f794bd335151f19e79adbb3ddf4c73

I opened: https://fedorahosted.org/sssd/ticket/3084 to track the more
systematic change.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] MAN: Config file merging

[Lukas Slebodnik]

On (07/07/16 10:05), Dan Lavu wrote:
>Looks good, ACK.
>
master:
* c82789aad172d7ebd9f616510bdbe950dccd51ac

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] MAN: Config file merging

[Dan Lavu]

Looks good, ACK.

On Thu, Jul 7, 2016 at 4:39 AM, Michal Židek  wrote:

> On 07/04/2016 06:28 PM, Michal Židek wrote:
>
>> On 07/01/2016 10:34 PM, Dan Lavu wrote:
>>
>>> I couldn't apply your patch to my repo, so I just modified the text in
>>> your patch. I have a question though, will this be the new default
>>> behavior for sssd 1.4.x onwards in Fedora and RHEL? Also I think the
>>> feature, being called "merging" is not very accurate, I think it should
>>> be 'include additional configuration files' or 'read additional
>>> configuration files', merging suggests that it will merge the
>>> configuration files into one file... thanks to Git. ;)
>>>
>>> Dan
>>>
>>>
>> Hi Dan,
>>
>> thank you for the new wording.
>>
>> I agree that calling the feature config file
>> merging could be confusing, so I used the verb
>> "include" instead of "merge" and changed the section
>> name.
>>
>> I also removed the SSSD version reference as Lukas
>> suggested.
>>
>> Otherwise the wording should be the same as you
>> proposed.
>>
>> To your question. Yes, this is going to be default
>> behavior with all new SSSD versions compiled with
>> new libini.
>>
>> See the attached patch.
>>
>> Michal
>>
>>
> Sorry, I forgot to CC Dan.
>
> Michal
>
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] sssctl: Add config-check command

[Lukas Slebodnik]

On (07/07/16 15:21), Lukas Slebodnik wrote:
>On (07/07/16 14:10), Michal Židek wrote:
>>On 07/07/2016 01:54 PM, Michal Židek wrote:
>>> On 07/07/2016 01:20 PM, Lukas Slebodnik wrote:
>>> > On (07/07/16 12:37), Michal Židek wrote:
>>> > > On 07/04/2016 05:27 PM, Michal Židek wrote:
>>> > > > On 07/01/2016 05:35 PM, Lukas Slebodnik wrote:
>>> > > > > On (01/07/16 17:26), Michal Židek wrote:
>>> > > > > > Hello!
>>> > > > > > 
>>> > > > > > This patch adds new command config-check
>>> > > > > > for sssctl tool. The output looks like this:
>>> > > > > > 
>>> > > > > > 
>>> > > > > > Issues identified by validators: 3
>>> > > > > > [rule/allowed_sections]: Section [SectionFOO] is not allowed.
>>> > > > > > Check for
>>> > > > > > typos.
>>> > > > > > [rule/allowed_sssd_options]: Attribute 'unlknowww' is not allowed 
>>> > > > > > in
>>> > > > > > section
>>> > > > > > 'sssd'. Check for typos.
>>> > > > > > [rule/allowed_sssd_options]: Attribute 'unknown_option' is not
>>> > > > > > allowed in
>>> > > > > > section 'sssd'. Check for typos.
>>> > > > > > 
>>> > > > > > Messages generated during configuration merging: 4
>>> > > > > > File conf.dd did not match provided patterns. Skipping.
>>> > > > > > Errors detected while parsing: /etc/sssd/conf.d/snip-bad.conf.
>>> > > > > > Error (5) on line 5: Equal sign is missing.
>>> > > > > > Due to errors file /etc/sssd/conf.d/snip-bad.conf is not 
>>> > > > > > considered.
>>> > > > > > Skipping.
>>> > > > > > 
>>> > > > > > Used configuration snippet files: 1
>>> > > > > > /etc/sssd/conf.d/snip-good.conf
>>> > > > > > 
>>> > > > > > 
>>> > > > > > Currently it can only be used by root and checks only
>>> > > > > > configuration in default path.
>>> > > > > > 
>>> > > > > > Michal
>>> > > > > 
>>> > > > > >  From 864091da5010b75d9c38e87ab3be467b8bc6f8f2 Mon Sep 17 00:00:00
>>> > > > > > 2001
>>> > > > > > From: =?UTF-8?q?Michal=20=C5=BDidek?= 
>>> > > > > > Date: Fri, 1 Jul 2016 15:10:30 +0200
>>> > > > > > Subject: [PATCH 1/4] sss_ini: Small refacoring of
>>> > > > > > sss_ini_call_validators
>>> > > > > > 
>>> > > > > > Separate logic to fill errobj so that
>>> > > > > > the errors can be printed by the caller.
>>> > > > > > ---
>>> > > > > > src/util/sss_ini.c | 49
>>> > > > > > -
>>> > > > > > src/util/sss_ini.h | 12 
>>> > > > > > 2 files changed, 48 insertions(+), 13 deletions(-)
>>> > > > > > 
>>> > > > > > diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c
>>> > > > > > index b4dbb07..78f5490 100644
>>> > > > > > --- a/src/util/sss_ini.c
>>> > > > > > +++ b/src/util/sss_ini.c
>>> > > > > > @@ -549,7 +549,6 @@ int sss_ini_call_validators(struct
>>> > > > > > sss_ini_initdata *data,
>>> > > > > > {
>>> > > > > > #ifdef HAVE_LIBINI_CONFIG_V1_3
>>> > > > > >   int ret;
>>> > > > > > -struct ini_cfgobj *rules_cfgobj = NULL;
>>> > > > > >   struct ini_errobj *errobj = NULL;
>>> > > > > > 
>>> > > > > >   ret = ini_errobj_create();
>>> > > > > > @@ -558,17 +557,12 @@ int sss_ini_call_validators(struct
>>> > > > > > sss_ini_initdata *data,
>>> > > > > >   goto done;
>>> > > > > >   }
>>> > > > > > 
>>> > > > > > -ret = ini_rules_read_from_file(rules_path, _cfgobj);
>>> > > > > > +ret = sss_ini_call_validators_errobj(data,
>>> > > > > > + rules_path,
>>> > > > > > + errobj);
>>> > > > > >   if (ret != EOK) {
>>> > > > > > -DEBUG(SSSDBG_FATAL_FAILURE,
>>> > > > > > -  "Failed to read sssd.conf schema %d [%s]\n", ret,
>>> > > > > > strerror(ret));
>>> > > > > > -goto done;
>>> > > > > > -}
>>> > > > > > -
>>> > > > > > -ret = ini_rules_check(rules_cfgobj, data->sssd_config, NULL,
>>> > > > > > errobj);
>>> > > > > > -if (ret != EOK) {
>>> > > > > > -DEBUG(SSSDBG_FATAL_FAILURE,
>>> > > > > > -  "ini_rules_check failed %d [%s]\n", ret,
>>> > > > > > strerror(ret));
>>> > > > > > +DEBUG(SSSDBG_CRIT_FAILURE,
>>> > > > > > +  "Failed to get errors from validators.\n");
>>> > > > > >   goto done;
>>> > > > > >   }
>>> > > > > > 
>>> > > > > > @@ -579,10 +573,10 @@ int sss_ini_call_validators(struct
>>> > > > > > sss_ini_initdata *data,
>>> > > > > >   ini_errobj_next(errobj);
>>> > > > > >   }
>>> > > > > > 
>>> > > > > > +ret = EOK;
>>> > > > > > +
>>> > > > > > done:
>>> > > > > > -if (rules_cfgobj) ini_config_destroy(rules_cfgobj);
>>> > > > > >   ini_errobj_destroy();
>>> > > > > > -
>>> > > > > >   return ret;
>>> > > > > > #else
>>> > > > > >   DEBUG(SSSDBG_TRACE_FUNC,
>>> > > > > > @@ -590,3 +584,32 @@ done:
>>> > > > > >   return EOK;
>>> > > > > > #endif /* HAVE_LIBINI_CONFIG_V1_3 */
>>> > > > > > }
>>> > > > > > +
>>> > > > > > +#ifdef HAVE_LIBINI_CONFIG_V1_3
>>> > > > > > +int sss_ini_call_validators_errobj(struct sss_ini_initdata *data,
>>> > 

[SSSD] Re: [PATCH] sssctl: Add config-check command

[Lukas Slebodnik]

On (07/07/16 14:10), Michal Židek wrote:
>On 07/07/2016 01:54 PM, Michal Židek wrote:
>> On 07/07/2016 01:20 PM, Lukas Slebodnik wrote:
>> > On (07/07/16 12:37), Michal Židek wrote:
>> > > On 07/04/2016 05:27 PM, Michal Židek wrote:
>> > > > On 07/01/2016 05:35 PM, Lukas Slebodnik wrote:
>> > > > > On (01/07/16 17:26), Michal Židek wrote:
>> > > > > > Hello!
>> > > > > > 
>> > > > > > This patch adds new command config-check
>> > > > > > for sssctl tool. The output looks like this:
>> > > > > > 
>> > > > > > 
>> > > > > > Issues identified by validators: 3
>> > > > > > [rule/allowed_sections]: Section [SectionFOO] is not allowed.
>> > > > > > Check for
>> > > > > > typos.
>> > > > > > [rule/allowed_sssd_options]: Attribute 'unlknowww' is not allowed 
>> > > > > > in
>> > > > > > section
>> > > > > > 'sssd'. Check for typos.
>> > > > > > [rule/allowed_sssd_options]: Attribute 'unknown_option' is not
>> > > > > > allowed in
>> > > > > > section 'sssd'. Check for typos.
>> > > > > > 
>> > > > > > Messages generated during configuration merging: 4
>> > > > > > File conf.dd did not match provided patterns. Skipping.
>> > > > > > Errors detected while parsing: /etc/sssd/conf.d/snip-bad.conf.
>> > > > > > Error (5) on line 5: Equal sign is missing.
>> > > > > > Due to errors file /etc/sssd/conf.d/snip-bad.conf is not 
>> > > > > > considered.
>> > > > > > Skipping.
>> > > > > > 
>> > > > > > Used configuration snippet files: 1
>> > > > > > /etc/sssd/conf.d/snip-good.conf
>> > > > > > 
>> > > > > > 
>> > > > > > Currently it can only be used by root and checks only
>> > > > > > configuration in default path.
>> > > > > > 
>> > > > > > Michal
>> > > > > 
>> > > > > >  From 864091da5010b75d9c38e87ab3be467b8bc6f8f2 Mon Sep 17 00:00:00
>> > > > > > 2001
>> > > > > > From: =?UTF-8?q?Michal=20=C5=BDidek?= 
>> > > > > > Date: Fri, 1 Jul 2016 15:10:30 +0200
>> > > > > > Subject: [PATCH 1/4] sss_ini: Small refacoring of
>> > > > > > sss_ini_call_validators
>> > > > > > 
>> > > > > > Separate logic to fill errobj so that
>> > > > > > the errors can be printed by the caller.
>> > > > > > ---
>> > > > > > src/util/sss_ini.c | 49
>> > > > > > -
>> > > > > > src/util/sss_ini.h | 12 
>> > > > > > 2 files changed, 48 insertions(+), 13 deletions(-)
>> > > > > > 
>> > > > > > diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c
>> > > > > > index b4dbb07..78f5490 100644
>> > > > > > --- a/src/util/sss_ini.c
>> > > > > > +++ b/src/util/sss_ini.c
>> > > > > > @@ -549,7 +549,6 @@ int sss_ini_call_validators(struct
>> > > > > > sss_ini_initdata *data,
>> > > > > > {
>> > > > > > #ifdef HAVE_LIBINI_CONFIG_V1_3
>> > > > > >   int ret;
>> > > > > > -struct ini_cfgobj *rules_cfgobj = NULL;
>> > > > > >   struct ini_errobj *errobj = NULL;
>> > > > > > 
>> > > > > >   ret = ini_errobj_create();
>> > > > > > @@ -558,17 +557,12 @@ int sss_ini_call_validators(struct
>> > > > > > sss_ini_initdata *data,
>> > > > > >   goto done;
>> > > > > >   }
>> > > > > > 
>> > > > > > -ret = ini_rules_read_from_file(rules_path, _cfgobj);
>> > > > > > +ret = sss_ini_call_validators_errobj(data,
>> > > > > > + rules_path,
>> > > > > > + errobj);
>> > > > > >   if (ret != EOK) {
>> > > > > > -DEBUG(SSSDBG_FATAL_FAILURE,
>> > > > > > -  "Failed to read sssd.conf schema %d [%s]\n", ret,
>> > > > > > strerror(ret));
>> > > > > > -goto done;
>> > > > > > -}
>> > > > > > -
>> > > > > > -ret = ini_rules_check(rules_cfgobj, data->sssd_config, NULL,
>> > > > > > errobj);
>> > > > > > -if (ret != EOK) {
>> > > > > > -DEBUG(SSSDBG_FATAL_FAILURE,
>> > > > > > -  "ini_rules_check failed %d [%s]\n", ret,
>> > > > > > strerror(ret));
>> > > > > > +DEBUG(SSSDBG_CRIT_FAILURE,
>> > > > > > +  "Failed to get errors from validators.\n");
>> > > > > >   goto done;
>> > > > > >   }
>> > > > > > 
>> > > > > > @@ -579,10 +573,10 @@ int sss_ini_call_validators(struct
>> > > > > > sss_ini_initdata *data,
>> > > > > >   ini_errobj_next(errobj);
>> > > > > >   }
>> > > > > > 
>> > > > > > +ret = EOK;
>> > > > > > +
>> > > > > > done:
>> > > > > > -if (rules_cfgobj) ini_config_destroy(rules_cfgobj);
>> > > > > >   ini_errobj_destroy();
>> > > > > > -
>> > > > > >   return ret;
>> > > > > > #else
>> > > > > >   DEBUG(SSSDBG_TRACE_FUNC,
>> > > > > > @@ -590,3 +584,32 @@ done:
>> > > > > >   return EOK;
>> > > > > > #endif /* HAVE_LIBINI_CONFIG_V1_3 */
>> > > > > > }
>> > > > > > +
>> > > > > > +#ifdef HAVE_LIBINI_CONFIG_V1_3
>> > > > > > +int sss_ini_call_validators_errobj(struct sss_ini_initdata *data,
>> > > > > > +   const char *rules_path,
>> > > > > > +   struct ini_errobj *errobj)
>> > > > > > +{
>> 

[SSSD] Re: [PATCH] LDAP: Lookup services by all protocols unless a protocol is specified

[Sumit Bose]

On Thu, Jul 07, 2016 at 01:26:13PM +0200, Pavel Březina wrote:
> On 07/07/2016 01:24 PM, Jakub Hrozek wrote:
> > On Thu, Jul 07, 2016 at 12:39:28PM +0200, Pavel Březina wrote:
> > > On 07/07/2016 12:34 PM, Jakub Hrozek wrote:
> > > > On Thu, Jul 07, 2016 at 10:16:03AM +0200, Sumit Bose wrote:
> > > > > resend
> > > > > - Forwarded message from Sumit Bose  -
> > > > > 
> > > > > Date: Wed, 6 Jul 2016 11:13:48 +0200
> > > > > From: Sumit Bose 
> > > > > To: sssd-devel@lists.fedorahosted.org
> > > > > Subject: Re: [SSSD] [PATCH] LDAP: Lookup services by all protocols 
> > > > > unless a
> > > > >   protocol is specified
> > > > > Message-ID: 
> > > > > <20160706091348.GD29143@p.Speedport_W_724V_Typ_A_05011603_00_009>
> > > > > References: <20160705103025.GB24232@hendrix>
> > > > > MIME-Version: 1.0
> > > > > Content-Type: text/plain; charset=us-ascii
> > > > > Content-Disposition: inline
> > > > > In-Reply-To: <20160705103025.GB24232@hendrix>
> > > > > User-Agent: Mutt/1.6.1 (2016-04-27)
> > > > > 
> > > > > On Tue, Jul 05, 2016 at 12:30:25PM +0200, Jakub Hrozek wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > the attached patch makes service lookups great again.
> > > > > > 
> > > > > > To reproduce, just run:
> > > > > >   getent service -s sss ldap
> > > > > > before the patch we would look up ipService="" because DP gives us 
> > > > > > an
> > > > > > empty string after the recent DP patches.
> > > > > 
> > > > > >   From ba9834637b3cc0d7d98f704ba70f9dcb6f9a70e9 Mon Sep 17 00:00:00 
> > > > > > 2001
> > > > > > From: Jakub Hrozek 
> > > > > > Date: Tue, 5 Jul 2016 12:23:23 +0200
> > > > > > Subject: [PATCH] LDAP: Lookup services by all protocols unless a 
> > > > > > protocol is
> > > > > >specified
> > > > > > 
> > > > > > The DP refactoring changed the way we handle strings from sbus. We 
> > > > > > no
> > > > > > longer receive NULL strings, but empty strings instead.
> > > > > > ---
> > > > > >src/providers/ldap/ldap_id_services.c | 2 +-
> > > > > >1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/src/providers/ldap/ldap_id_services.c 
> > > > > > b/src/providers/ldap/ldap_id_services.c
> > > > > > index 
> > > > > > 77215127b53297d840eaa4d2f35a75eedb085e43..db47e3fc55eea969371d61c3c5ac7f818196f3d5
> > > > > >  100644
> > > > > > --- a/src/providers/ldap/ldap_id_services.c
> > > > > > +++ b/src/providers/ldap/ldap_id_services.c
> > > > > > @@ -114,7 +114,7 @@ services_get_send(TALLOC_CTX *mem_ctx,
> > > > > >ret = sss_filter_sanitize(state, name, _name);
> > > > > >if (ret != EOK)  goto error;
> > > > > > 
> > > > > > -if (protocol) {
> > > > > > +if (protocol && protocol[0] != '\0') {
> > > > > >ret = sss_filter_sanitize(state, protocol, 
> > > > > > _protocol);
> > > > > >if (ret != EOK)  goto error;
> > > > > >}
> > > > > 
> > > > > since the sysdb calls later on in the request expect protocol==NULL as
> > > > > well I would suggest something like
> > > > > 
> > > > >state->sysdb = sdom->dom->sysdb;
> > > > >state->name = name;
> > > > >state->protocol = protocol;
> > > > > +if (state->protocol[0] == '\0') {
> > > > > +state->protocol = NULL;
> > > > > +}
> > > > >state->filter_type = filter_type;
> > > > >state->noexist_delete = noexist_delete;
> > > > > 
> > > > > 
> > > > > in only use state->protocol in the request.
> > > > 
> > > > OK, done (we use state->protocol up to a point, then clean_protocol). I
> > > > also added a check to cover the unlikely situation where sbus might give
> > > > us a NULL string, just to error and don't crash.
> > > 
> > > Hi, nice catch. I think it may be better to amend check_and_parse_filter()
> > > in dp_target_id.c so it assign NULL instead of "" in data->filter_value 
> > > (and
> > > other) using SBUS_SET_STRING(). Though I didn't check if other look up 
> > > types
> > > (user/group/...) don't expect empty string instead.
> > 
> > Yes, this makes sense and a quick git grep shows that the providers
> > expect NULL, not empty string (for example for enumeration which we
> > already special-case in check_and_parse_filter). But at this point,
> > I would prefer to push this patch, release 1.14.0 and then patch
> > check_and_parse_filter() and run tests again to make sure we don't break
> > some strange corner case with a late sbus fix.
> > 
> > Attached is a new patch that doesn't error out if protocol is NULL.
> 
> Ok, I'm fine with this. The code looks good to me.

Looks like my mail in between got delayed, CI passed
http://sssd-ci.duckdns.org/logs/job/47/43/summary.html

ACK

bye,
Sumit

> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
___

[SSSD] Re: [PATCH] sssctl: Add config-check command

[Michal Židek]

On 07/07/2016 02:15 PM, Pavel Březina wrote:

On 07/07/2016 01:54 PM, Michal Židek wrote:

On 07/07/2016 01:20 PM, Lukas Slebodnik wrote:

On (07/07/16 12:37), Michal Židek wrote:

On 07/04/2016 05:27 PM, Michal Židek wrote:

On 07/01/2016 05:35 PM, Lukas Slebodnik wrote:

On (01/07/16 17:26), Michal Židek wrote:

Hello!

This patch adds new command config-check
for sssctl tool. The output looks like this:


Issues identified by validators: 3
[rule/allowed_sections]: Section [SectionFOO] is not allowed.
Check for
typos.
[rule/allowed_sssd_options]: Attribute 'unlknowww' is not allowed in
section
'sssd'. Check for typos.
[rule/allowed_sssd_options]: Attribute 'unknown_option' is not
allowed in
section 'sssd'. Check for typos.

Messages generated during configuration merging: 4
File conf.dd did not match provided patterns. Skipping.
Errors detected while parsing: /etc/sssd/conf.d/snip-bad.conf.
Error (5) on line 5: Equal sign is missing.
Due to errors file /etc/sssd/conf.d/snip-bad.conf is not considered.
Skipping.

Used configuration snippet files: 1
/etc/sssd/conf.d/snip-good.conf


Currently it can only be used by root and checks only
configuration in default path.

Michal



 From 864091da5010b75d9c38e87ab3be467b8bc6f8f2 Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
Date: Fri, 1 Jul 2016 15:10:30 +0200
Subject: [PATCH 1/4] sss_ini: Small refacoring of
sss_ini_call_validators

Separate logic to fill errobj so that
the errors can be printed by the caller.
---
src/util/sss_ini.c | 49
-
src/util/sss_ini.h | 12 
2 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c
index b4dbb07..78f5490 100644
--- a/src/util/sss_ini.c
+++ b/src/util/sss_ini.c
@@ -549,7 +549,6 @@ int sss_ini_call_validators(struct
sss_ini_initdata *data,
{
#ifdef HAVE_LIBINI_CONFIG_V1_3
  int ret;
-struct ini_cfgobj *rules_cfgobj = NULL;
  struct ini_errobj *errobj = NULL;

  ret = ini_errobj_create();
@@ -558,17 +557,12 @@ int sss_ini_call_validators(struct
sss_ini_initdata *data,
  goto done;
  }

-ret = ini_rules_read_from_file(rules_path, _cfgobj);
+ret = sss_ini_call_validators_errobj(data,
+ rules_path,
+ errobj);
  if (ret != EOK) {
-DEBUG(SSSDBG_FATAL_FAILURE,
-  "Failed to read sssd.conf schema %d [%s]\n", ret,
strerror(ret));
-goto done;
-}
-
-ret = ini_rules_check(rules_cfgobj, data->sssd_config, NULL,
errobj);
-if (ret != EOK) {
-DEBUG(SSSDBG_FATAL_FAILURE,
-  "ini_rules_check failed %d [%s]\n", ret,
strerror(ret));
+DEBUG(SSSDBG_CRIT_FAILURE,
+  "Failed to get errors from validators.\n");
  goto done;
  }

@@ -579,10 +573,10 @@ int sss_ini_call_validators(struct
sss_ini_initdata *data,
  ini_errobj_next(errobj);
  }

+ret = EOK;
+
done:
-if (rules_cfgobj) ini_config_destroy(rules_cfgobj);
  ini_errobj_destroy();
-
  return ret;
#else
  DEBUG(SSSDBG_TRACE_FUNC,
@@ -590,3 +584,32 @@ done:
  return EOK;
#endif /* HAVE_LIBINI_CONFIG_V1_3 */
}
+
+#ifdef HAVE_LIBINI_CONFIG_V1_3
+int sss_ini_call_validators_errobj(struct sss_ini_initdata *data,
+   const char *rules_path,
+   struct ini_errobj *errobj)
+{
+int ret;
+struct ini_cfgobj *rules_cfgobj = NULL;
+
+ret = ini_rules_read_from_file(rules_path, _cfgobj);
+if (ret != EOK) {
+DEBUG(SSSDBG_FATAL_FAILURE,
+  "Failed to read sssd.conf schema %d [%s]\n", ret,
strerror(ret));
+goto done;
+}
+
+ret = ini_rules_check(rules_cfgobj, data->sssd_config, NULL,
errobj);
+if (ret != EOK) {
+DEBUG(SSSDBG_FATAL_FAILURE,
+  "ini_rules_check failed %d [%s]\n", ret,
strerror(ret));
+goto done;
+}
+
+done:
+if (rules_cfgobj) ini_config_destroy(rules_cfgobj);
+
+return ret;
+}
+#endif /* HAVE_LIBINI_CONFIG_V1_3 */
diff --git a/src/util/sss_ini.h b/src/util/sss_ini.h
index 7734bab..77943d6 100644
--- a/src/util/sss_ini.h
+++ b/src/util/sss_ini.h
@@ -27,6 +27,10 @@
#ifndef __SSS_INI_H__
#define __SSS_INI_H__

+#ifdef HAVE_LIBINI_CONFIG_V1_3
+#include 
+#endif
+
/* Structure declarations */

/* INI data structure */
@@ -83,4 +87,12 @@ int sss_confdb_create_ldif(TALLOC_CTX *mem_ctx,
int sss_ini_call_validators(struct sss_ini_initdata *data,
  const char *rules_path);

+#ifdef HAVE_LIBINI_CONFIG_V1_3
+/* Get errors from validators with ini_errobj */
+int sss_ini_call_validators_errobj(struct sss_ini_initdata *data,
+   const char *rules_path,
+   struct ini_errobj *errobj);
+#endif /* HAVE_LIBINI_CONFIG_V1_3 */
+

NACK

src/util/sss_ini.h must not contain conditional 

[SSSD] Re: [PATCH] sssctl: Add config-check command

[Pavel Březina]

On 07/07/2016 01:54 PM, Michal Židek wrote:

On 07/07/2016 01:20 PM, Lukas Slebodnik wrote:

On (07/07/16 12:37), Michal Židek wrote:

On 07/04/2016 05:27 PM, Michal Židek wrote:

On 07/01/2016 05:35 PM, Lukas Slebodnik wrote:

On (01/07/16 17:26), Michal Židek wrote:

Hello!

This patch adds new command config-check
for sssctl tool. The output looks like this:


Issues identified by validators: 3
[rule/allowed_sections]: Section [SectionFOO] is not allowed.
Check for
typos.
[rule/allowed_sssd_options]: Attribute 'unlknowww' is not allowed in
section
'sssd'. Check for typos.
[rule/allowed_sssd_options]: Attribute 'unknown_option' is not
allowed in
section 'sssd'. Check for typos.

Messages generated during configuration merging: 4
File conf.dd did not match provided patterns. Skipping.
Errors detected while parsing: /etc/sssd/conf.d/snip-bad.conf.
Error (5) on line 5: Equal sign is missing.
Due to errors file /etc/sssd/conf.d/snip-bad.conf is not considered.
Skipping.

Used configuration snippet files: 1
/etc/sssd/conf.d/snip-good.conf


Currently it can only be used by root and checks only
configuration in default path.

Michal



 From 864091da5010b75d9c38e87ab3be467b8bc6f8f2 Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
Date: Fri, 1 Jul 2016 15:10:30 +0200
Subject: [PATCH 1/4] sss_ini: Small refacoring of
sss_ini_call_validators

Separate logic to fill errobj so that
the errors can be printed by the caller.
---
src/util/sss_ini.c | 49
-
src/util/sss_ini.h | 12 
2 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c
index b4dbb07..78f5490 100644
--- a/src/util/sss_ini.c
+++ b/src/util/sss_ini.c
@@ -549,7 +549,6 @@ int sss_ini_call_validators(struct
sss_ini_initdata *data,
{
#ifdef HAVE_LIBINI_CONFIG_V1_3
  int ret;
-struct ini_cfgobj *rules_cfgobj = NULL;
  struct ini_errobj *errobj = NULL;

  ret = ini_errobj_create();
@@ -558,17 +557,12 @@ int sss_ini_call_validators(struct
sss_ini_initdata *data,
  goto done;
  }

-ret = ini_rules_read_from_file(rules_path, _cfgobj);
+ret = sss_ini_call_validators_errobj(data,
+ rules_path,
+ errobj);
  if (ret != EOK) {
-DEBUG(SSSDBG_FATAL_FAILURE,
-  "Failed to read sssd.conf schema %d [%s]\n", ret,
strerror(ret));
-goto done;
-}
-
-ret = ini_rules_check(rules_cfgobj, data->sssd_config, NULL,
errobj);
-if (ret != EOK) {
-DEBUG(SSSDBG_FATAL_FAILURE,
-  "ini_rules_check failed %d [%s]\n", ret,
strerror(ret));
+DEBUG(SSSDBG_CRIT_FAILURE,
+  "Failed to get errors from validators.\n");
  goto done;
  }

@@ -579,10 +573,10 @@ int sss_ini_call_validators(struct
sss_ini_initdata *data,
  ini_errobj_next(errobj);
  }

+ret = EOK;
+
done:
-if (rules_cfgobj) ini_config_destroy(rules_cfgobj);
  ini_errobj_destroy();
-
  return ret;
#else
  DEBUG(SSSDBG_TRACE_FUNC,
@@ -590,3 +584,32 @@ done:
  return EOK;
#endif /* HAVE_LIBINI_CONFIG_V1_3 */
}
+
+#ifdef HAVE_LIBINI_CONFIG_V1_3
+int sss_ini_call_validators_errobj(struct sss_ini_initdata *data,
+   const char *rules_path,
+   struct ini_errobj *errobj)
+{
+int ret;
+struct ini_cfgobj *rules_cfgobj = NULL;
+
+ret = ini_rules_read_from_file(rules_path, _cfgobj);
+if (ret != EOK) {
+DEBUG(SSSDBG_FATAL_FAILURE,
+  "Failed to read sssd.conf schema %d [%s]\n", ret,
strerror(ret));
+goto done;
+}
+
+ret = ini_rules_check(rules_cfgobj, data->sssd_config, NULL,
errobj);
+if (ret != EOK) {
+DEBUG(SSSDBG_FATAL_FAILURE,
+  "ini_rules_check failed %d [%s]\n", ret,
strerror(ret));
+goto done;
+}
+
+done:
+if (rules_cfgobj) ini_config_destroy(rules_cfgobj);
+
+return ret;
+}
+#endif /* HAVE_LIBINI_CONFIG_V1_3 */
diff --git a/src/util/sss_ini.h b/src/util/sss_ini.h
index 7734bab..77943d6 100644
--- a/src/util/sss_ini.h
+++ b/src/util/sss_ini.h
@@ -27,6 +27,10 @@
#ifndef __SSS_INI_H__
#define __SSS_INI_H__

+#ifdef HAVE_LIBINI_CONFIG_V1_3
+#include 
+#endif
+
/* Structure declarations */

/* INI data structure */
@@ -83,4 +87,12 @@ int sss_confdb_create_ldif(TALLOC_CTX *mem_ctx,
int sss_ini_call_validators(struct sss_ini_initdata *data,
  const char *rules_path);

+#ifdef HAVE_LIBINI_CONFIG_V1_3
+/* Get errors from validators with ini_errobj */
+int sss_ini_call_validators_errobj(struct sss_ini_initdata *data,
+   const char *rules_path,
+   struct ini_errobj *errobj);
+#endif /* HAVE_LIBINI_CONFIG_V1_3 */
+

NACK

src/util/sss_ini.h must not contain conditional build.
That's the putpose of implementation file.

[SSSD] Re: [PATCH] sssctl: Add config-check command

[Michal Židek]

On 07/07/2016 01:54 PM, Michal Židek wrote:

On 07/07/2016 01:20 PM, Lukas Slebodnik wrote:

On (07/07/16 12:37), Michal Židek wrote:

On 07/04/2016 05:27 PM, Michal Židek wrote:

On 07/01/2016 05:35 PM, Lukas Slebodnik wrote:

On (01/07/16 17:26), Michal Židek wrote:

Hello!

This patch adds new command config-check
for sssctl tool. The output looks like this:


Issues identified by validators: 3
[rule/allowed_sections]: Section [SectionFOO] is not allowed.
Check for
typos.
[rule/allowed_sssd_options]: Attribute 'unlknowww' is not allowed in
section
'sssd'. Check for typos.
[rule/allowed_sssd_options]: Attribute 'unknown_option' is not
allowed in
section 'sssd'. Check for typos.

Messages generated during configuration merging: 4
File conf.dd did not match provided patterns. Skipping.
Errors detected while parsing: /etc/sssd/conf.d/snip-bad.conf.
Error (5) on line 5: Equal sign is missing.
Due to errors file /etc/sssd/conf.d/snip-bad.conf is not considered.
Skipping.

Used configuration snippet files: 1
/etc/sssd/conf.d/snip-good.conf


Currently it can only be used by root and checks only
configuration in default path.

Michal



 From 864091da5010b75d9c38e87ab3be467b8bc6f8f2 Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
Date: Fri, 1 Jul 2016 15:10:30 +0200
Subject: [PATCH 1/4] sss_ini: Small refacoring of
sss_ini_call_validators

Separate logic to fill errobj so that
the errors can be printed by the caller.
---
src/util/sss_ini.c | 49
-
src/util/sss_ini.h | 12 
2 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c
index b4dbb07..78f5490 100644
--- a/src/util/sss_ini.c
+++ b/src/util/sss_ini.c
@@ -549,7 +549,6 @@ int sss_ini_call_validators(struct
sss_ini_initdata *data,
{
#ifdef HAVE_LIBINI_CONFIG_V1_3
  int ret;
-struct ini_cfgobj *rules_cfgobj = NULL;
  struct ini_errobj *errobj = NULL;

  ret = ini_errobj_create();
@@ -558,17 +557,12 @@ int sss_ini_call_validators(struct
sss_ini_initdata *data,
  goto done;
  }

-ret = ini_rules_read_from_file(rules_path, _cfgobj);
+ret = sss_ini_call_validators_errobj(data,
+ rules_path,
+ errobj);
  if (ret != EOK) {
-DEBUG(SSSDBG_FATAL_FAILURE,
-  "Failed to read sssd.conf schema %d [%s]\n", ret,
strerror(ret));
-goto done;
-}
-
-ret = ini_rules_check(rules_cfgobj, data->sssd_config, NULL,
errobj);
-if (ret != EOK) {
-DEBUG(SSSDBG_FATAL_FAILURE,
-  "ini_rules_check failed %d [%s]\n", ret,
strerror(ret));
+DEBUG(SSSDBG_CRIT_FAILURE,
+  "Failed to get errors from validators.\n");
  goto done;
  }

@@ -579,10 +573,10 @@ int sss_ini_call_validators(struct
sss_ini_initdata *data,
  ini_errobj_next(errobj);
  }

+ret = EOK;
+
done:
-if (rules_cfgobj) ini_config_destroy(rules_cfgobj);
  ini_errobj_destroy();
-
  return ret;
#else
  DEBUG(SSSDBG_TRACE_FUNC,
@@ -590,3 +584,32 @@ done:
  return EOK;
#endif /* HAVE_LIBINI_CONFIG_V1_3 */
}
+
+#ifdef HAVE_LIBINI_CONFIG_V1_3
+int sss_ini_call_validators_errobj(struct sss_ini_initdata *data,
+   const char *rules_path,
+   struct ini_errobj *errobj)
+{
+int ret;
+struct ini_cfgobj *rules_cfgobj = NULL;
+
+ret = ini_rules_read_from_file(rules_path, _cfgobj);
+if (ret != EOK) {
+DEBUG(SSSDBG_FATAL_FAILURE,
+  "Failed to read sssd.conf schema %d [%s]\n", ret,
strerror(ret));
+goto done;
+}
+
+ret = ini_rules_check(rules_cfgobj, data->sssd_config, NULL,
errobj);
+if (ret != EOK) {
+DEBUG(SSSDBG_FATAL_FAILURE,
+  "ini_rules_check failed %d [%s]\n", ret,
strerror(ret));
+goto done;
+}
+
+done:
+if (rules_cfgobj) ini_config_destroy(rules_cfgobj);
+
+return ret;
+}
+#endif /* HAVE_LIBINI_CONFIG_V1_3 */
diff --git a/src/util/sss_ini.h b/src/util/sss_ini.h
index 7734bab..77943d6 100644
--- a/src/util/sss_ini.h
+++ b/src/util/sss_ini.h
@@ -27,6 +27,10 @@
#ifndef __SSS_INI_H__
#define __SSS_INI_H__

+#ifdef HAVE_LIBINI_CONFIG_V1_3
+#include 
+#endif
+
/* Structure declarations */

/* INI data structure */
@@ -83,4 +87,12 @@ int sss_confdb_create_ldif(TALLOC_CTX *mem_ctx,
int sss_ini_call_validators(struct sss_ini_initdata *data,
  const char *rules_path);

+#ifdef HAVE_LIBINI_CONFIG_V1_3
+/* Get errors from validators with ini_errobj */
+int sss_ini_call_validators_errobj(struct sss_ini_initdata *data,
+   const char *rules_path,
+   struct ini_errobj *errobj);
+#endif /* HAVE_LIBINI_CONFIG_V1_3 */
+

NACK

src/util/sss_ini.h must not contain conditional build.
That's the putpose of implementation file.

[SSSD] Re: [PATCH] sssctl: Add config-check command

[Michal Židek]

On 07/07/2016 01:20 PM, Lukas Slebodnik wrote:

On (07/07/16 12:37), Michal Židek wrote:

On 07/04/2016 05:27 PM, Michal Židek wrote:

On 07/01/2016 05:35 PM, Lukas Slebodnik wrote:

On (01/07/16 17:26), Michal Židek wrote:

Hello!

This patch adds new command config-check
for sssctl tool. The output looks like this:


Issues identified by validators: 3
[rule/allowed_sections]: Section [SectionFOO] is not allowed. Check for
typos.
[rule/allowed_sssd_options]: Attribute 'unlknowww' is not allowed in
section
'sssd'. Check for typos.
[rule/allowed_sssd_options]: Attribute 'unknown_option' is not
allowed in
section 'sssd'. Check for typos.

Messages generated during configuration merging: 4
File conf.dd did not match provided patterns. Skipping.
Errors detected while parsing: /etc/sssd/conf.d/snip-bad.conf.
Error (5) on line 5: Equal sign is missing.
Due to errors file /etc/sssd/conf.d/snip-bad.conf is not considered.
Skipping.

Used configuration snippet files: 1
/etc/sssd/conf.d/snip-good.conf


Currently it can only be used by root and checks only
configuration in default path.

Michal



 From 864091da5010b75d9c38e87ab3be467b8bc6f8f2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
Date: Fri, 1 Jul 2016 15:10:30 +0200
Subject: [PATCH 1/4] sss_ini: Small refacoring of
sss_ini_call_validators

Separate logic to fill errobj so that
the errors can be printed by the caller.
---
src/util/sss_ini.c | 49
-
src/util/sss_ini.h | 12 
2 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c
index b4dbb07..78f5490 100644
--- a/src/util/sss_ini.c
+++ b/src/util/sss_ini.c
@@ -549,7 +549,6 @@ int sss_ini_call_validators(struct
sss_ini_initdata *data,
{
#ifdef HAVE_LIBINI_CONFIG_V1_3
  int ret;
-struct ini_cfgobj *rules_cfgobj = NULL;
  struct ini_errobj *errobj = NULL;

  ret = ini_errobj_create();
@@ -558,17 +557,12 @@ int sss_ini_call_validators(struct
sss_ini_initdata *data,
  goto done;
  }

-ret = ini_rules_read_from_file(rules_path, _cfgobj);
+ret = sss_ini_call_validators_errobj(data,
+ rules_path,
+ errobj);
  if (ret != EOK) {
-DEBUG(SSSDBG_FATAL_FAILURE,
-  "Failed to read sssd.conf schema %d [%s]\n", ret,
strerror(ret));
-goto done;
-}
-
-ret = ini_rules_check(rules_cfgobj, data->sssd_config, NULL,
errobj);
-if (ret != EOK) {
-DEBUG(SSSDBG_FATAL_FAILURE,
-  "ini_rules_check failed %d [%s]\n", ret, strerror(ret));
+DEBUG(SSSDBG_CRIT_FAILURE,
+  "Failed to get errors from validators.\n");
  goto done;
  }

@@ -579,10 +573,10 @@ int sss_ini_call_validators(struct
sss_ini_initdata *data,
  ini_errobj_next(errobj);
  }

+ret = EOK;
+
done:
-if (rules_cfgobj) ini_config_destroy(rules_cfgobj);
  ini_errobj_destroy();
-
  return ret;
#else
  DEBUG(SSSDBG_TRACE_FUNC,
@@ -590,3 +584,32 @@ done:
  return EOK;
#endif /* HAVE_LIBINI_CONFIG_V1_3 */
}
+
+#ifdef HAVE_LIBINI_CONFIG_V1_3
+int sss_ini_call_validators_errobj(struct sss_ini_initdata *data,
+   const char *rules_path,
+   struct ini_errobj *errobj)
+{
+int ret;
+struct ini_cfgobj *rules_cfgobj = NULL;
+
+ret = ini_rules_read_from_file(rules_path, _cfgobj);
+if (ret != EOK) {
+DEBUG(SSSDBG_FATAL_FAILURE,
+  "Failed to read sssd.conf schema %d [%s]\n", ret,
strerror(ret));
+goto done;
+}
+
+ret = ini_rules_check(rules_cfgobj, data->sssd_config, NULL,
errobj);
+if (ret != EOK) {
+DEBUG(SSSDBG_FATAL_FAILURE,
+  "ini_rules_check failed %d [%s]\n", ret, strerror(ret));
+goto done;
+}
+
+done:
+if (rules_cfgobj) ini_config_destroy(rules_cfgobj);
+
+return ret;
+}
+#endif /* HAVE_LIBINI_CONFIG_V1_3 */
diff --git a/src/util/sss_ini.h b/src/util/sss_ini.h
index 7734bab..77943d6 100644
--- a/src/util/sss_ini.h
+++ b/src/util/sss_ini.h
@@ -27,6 +27,10 @@
#ifndef __SSS_INI_H__
#define __SSS_INI_H__

+#ifdef HAVE_LIBINI_CONFIG_V1_3
+#include 
+#endif
+
/* Structure declarations */

/* INI data structure */
@@ -83,4 +87,12 @@ int sss_confdb_create_ldif(TALLOC_CTX *mem_ctx,
int sss_ini_call_validators(struct sss_ini_initdata *data,
  const char *rules_path);

+#ifdef HAVE_LIBINI_CONFIG_V1_3
+/* Get errors from validators with ini_errobj */
+int sss_ini_call_validators_errobj(struct sss_ini_initdata *data,
+   const char *rules_path,
+   struct ini_errobj *errobj);
+#endif /* HAVE_LIBINI_CONFIG_V1_3 */
+

NACK

src/util/sss_ini.h must not contain conditional build.
That's the putpose of implementation file.

Header file shoudl contain just a 

[SSSD] Re: [PATCH] sudo: solve problems with fully qualified names

[Jakub Hrozek]

On Wed, Jul 06, 2016 at 06:20:00PM +0200, Jakub Hrozek wrote:
> On Wed, Jul 06, 2016 at 03:23:26PM +0200, Jakub Hrozek wrote:
> > On Wed, Jun 01, 2016 at 11:52:44AM +0200, Pavel Březina wrote:
> > > On 05/31/2016 01:44 PM, Jakub Hrozek wrote:
> > > > On Fri, May 27, 2016 at 11:54:20AM +0200, Pavel Březina wrote:
> > > > > See commit message for details.
> > > > > 
> > > > > Two configurations needs to be tested -- a domain with
> > > > > use_fully_qualified_name = true and configuration with IPA-AD trusts 
> > > > > where
> > > > > default_domain_suffix is set to AD domain.
> > > > 
> > > > >  From 25f8cb5101f824c53df526b2ab52b8c67dd72539 Mon Sep 17 00:00:00 
> > > > > 2001
> > > > > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> > > > > Date: Thu, 26 May 2016 11:37:30 +0200
> > > > > Subject: [PATCH] sudo: solve problems with fully qualified names
> > > > > 
> > > > > sudo expects the same name in sudo rule as login name. Therefore
> > > > > if fully qualified name is used or even enforced by setting
> > > > > use_fully_qualified_names to true or by forcing default domain
> > > > > with default_domain_suffix sssd is able to correctly return the
> > > > > rules but sudo can't match the user with contect of sudoUser
> > > > > attribute since it is not qualified.
> > > > > 
> > > > > This patch changes the rules on the fly to avoid using names at all.
> > > > > We do this in two steps:
> > > > > 1. We fetch all rules that match current user name, id or groups and
> > > > > replace sudoUser attribute with sudoUser: #uid.
> > > > > 2. We fetch complementry rules that contain netgroups since it is
> > > > > expected we don't have infromation about existing netgroups in
> > > > > cache, sudo still needs to evaluate it for us if needed.
> > > > > 
> > > > > This patch also remove test for sysdb_get_sudo_filter since it wasn't
> > > > > sufficient anyway and I did not rewrite it since I don't thing it
> > > > > is a good thing to have filter tests that depends on exact filter
> > > > > order.
> > > > > 
> > > > > Resolves:
> > > > > https://fedorahosted.org/sssd/ticket/2919
> > > > 
> > > > There still seems to be some issue in sysdb tests:
> > > > FAIL: test_sysdb_sudo
> > > > =
> > > > [==] Running 12 test(s).
> > > > [ RUN  ] test_store_sudo
> > > > [   OK ] test_store_sudo
> > > > [ RUN  ] test_sudo_purge_by_filter
> > > > [   OK ] test_sudo_purge_by_filter
> > > > [ RUN  ] test_sudo_purge_by_rules
> > > > [   OK ] test_sudo_purge_by_rules
> > > > [ RUN  ] test_sudo_set_get_last_full_refresh
> > > > [   OK ] test_sudo_set_get_last_full_refresh
> > > > [ RUN  ] test_get_sudo_user_info
> > > > [   OK ] test_get_sudo_user_info
> > > > [ RUN  ] test_get_sudo_user_info_nogroup
> > > > [   OK ] test_get_sudo_user_info_nogroup
> > > > [ RUN  ] test_get_sudo_nouser
> > > > (Tue May 31 11:38:50:598671 2016) [sssd] [sysdb_get_sudo_user_info] 
> > > > (0x0020): Error looking up user no_user
> > > > [   OK ] test_get_sudo_nouser
> > > > [ RUN  ] test_set_sudo_rule_attr_add
> > > > [   OK ] test_set_sudo_rule_attr_add
> > > > [ RUN  ] test_set_sudo_rule_attr_replace
> > > > [   OK ] test_set_sudo_rule_attr_replace
> > > > [ RUN  ] test_set_sudo_rule_attr_delete
> > > > [   OK ] test_set_sudo_rule_attr_delete
> > > > [ RUN  ] test_search_sudo_rules
> > > > (Tue May 31 11:38:50:615209 2016) [sssd] [talloc_log_fn] (0x0010): Bad 
> > > > talloc magic value - unknown value
> > > > FAIL test_sysdb_sudo (exit status: 134)
> > > 
> > > Sorry about that. It should be fixed now.
> > > 
> > 
> > I did quite a bit of testing with this patch and it seems to work fine.
> > 
> > ACK
> 
> btw I would prefer to push this patch together with my sysdb patchset

* master: 61913b8f0d1ba54d82640500d7486fac5f72b030
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] LDAP: Lookup services by all protocols unless a protocol is specified

[Pavel Březina]

On 07/07/2016 01:24 PM, Jakub Hrozek wrote:

On Thu, Jul 07, 2016 at 12:39:28PM +0200, Pavel Březina wrote:

On 07/07/2016 12:34 PM, Jakub Hrozek wrote:

On Thu, Jul 07, 2016 at 10:16:03AM +0200, Sumit Bose wrote:

resend
- Forwarded message from Sumit Bose  -

Date: Wed, 6 Jul 2016 11:13:48 +0200
From: Sumit Bose 
To: sssd-devel@lists.fedorahosted.org
Subject: Re: [SSSD] [PATCH] LDAP: Lookup services by all protocols unless a
protocol is specified
Message-ID: <20160706091348.GD29143@p.Speedport_W_724V_Typ_A_05011603_00_009>
References: <20160705103025.GB24232@hendrix>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160705103025.GB24232@hendrix>
User-Agent: Mutt/1.6.1 (2016-04-27)

On Tue, Jul 05, 2016 at 12:30:25PM +0200, Jakub Hrozek wrote:

Hi,

the attached patch makes service lookups great again.

To reproduce, just run:
  getent service -s sss ldap
before the patch we would look up ipService="" because DP gives us an
empty string after the recent DP patches.



  From ba9834637b3cc0d7d98f704ba70f9dcb6f9a70e9 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Tue, 5 Jul 2016 12:23:23 +0200
Subject: [PATCH] LDAP: Lookup services by all protocols unless a protocol is
   specified

The DP refactoring changed the way we handle strings from sbus. We no
longer receive NULL strings, but empty strings instead.
---
   src/providers/ldap/ldap_id_services.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/providers/ldap/ldap_id_services.c 
b/src/providers/ldap/ldap_id_services.c
index 
77215127b53297d840eaa4d2f35a75eedb085e43..db47e3fc55eea969371d61c3c5ac7f818196f3d5
 100644
--- a/src/providers/ldap/ldap_id_services.c
+++ b/src/providers/ldap/ldap_id_services.c
@@ -114,7 +114,7 @@ services_get_send(TALLOC_CTX *mem_ctx,
   ret = sss_filter_sanitize(state, name, _name);
   if (ret != EOK)  goto error;

-if (protocol) {
+if (protocol && protocol[0] != '\0') {
   ret = sss_filter_sanitize(state, protocol, _protocol);
   if (ret != EOK)  goto error;
   }


since the sysdb calls later on in the request expect protocol==NULL as
well I would suggest something like

   state->sysdb = sdom->dom->sysdb;
   state->name = name;
   state->protocol = protocol;
+if (state->protocol[0] == '\0') {
+state->protocol = NULL;
+}
   state->filter_type = filter_type;
   state->noexist_delete = noexist_delete;


in only use state->protocol in the request.


OK, done (we use state->protocol up to a point, then clean_protocol). I
also added a check to cover the unlikely situation where sbus might give
us a NULL string, just to error and don't crash.


Hi, nice catch. I think it may be better to amend check_and_parse_filter()
in dp_target_id.c so it assign NULL instead of "" in data->filter_value (and
other) using SBUS_SET_STRING(). Though I didn't check if other look up types
(user/group/...) don't expect empty string instead.


Yes, this makes sense and a quick git grep shows that the providers
expect NULL, not empty string (for example for enumeration which we
already special-case in check_and_parse_filter). But at this point,
I would prefer to push this patch, release 1.14.0 and then patch
check_and_parse_filter() and run tests again to make sure we don't break
some strange corner case with a late sbus fix.

Attached is a new patch that doesn't error out if protocol is NULL.


Ok, I'm fine with this. The code looks good to me.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] LDAP: Lookup services by all protocols unless a protocol is specified

[Jakub Hrozek]

On Thu, Jul 07, 2016 at 12:39:28PM +0200, Pavel Březina wrote:
> On 07/07/2016 12:34 PM, Jakub Hrozek wrote:
> > On Thu, Jul 07, 2016 at 10:16:03AM +0200, Sumit Bose wrote:
> > > resend
> > > - Forwarded message from Sumit Bose  -
> > > 
> > > Date: Wed, 6 Jul 2016 11:13:48 +0200
> > > From: Sumit Bose 
> > > To: sssd-devel@lists.fedorahosted.org
> > > Subject: Re: [SSSD] [PATCH] LDAP: Lookup services by all protocols unless 
> > > a
> > >   protocol is specified
> > > Message-ID: 
> > > <20160706091348.GD29143@p.Speedport_W_724V_Typ_A_05011603_00_009>
> > > References: <20160705103025.GB24232@hendrix>
> > > MIME-Version: 1.0
> > > Content-Type: text/plain; charset=us-ascii
> > > Content-Disposition: inline
> > > In-Reply-To: <20160705103025.GB24232@hendrix>
> > > User-Agent: Mutt/1.6.1 (2016-04-27)
> > > 
> > > On Tue, Jul 05, 2016 at 12:30:25PM +0200, Jakub Hrozek wrote:
> > > > Hi,
> > > > 
> > > > the attached patch makes service lookups great again.
> > > > 
> > > > To reproduce, just run:
> > > >  getent service -s sss ldap
> > > > before the patch we would look up ipService="" because DP gives us an
> > > > empty string after the recent DP patches.
> > > 
> > > >  From ba9834637b3cc0d7d98f704ba70f9dcb6f9a70e9 Mon Sep 17 00:00:00 2001
> > > > From: Jakub Hrozek 
> > > > Date: Tue, 5 Jul 2016 12:23:23 +0200
> > > > Subject: [PATCH] LDAP: Lookup services by all protocols unless a 
> > > > protocol is
> > > >   specified
> > > > 
> > > > The DP refactoring changed the way we handle strings from sbus. We no
> > > > longer receive NULL strings, but empty strings instead.
> > > > ---
> > > >   src/providers/ldap/ldap_id_services.c | 2 +-
> > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/src/providers/ldap/ldap_id_services.c 
> > > > b/src/providers/ldap/ldap_id_services.c
> > > > index 
> > > > 77215127b53297d840eaa4d2f35a75eedb085e43..db47e3fc55eea969371d61c3c5ac7f818196f3d5
> > > >  100644
> > > > --- a/src/providers/ldap/ldap_id_services.c
> > > > +++ b/src/providers/ldap/ldap_id_services.c
> > > > @@ -114,7 +114,7 @@ services_get_send(TALLOC_CTX *mem_ctx,
> > > >   ret = sss_filter_sanitize(state, name, _name);
> > > >   if (ret != EOK)  goto error;
> > > > 
> > > > -if (protocol) {
> > > > +if (protocol && protocol[0] != '\0') {
> > > >   ret = sss_filter_sanitize(state, protocol, _protocol);
> > > >   if (ret != EOK)  goto error;
> > > >   }
> > > 
> > > since the sysdb calls later on in the request expect protocol==NULL as
> > > well I would suggest something like
> > > 
> > >   state->sysdb = sdom->dom->sysdb;
> > >   state->name = name;
> > >   state->protocol = protocol;
> > > +if (state->protocol[0] == '\0') {
> > > +state->protocol = NULL;
> > > +}
> > >   state->filter_type = filter_type;
> > >   state->noexist_delete = noexist_delete;
> > > 
> > > 
> > > in only use state->protocol in the request.
> > 
> > OK, done (we use state->protocol up to a point, then clean_protocol). I
> > also added a check to cover the unlikely situation where sbus might give
> > us a NULL string, just to error and don't crash.
> 
> Hi, nice catch. I think it may be better to amend check_and_parse_filter()
> in dp_target_id.c so it assign NULL instead of "" in data->filter_value (and
> other) using SBUS_SET_STRING(). Though I didn't check if other look up types
> (user/group/...) don't expect empty string instead.

Yes, this makes sense and a quick git grep shows that the providers
expect NULL, not empty string (for example for enumeration which we
already special-case in check_and_parse_filter). But at this point,
I would prefer to push this patch, release 1.14.0 and then patch
check_and_parse_filter() and run tests again to make sure we don't break
some strange corner case with a late sbus fix.

Attached is a new patch that doesn't error out if protocol is NULL.
>From aafc94e76722019f8b921213a48867fb97be6a03 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Tue, 5 Jul 2016 12:23:23 +0200
Subject: [PATCH] LDAP: Lookup services by all protocols unless a protocol is
 specified

The DP refactoring changed the way we handle strings from sbus. We no
longer receive NULL strings, but empty strings instead.
---
 src/providers/ldap/ldap_id_services.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/providers/ldap/ldap_id_services.c 
b/src/providers/ldap/ldap_id_services.c
index 
77215127b53297d840eaa4d2f35a75eedb085e43..401228c20af31ae2df9bb3d35ed25fb6f06b1839
 100644
--- a/src/providers/ldap/ldap_id_services.c
+++ b/src/providers/ldap/ldap_id_services.c
@@ -89,6 +89,9 @@ services_get_send(TALLOC_CTX *mem_ctx,
 state->sysdb = sdom->dom->sysdb;
 state->name = name;
 state->protocol = protocol;
+if (state->protocol != NULL && state->protocol[0] == '\0') {
+state->protocol = 

[SSSD] Re: [PATCH] sssctl: Add config-check command

[Lukas Slebodnik]

On (07/07/16 12:37), Michal Židek wrote:
>On 07/04/2016 05:27 PM, Michal Židek wrote:
>> On 07/01/2016 05:35 PM, Lukas Slebodnik wrote:
>> > On (01/07/16 17:26), Michal Židek wrote:
>> > > Hello!
>> > > 
>> > > This patch adds new command config-check
>> > > for sssctl tool. The output looks like this:
>> > > 
>> > > 
>> > > Issues identified by validators: 3
>> > > [rule/allowed_sections]: Section [SectionFOO] is not allowed. Check for
>> > > typos.
>> > > [rule/allowed_sssd_options]: Attribute 'unlknowww' is not allowed in
>> > > section
>> > > 'sssd'. Check for typos.
>> > > [rule/allowed_sssd_options]: Attribute 'unknown_option' is not
>> > > allowed in
>> > > section 'sssd'. Check for typos.
>> > > 
>> > > Messages generated during configuration merging: 4
>> > > File conf.dd did not match provided patterns. Skipping.
>> > > Errors detected while parsing: /etc/sssd/conf.d/snip-bad.conf.
>> > > Error (5) on line 5: Equal sign is missing.
>> > > Due to errors file /etc/sssd/conf.d/snip-bad.conf is not considered.
>> > > Skipping.
>> > > 
>> > > Used configuration snippet files: 1
>> > > /etc/sssd/conf.d/snip-good.conf
>> > > 
>> > > 
>> > > Currently it can only be used by root and checks only
>> > > configuration in default path.
>> > > 
>> > > Michal
>> > 
>> > > From 864091da5010b75d9c38e87ab3be467b8bc6f8f2 Mon Sep 17 00:00:00 2001
>> > > From: =?UTF-8?q?Michal=20=C5=BDidek?= 
>> > > Date: Fri, 1 Jul 2016 15:10:30 +0200
>> > > Subject: [PATCH 1/4] sss_ini: Small refacoring of
>> > > sss_ini_call_validators
>> > > 
>> > > Separate logic to fill errobj so that
>> > > the errors can be printed by the caller.
>> > > ---
>> > > src/util/sss_ini.c | 49
>> > > -
>> > > src/util/sss_ini.h | 12 
>> > > 2 files changed, 48 insertions(+), 13 deletions(-)
>> > > 
>> > > diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c
>> > > index b4dbb07..78f5490 100644
>> > > --- a/src/util/sss_ini.c
>> > > +++ b/src/util/sss_ini.c
>> > > @@ -549,7 +549,6 @@ int sss_ini_call_validators(struct
>> > > sss_ini_initdata *data,
>> > > {
>> > > #ifdef HAVE_LIBINI_CONFIG_V1_3
>> > >  int ret;
>> > > -struct ini_cfgobj *rules_cfgobj = NULL;
>> > >  struct ini_errobj *errobj = NULL;
>> > > 
>> > >  ret = ini_errobj_create();
>> > > @@ -558,17 +557,12 @@ int sss_ini_call_validators(struct
>> > > sss_ini_initdata *data,
>> > >  goto done;
>> > >  }
>> > > 
>> > > -ret = ini_rules_read_from_file(rules_path, _cfgobj);
>> > > +ret = sss_ini_call_validators_errobj(data,
>> > > + rules_path,
>> > > + errobj);
>> > >  if (ret != EOK) {
>> > > -DEBUG(SSSDBG_FATAL_FAILURE,
>> > > -  "Failed to read sssd.conf schema %d [%s]\n", ret,
>> > > strerror(ret));
>> > > -goto done;
>> > > -}
>> > > -
>> > > -ret = ini_rules_check(rules_cfgobj, data->sssd_config, NULL,
>> > > errobj);
>> > > -if (ret != EOK) {
>> > > -DEBUG(SSSDBG_FATAL_FAILURE,
>> > > -  "ini_rules_check failed %d [%s]\n", ret, strerror(ret));
>> > > +DEBUG(SSSDBG_CRIT_FAILURE,
>> > > +  "Failed to get errors from validators.\n");
>> > >  goto done;
>> > >  }
>> > > 
>> > > @@ -579,10 +573,10 @@ int sss_ini_call_validators(struct
>> > > sss_ini_initdata *data,
>> > >  ini_errobj_next(errobj);
>> > >  }
>> > > 
>> > > +ret = EOK;
>> > > +
>> > > done:
>> > > -if (rules_cfgobj) ini_config_destroy(rules_cfgobj);
>> > >  ini_errobj_destroy();
>> > > -
>> > >  return ret;
>> > > #else
>> > >  DEBUG(SSSDBG_TRACE_FUNC,
>> > > @@ -590,3 +584,32 @@ done:
>> > >  return EOK;
>> > > #endif /* HAVE_LIBINI_CONFIG_V1_3 */
>> > > }
>> > > +
>> > > +#ifdef HAVE_LIBINI_CONFIG_V1_3
>> > > +int sss_ini_call_validators_errobj(struct sss_ini_initdata *data,
>> > > +   const char *rules_path,
>> > > +   struct ini_errobj *errobj)
>> > > +{
>> > > +int ret;
>> > > +struct ini_cfgobj *rules_cfgobj = NULL;
>> > > +
>> > > +ret = ini_rules_read_from_file(rules_path, _cfgobj);
>> > > +if (ret != EOK) {
>> > > +DEBUG(SSSDBG_FATAL_FAILURE,
>> > > +  "Failed to read sssd.conf schema %d [%s]\n", ret,
>> > > strerror(ret));
>> > > +goto done;
>> > > +}
>> > > +
>> > > +ret = ini_rules_check(rules_cfgobj, data->sssd_config, NULL,
>> > > errobj);
>> > > +if (ret != EOK) {
>> > > +DEBUG(SSSDBG_FATAL_FAILURE,
>> > > +  "ini_rules_check failed %d [%s]\n", ret, strerror(ret));
>> > > +goto done;
>> > > +}
>> > > +
>> > > +done:
>> > > +if (rules_cfgobj) ini_config_destroy(rules_cfgobj);
>> > > +
>> > > +return ret;
>> > > +}
>> > > +#endif /* HAVE_LIBINI_CONFIG_V1_3 */
>> > > diff --git a/src/util/sss_ini.h b/src/util/sss_ini.h
>> > > index 

[SSSD] Re: [PATCH][PUSHED] MAN: Include idmap_sss.8.xml in the manpage sources

[Jakub Hrozek]

On Thu, Jul 07, 2016 at 01:06:31PM +0200, Lukas Slebodnik wrote:
> On (07/07/16 13:05), Pavel Březina wrote:
> >On 06/30/2016 03:37 PM, Lukas Slebodnik wrote:
> >> On (30/06/16 09:28), Sumit Bose wrote:
> >> > On Wed, Jun 29, 2016 at 11:23:55PM +0200, Jakub Hrozek wrote:
> >> > > Hi,
> >> > > I pushed the attached patch under the one-liner rule (which I'm not a
> >> > > big fan of, but it's late and I've been wanting to do this release for
> >> > > almost a week..)
> >> > > 
> >> > > I hope it's OK with everybody. I added a silly RB to satisfy our git
> >> > > hook.
> >> > 
> >> > Thank you for catching this. The patch makes sense and looks good by
> >> > careful visual inspection. So afterwards-ACK.
> >> > 
> >> > bye,
> >> > Sumit
> >> > 
> >> > >  From 1dab34228cefddcd6bedef2d2cee70a620c8ace8 Mon Sep 17 00:00:00 2001
> >> > > From: Jakub Hrozek 
> >> > > Date: Wed, 29 Jun 2016 23:16:54 +0200
> >> > > Subject: [PATCH] MAN: Include idmap_sss.8.xml in the manpage sources
> >> > > 
> >> > > Reviewed-by: N/A, one-liner before release
> >> > > ---
> >> > >   src/man/po/po4a.cfg | 1 +
> >> > >   1 file changed, 1 insertion(+)
> >> > > 
> >> > > diff --git a/src/man/po/po4a.cfg b/src/man/po/po4a.cfg
> >> > > index 
> >> > > 6dbf11906164e1601964b2e434bfef49455ef17e..ca9d66a2a616f4ced179fb78edf50f94b77dcec3
> >> > >  100644
> >> > > --- a/src/man/po/po4a.cfg
> >> > > +++ b/src/man/po/po4a.cfg
> >> > > @@ -26,6 +26,7 @@
> >> > >   [type:docbook] sss_rpcidmapd.5.xml 
> >> > > $lang:$(builddir)/$lang/sss_rpcidmapd.5.xml
> >> > >   [type:docbook] sss_ssh_authorizedkeys.1.xml 
> >> > > $lang:$(builddir)/$lang/sss_ssh_authorizedkeys.1.xml
> >> > >   [type:docbook] sss_ssh_knownhostsproxy.1.xml 
> >> > > $lang:$(builddir)/$lang/sss_ssh_knownhostsproxy.1.xml
> >> > > +[type:docbook] idmap_sss.8.xml $lang:$(builddir)/$lang/idmap_sss.8.xml
> >> > >   [type:docbook] include/service_discovery.xml 
> >> > > $lang:$(builddir)/$lang/include/service_discovery.xml opt:"-k 0"
> >> > >   [type:docbook] include/upstream.xml 
> >> > > $lang:$(builddir)/$lang/include/upstream.xml opt:"-k 0"
> >> > >   [type:docbook] include/failover.xml 
> >> > > $lang:$(builddir)/$lang/include/failover.xml opt:"-k 0"
> >> > > --
> >> > > 2.4.11
> >> > > 
> >> > 
> >> It happened more time that we forgot to add man page there.
> >> Maybe it would be good to write a test. (bash script should be enough)
> >> We already have a test to check traling whitestapces
> >> src/tests/whitespace_test.
> >
> >Instead of writing a test, it would be better to write a script to generate
> >po4a.cfg imho.
> >
> That's good idea as well.

I know we tried it in the past, but there was an issue generatingn the
file with parallel build. But maybe we didn't try hard enough.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH][PUSHED] MAN: Include idmap_sss.8.xml in the manpage sources

[Lukas Slebodnik]

On (07/07/16 13:05), Pavel Březina wrote:
>On 06/30/2016 03:37 PM, Lukas Slebodnik wrote:
>> On (30/06/16 09:28), Sumit Bose wrote:
>> > On Wed, Jun 29, 2016 at 11:23:55PM +0200, Jakub Hrozek wrote:
>> > > Hi,
>> > > I pushed the attached patch under the one-liner rule (which I'm not a
>> > > big fan of, but it's late and I've been wanting to do this release for
>> > > almost a week..)
>> > > 
>> > > I hope it's OK with everybody. I added a silly RB to satisfy our git
>> > > hook.
>> > 
>> > Thank you for catching this. The patch makes sense and looks good by
>> > careful visual inspection. So afterwards-ACK.
>> > 
>> > bye,
>> > Sumit
>> > 
>> > >  From 1dab34228cefddcd6bedef2d2cee70a620c8ace8 Mon Sep 17 00:00:00 2001
>> > > From: Jakub Hrozek 
>> > > Date: Wed, 29 Jun 2016 23:16:54 +0200
>> > > Subject: [PATCH] MAN: Include idmap_sss.8.xml in the manpage sources
>> > > 
>> > > Reviewed-by: N/A, one-liner before release
>> > > ---
>> > >   src/man/po/po4a.cfg | 1 +
>> > >   1 file changed, 1 insertion(+)
>> > > 
>> > > diff --git a/src/man/po/po4a.cfg b/src/man/po/po4a.cfg
>> > > index 
>> > > 6dbf11906164e1601964b2e434bfef49455ef17e..ca9d66a2a616f4ced179fb78edf50f94b77dcec3
>> > >  100644
>> > > --- a/src/man/po/po4a.cfg
>> > > +++ b/src/man/po/po4a.cfg
>> > > @@ -26,6 +26,7 @@
>> > >   [type:docbook] sss_rpcidmapd.5.xml 
>> > > $lang:$(builddir)/$lang/sss_rpcidmapd.5.xml
>> > >   [type:docbook] sss_ssh_authorizedkeys.1.xml 
>> > > $lang:$(builddir)/$lang/sss_ssh_authorizedkeys.1.xml
>> > >   [type:docbook] sss_ssh_knownhostsproxy.1.xml 
>> > > $lang:$(builddir)/$lang/sss_ssh_knownhostsproxy.1.xml
>> > > +[type:docbook] idmap_sss.8.xml $lang:$(builddir)/$lang/idmap_sss.8.xml
>> > >   [type:docbook] include/service_discovery.xml 
>> > > $lang:$(builddir)/$lang/include/service_discovery.xml opt:"-k 0"
>> > >   [type:docbook] include/upstream.xml 
>> > > $lang:$(builddir)/$lang/include/upstream.xml opt:"-k 0"
>> > >   [type:docbook] include/failover.xml 
>> > > $lang:$(builddir)/$lang/include/failover.xml opt:"-k 0"
>> > > --
>> > > 2.4.11
>> > > 
>> > 
>> It happened more time that we forgot to add man page there.
>> Maybe it would be good to write a test. (bash script should be enough)
>> We already have a test to check traling whitestapces
>> src/tests/whitespace_test.
>
>Instead of writing a test, it would be better to write a script to generate
>po4a.cfg imho.
>
That's good idea as well.

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] CONFIGURE: Inform about optional build dependencies

[Lukas Slebodnik]

On (05/07/16 07:59), Jakub Hrozek wrote:
>On Fri, Jul 01, 2016 at 08:45:53AM +0200, Lukas Slebodnik wrote:
>> ehlo,
>>
>> We usually inform about optional build dependencies at configure time
>> and which option can disable checking of this dependency.
>>
>> LS
>
>I agree with the intent, but I would just say we need such and such
>library, instead of a header file. This is also what we say when the
>samba libraries are not found.
Detection of http_parser and libjansson is slit into 3 parts.
a) try detect with pkg-config (http_parser doesn' have it)
b) detect header file
c) check for specific function if header file is available.

I modified error mesasge for the 2nd step.
Error message for library with wrong function is different.
  AC_MSG_ERROR([libhttp_parser missing http_parser_init])
I didn't change it because hint should be added
to the 2nd step and not the 3rd step.

I choose to inform about header file because the name of package
is not the same on all distributions
 (debian:libhttp-parser-dev fedora:http-parser-devel)

But feel free to propose better sentences.

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH][PUSHED] MAN: Include idmap_sss.8.xml in the manpage sources

[Pavel Březina]

On 06/30/2016 03:37 PM, Lukas Slebodnik wrote:

On (30/06/16 09:28), Sumit Bose wrote:

On Wed, Jun 29, 2016 at 11:23:55PM +0200, Jakub Hrozek wrote:

Hi,
I pushed the attached patch under the one-liner rule (which I'm not a
big fan of, but it's late and I've been wanting to do this release for
almost a week..)

I hope it's OK with everybody. I added a silly RB to satisfy our git
hook.


Thank you for catching this. The patch makes sense and looks good by
careful visual inspection. So afterwards-ACK.

bye,
Sumit


 From 1dab34228cefddcd6bedef2d2cee70a620c8ace8 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Wed, 29 Jun 2016 23:16:54 +0200
Subject: [PATCH] MAN: Include idmap_sss.8.xml in the manpage sources

Reviewed-by: N/A, one-liner before release
---
  src/man/po/po4a.cfg | 1 +
  1 file changed, 1 insertion(+)

diff --git a/src/man/po/po4a.cfg b/src/man/po/po4a.cfg
index 
6dbf11906164e1601964b2e434bfef49455ef17e..ca9d66a2a616f4ced179fb78edf50f94b77dcec3
 100644
--- a/src/man/po/po4a.cfg
+++ b/src/man/po/po4a.cfg
@@ -26,6 +26,7 @@
  [type:docbook] sss_rpcidmapd.5.xml $lang:$(builddir)/$lang/sss_rpcidmapd.5.xml
  [type:docbook] sss_ssh_authorizedkeys.1.xml 
$lang:$(builddir)/$lang/sss_ssh_authorizedkeys.1.xml
  [type:docbook] sss_ssh_knownhostsproxy.1.xml 
$lang:$(builddir)/$lang/sss_ssh_knownhostsproxy.1.xml
+[type:docbook] idmap_sss.8.xml $lang:$(builddir)/$lang/idmap_sss.8.xml
  [type:docbook] include/service_discovery.xml 
$lang:$(builddir)/$lang/include/service_discovery.xml opt:"-k 0"
  [type:docbook] include/upstream.xml $lang:$(builddir)/$lang/include/upstream.xml 
opt:"-k 0"
  [type:docbook] include/failover.xml $lang:$(builddir)/$lang/include/failover.xml 
opt:"-k 0"
--
2.4.11




It happened more time that we forgot to add man page there.
Maybe it would be good to write a test. (bash script should be enough)
We already have a test to check traling whitestapces
src/tests/whitespace_test.


Instead of writing a test, it would be better to write a script to 
generate po4a.cfg imho.


___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] sssctl: print a message when managing sssd

[Pavel Březina]

On 07/05/2016 07:49 AM, Jakub Hrozek wrote:

On Fri, Jul 01, 2016 at 01:24:35PM +0200, Pavel Březina wrote:


 From 8b877579f3d1a9bbfa728a6e78ff829d936efbb1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
Date: Fri, 1 Jul 2016 13:23:57 +0200
Subject: [PATCH] sssctl: print a message when managing sssd


Shouldn't this be specific to the service manager used? systemctl is
quiet so this does add additional verbosity, but init scripts printed
the starting/stopping message as well, so we would print them twice.

In general, I'm more in favor of tools being silent unless something
goes bad.


I think we should print something when we are doing more operations such 
as in remove-cache we are stopping and starting SSSD so I think it may 
be nice to tell administrator that we have actually stopped sssd but 
that we were unable to start it. But if you think we should stay silence 
and just print an error feel free to ignore this patch or tell me if I 
should make it systemd specific message.

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] PAM/KRB5: optional otp and password prompting

[Jakub Hrozek]

On Thu, Jul 07, 2016 at 12:39:01PM +0200, Jakub Hrozek wrote:
> The prompts were changes as Nathaniel suggested and the basic sanity
> checks worked with this patch. I'm going to push with Nathaniel's RB.

* master: 78027feeb56d6fe216f699be86a4716aaef3f628
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] Fix packet size calculation in sss_packet_new

[Lukas Slebodnik]

On (07/07/16 12:33), Pavel Březina wrote:
>On 07/07/2016 11:55 AM, Nikolai Kondrashov wrote:
>> Hi everyone,
>> 
>> The attached patch fixes potential packet buffer overflow with certain body
>> sizes. Found while reading through SSSD code.
>> 
>> Nick
>
>Ack, nice catch. I fixed this one in packet_grow some time ago.
master:
* 740bfe1a5bf519de8e13bdce5c4143b0f24d7433

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] LDAP: Lookup services by all protocols unless a protocol is specified

[Pavel Březina]

On 07/07/2016 12:34 PM, Jakub Hrozek wrote:

On Thu, Jul 07, 2016 at 10:16:03AM +0200, Sumit Bose wrote:

resend
- Forwarded message from Sumit Bose  -

Date: Wed, 6 Jul 2016 11:13:48 +0200
From: Sumit Bose 
To: sssd-devel@lists.fedorahosted.org
Subject: Re: [SSSD] [PATCH] LDAP: Lookup services by all protocols unless a
protocol is specified
Message-ID: <20160706091348.GD29143@p.Speedport_W_724V_Typ_A_05011603_00_009>
References: <20160705103025.GB24232@hendrix>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160705103025.GB24232@hendrix>
User-Agent: Mutt/1.6.1 (2016-04-27)

On Tue, Jul 05, 2016 at 12:30:25PM +0200, Jakub Hrozek wrote:

Hi,

the attached patch makes service lookups great again.

To reproduce, just run:
 getent service -s sss ldap
before the patch we would look up ipService="" because DP gives us an
empty string after the recent DP patches.



 From ba9834637b3cc0d7d98f704ba70f9dcb6f9a70e9 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Tue, 5 Jul 2016 12:23:23 +0200
Subject: [PATCH] LDAP: Lookup services by all protocols unless a protocol is
  specified

The DP refactoring changed the way we handle strings from sbus. We no
longer receive NULL strings, but empty strings instead.
---
  src/providers/ldap/ldap_id_services.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/providers/ldap/ldap_id_services.c 
b/src/providers/ldap/ldap_id_services.c
index 
77215127b53297d840eaa4d2f35a75eedb085e43..db47e3fc55eea969371d61c3c5ac7f818196f3d5
 100644
--- a/src/providers/ldap/ldap_id_services.c
+++ b/src/providers/ldap/ldap_id_services.c
@@ -114,7 +114,7 @@ services_get_send(TALLOC_CTX *mem_ctx,
  ret = sss_filter_sanitize(state, name, _name);
  if (ret != EOK)  goto error;

-if (protocol) {
+if (protocol && protocol[0] != '\0') {
  ret = sss_filter_sanitize(state, protocol, _protocol);
  if (ret != EOK)  goto error;
  }


since the sysdb calls later on in the request expect protocol==NULL as
well I would suggest something like

  state->sysdb = sdom->dom->sysdb;
  state->name = name;
  state->protocol = protocol;
+if (state->protocol[0] == '\0') {
+state->protocol = NULL;
+}
  state->filter_type = filter_type;
  state->noexist_delete = noexist_delete;


in only use state->protocol in the request.


OK, done (we use state->protocol up to a point, then clean_protocol). I
also added a check to cover the unlikely situation where sbus might give
us a NULL string, just to error and don't crash.


Hi, nice catch. I think it may be better to amend 
check_and_parse_filter() in dp_target_id.c so it assign NULL instead of 
"" in data->filter_value (and other) using SBUS_SET_STRING(). Though I 
didn't check if other look up types (user/group/...) don't expect empty 
string instead.

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] sssctl: Add config-check command

[Michal Židek]

On 07/04/2016 05:27 PM, Michal Židek wrote:

On 07/01/2016 05:35 PM, Lukas Slebodnik wrote:

On (01/07/16 17:26), Michal Židek wrote:

Hello!

This patch adds new command config-check
for sssctl tool. The output looks like this:


Issues identified by validators: 3
[rule/allowed_sections]: Section [SectionFOO] is not allowed. Check for
typos.
[rule/allowed_sssd_options]: Attribute 'unlknowww' is not allowed in
section
'sssd'. Check for typos.
[rule/allowed_sssd_options]: Attribute 'unknown_option' is not
allowed in
section 'sssd'. Check for typos.

Messages generated during configuration merging: 4
File conf.dd did not match provided patterns. Skipping.
Errors detected while parsing: /etc/sssd/conf.d/snip-bad.conf.
Error (5) on line 5: Equal sign is missing.
Due to errors file /etc/sssd/conf.d/snip-bad.conf is not considered.
Skipping.

Used configuration snippet files: 1
/etc/sssd/conf.d/snip-good.conf


Currently it can only be used by root and checks only
configuration in default path.

Michal



From 864091da5010b75d9c38e87ab3be467b8bc6f8f2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
Date: Fri, 1 Jul 2016 15:10:30 +0200
Subject: [PATCH 1/4] sss_ini: Small refacoring of
sss_ini_call_validators

Separate logic to fill errobj so that
the errors can be printed by the caller.
---
src/util/sss_ini.c | 49
-
src/util/sss_ini.h | 12 
2 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c
index b4dbb07..78f5490 100644
--- a/src/util/sss_ini.c
+++ b/src/util/sss_ini.c
@@ -549,7 +549,6 @@ int sss_ini_call_validators(struct
sss_ini_initdata *data,
{
#ifdef HAVE_LIBINI_CONFIG_V1_3
 int ret;
-struct ini_cfgobj *rules_cfgobj = NULL;
 struct ini_errobj *errobj = NULL;

 ret = ini_errobj_create();
@@ -558,17 +557,12 @@ int sss_ini_call_validators(struct
sss_ini_initdata *data,
 goto done;
 }

-ret = ini_rules_read_from_file(rules_path, _cfgobj);
+ret = sss_ini_call_validators_errobj(data,
+ rules_path,
+ errobj);
 if (ret != EOK) {
-DEBUG(SSSDBG_FATAL_FAILURE,
-  "Failed to read sssd.conf schema %d [%s]\n", ret,
strerror(ret));
-goto done;
-}
-
-ret = ini_rules_check(rules_cfgobj, data->sssd_config, NULL,
errobj);
-if (ret != EOK) {
-DEBUG(SSSDBG_FATAL_FAILURE,
-  "ini_rules_check failed %d [%s]\n", ret, strerror(ret));
+DEBUG(SSSDBG_CRIT_FAILURE,
+  "Failed to get errors from validators.\n");
 goto done;
 }

@@ -579,10 +573,10 @@ int sss_ini_call_validators(struct
sss_ini_initdata *data,
 ini_errobj_next(errobj);
 }

+ret = EOK;
+
done:
-if (rules_cfgobj) ini_config_destroy(rules_cfgobj);
 ini_errobj_destroy();
-
 return ret;
#else
 DEBUG(SSSDBG_TRACE_FUNC,
@@ -590,3 +584,32 @@ done:
 return EOK;
#endif /* HAVE_LIBINI_CONFIG_V1_3 */
}
+
+#ifdef HAVE_LIBINI_CONFIG_V1_3
+int sss_ini_call_validators_errobj(struct sss_ini_initdata *data,
+   const char *rules_path,
+   struct ini_errobj *errobj)
+{
+int ret;
+struct ini_cfgobj *rules_cfgobj = NULL;
+
+ret = ini_rules_read_from_file(rules_path, _cfgobj);
+if (ret != EOK) {
+DEBUG(SSSDBG_FATAL_FAILURE,
+  "Failed to read sssd.conf schema %d [%s]\n", ret,
strerror(ret));
+goto done;
+}
+
+ret = ini_rules_check(rules_cfgobj, data->sssd_config, NULL,
errobj);
+if (ret != EOK) {
+DEBUG(SSSDBG_FATAL_FAILURE,
+  "ini_rules_check failed %d [%s]\n", ret, strerror(ret));
+goto done;
+}
+
+done:
+if (rules_cfgobj) ini_config_destroy(rules_cfgobj);
+
+return ret;
+}
+#endif /* HAVE_LIBINI_CONFIG_V1_3 */
diff --git a/src/util/sss_ini.h b/src/util/sss_ini.h
index 7734bab..77943d6 100644
--- a/src/util/sss_ini.h
+++ b/src/util/sss_ini.h
@@ -27,6 +27,10 @@
#ifndef __SSS_INI_H__
#define __SSS_INI_H__

+#ifdef HAVE_LIBINI_CONFIG_V1_3
+#include 
+#endif
+
/* Structure declarations */

/* INI data structure */
@@ -83,4 +87,12 @@ int sss_confdb_create_ldif(TALLOC_CTX *mem_ctx,
int sss_ini_call_validators(struct sss_ini_initdata *data,
 const char *rules_path);

+#ifdef HAVE_LIBINI_CONFIG_V1_3
+/* Get errors from validators with ini_errobj */
+int sss_ini_call_validators_errobj(struct sss_ini_initdata *data,
+   const char *rules_path,
+   struct ini_errobj *errobj);
+#endif /* HAVE_LIBINI_CONFIG_V1_3 */
+

NACK

src/util/sss_ini.h must not contain conditional build.
That's the putpose of implementation file.

Header file shoudl contain just a prototypes
becuase there might be missing prototypes if you forgot
include "config.h" before this header file.

LS

[SSSD] Re: [PATCH] LDAP: Lookup services by all protocols unless a protocol is specified

[Jakub Hrozek]

On Thu, Jul 07, 2016 at 10:16:03AM +0200, Sumit Bose wrote:
> resend
> - Forwarded message from Sumit Bose  -
> 
> Date: Wed, 6 Jul 2016 11:13:48 +0200
> From: Sumit Bose 
> To: sssd-devel@lists.fedorahosted.org
> Subject: Re: [SSSD] [PATCH] LDAP: Lookup services by all protocols unless a
>   protocol is specified
> Message-ID: <20160706091348.GD29143@p.Speedport_W_724V_Typ_A_05011603_00_009>
> References: <20160705103025.GB24232@hendrix>
> MIME-Version: 1.0
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> In-Reply-To: <20160705103025.GB24232@hendrix>
> User-Agent: Mutt/1.6.1 (2016-04-27)
> 
> On Tue, Jul 05, 2016 at 12:30:25PM +0200, Jakub Hrozek wrote:
> > Hi,
> > 
> > the attached patch makes service lookups great again.
> > 
> > To reproduce, just run:
> > getent service -s sss ldap
> > before the patch we would look up ipService="" because DP gives us an
> > empty string after the recent DP patches.
> 
> > From ba9834637b3cc0d7d98f704ba70f9dcb6f9a70e9 Mon Sep 17 00:00:00 2001
> > From: Jakub Hrozek 
> > Date: Tue, 5 Jul 2016 12:23:23 +0200
> > Subject: [PATCH] LDAP: Lookup services by all protocols unless a protocol is
> >  specified
> > 
> > The DP refactoring changed the way we handle strings from sbus. We no
> > longer receive NULL strings, but empty strings instead.
> > ---
> >  src/providers/ldap/ldap_id_services.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/providers/ldap/ldap_id_services.c 
> > b/src/providers/ldap/ldap_id_services.c
> > index 
> > 77215127b53297d840eaa4d2f35a75eedb085e43..db47e3fc55eea969371d61c3c5ac7f818196f3d5
> >  100644
> > --- a/src/providers/ldap/ldap_id_services.c
> > +++ b/src/providers/ldap/ldap_id_services.c
> > @@ -114,7 +114,7 @@ services_get_send(TALLOC_CTX *mem_ctx,
> >  ret = sss_filter_sanitize(state, name, _name);
> >  if (ret != EOK)  goto error;
> >  
> > -if (protocol) {
> > +if (protocol && protocol[0] != '\0') {
> >  ret = sss_filter_sanitize(state, protocol, _protocol);
> >  if (ret != EOK)  goto error;
> >  }
> 
> since the sysdb calls later on in the request expect protocol==NULL as
> well I would suggest something like 
> 
>  state->sysdb = sdom->dom->sysdb;
>  state->name = name;
>  state->protocol = protocol;
> +if (state->protocol[0] == '\0') {
> +state->protocol = NULL;
> +}
>  state->filter_type = filter_type;
>  state->noexist_delete = noexist_delete;
> 
> 
> in only use state->protocol in the request.

OK, done (we use state->protocol up to a point, then clean_protocol). I
also added a check to cover the unlikely situation where sbus might give
us a NULL string, just to error and don't crash.
>From e31ef4b5d6e2058b8d0495afcae753463e17e48c Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Tue, 5 Jul 2016 12:23:23 +0200
Subject: [PATCH] LDAP: Lookup services by all protocols unless a protocol is
 specified

The DP refactoring changed the way we handle strings from sbus. We no
longer receive NULL strings, but empty strings instead.
---
 src/providers/ldap/ldap_id_services.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/providers/ldap/ldap_id_services.c 
b/src/providers/ldap/ldap_id_services.c
index 
77215127b53297d840eaa4d2f35a75eedb085e43..78568655ba393b6ad702cd1d39f7bf2b76018d6e
 100644
--- a/src/providers/ldap/ldap_id_services.c
+++ b/src/providers/ldap/ldap_id_services.c
@@ -80,6 +80,11 @@ services_get_send(TALLOC_CTX *mem_ctx,
 req = tevent_req_create(mem_ctx, , struct sdap_services_get_state);
 if (!req) return NULL;
 
+if (protocol == NULL) {
+ret = EINVAL;
+goto error;
+}
+
 state->ev = ev;
 state->id_ctx = id_ctx;
 state->sdom = sdom;
@@ -89,6 +94,9 @@ services_get_send(TALLOC_CTX *mem_ctx,
 state->sysdb = sdom->dom->sysdb;
 state->name = name;
 state->protocol = protocol;
+if (state->protocol[0] == '\0') {
+state->protocol = NULL;
+}
 state->filter_type = filter_type;
 state->noexist_delete = noexist_delete;
 
@@ -114,8 +122,8 @@ services_get_send(TALLOC_CTX *mem_ctx,
 ret = sss_filter_sanitize(state, name, _name);
 if (ret != EOK)  goto error;
 
-if (protocol) {
-ret = sss_filter_sanitize(state, protocol, _protocol);
+if (state->protocol != NULL) {
+ret = sss_filter_sanitize(state, state->protocol, _protocol);
 if (ret != EOK)  goto error;
 }
 
-- 
2.4.11

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] Fix packet size calculation in sss_packet_new

[Pavel Březina]

On 07/07/2016 11:55 AM, Nikolai Kondrashov wrote:

Hi everyone,

The attached patch fixes potential packet buffer overflow with certain body
sizes. Found while reading through SSSD code.

Nick


Ack, nice catch. I fixed this one in packet_grow some time ago.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [PATCH] Fix packet size calculation in sss_packet_new

[Nikolai Kondrashov]

Hi everyone,

The attached patch fixes potential packet buffer overflow with certain body
sizes. Found while reading through SSSD code.

Nick
>From d708e1915e4464db9a2b0990c732c4e2edb0c0df Mon Sep 17 00:00:00 2001
From: Nikolai Kondrashov 
Date: Thu, 7 Jul 2016 12:48:42 +0300
Subject: [PATCH] Fix packet size calculation in sss_packet_new

Use division instead of modulo while rounding the created packet size up
to a multiple of RV_PACKET_MEM_SIZE in sss_packet_new. This fixes
potentially packet buffer overflows with certain body sizes.
---
 src/responder/common/responder_packet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/responder/common/responder_packet.c b/src/responder/common/responder_packet.c
index 1a201c1..4f5e110 100644
--- a/src/responder/common/responder_packet.c
+++ b/src/responder/common/responder_packet.c
@@ -75,7 +75,7 @@ int sss_packet_new(TALLOC_CTX *mem_ctx, size_t size,
 if (!packet) return ENOMEM;
 
 if (size) {
-int n = (size + SSS_NSS_HEADER_SIZE) % RV_PACKET_MEM_SIZE;
+int n = (size + SSS_NSS_HEADER_SIZE) / RV_PACKET_MEM_SIZE;
 packet->memsize = (n + 1) * RV_PACKET_MEM_SIZE;
 } else {
 packet->memsize = RV_PACKET_MEM_SIZE;
-- 
2.8.1

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: fully qualified sysdb names for users and groups

[Jakub Hrozek]

On Thu, Jul 07, 2016 at 11:45:42AM +0200, Lukas Slebodnik wrote:
> On (07/07/16 11:39), Pavel Březina wrote:
> >On 07/06/2016 11:21 PM, Sumit Bose wrote:
> >> 
> >> ok, CI passed http://sssd-ci.duckdns.org/logs/job/47/41/summary.html
> >> 
> >> ACK
> >
> >I went through the code changes and it looks good to me. I have just one
> >nitpick for the changes in sssctl, can you move filter construction and the
> >whole magic around fqn in sssctl_find_object() to a separate function?
> too late for review :-)
> 
> Patches have been already pushed to master
>   (Thu,  7 Jul 2016 08:53:57 + (UTC))
> Jakub just forgot to send a mail.

Yes, please, send a follow-up patch.

This thread was pushed as e6b6b9f..c88b63b.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: fully qualified sysdb names for users and groups

[Lukas Slebodnik]

On (07/07/16 11:39), Pavel Březina wrote:
>On 07/06/2016 11:21 PM, Sumit Bose wrote:
>> 
>> ok, CI passed http://sssd-ci.duckdns.org/logs/job/47/41/summary.html
>> 
>> ACK
>
>I went through the code changes and it looks good to me. I have just one
>nitpick for the changes in sssctl, can you move filter construction and the
>whole magic around fqn in sssctl_find_object() to a separate function?
too late for review :-)

Patches have been already pushed to master
  (Thu,  7 Jul 2016 08:53:57 + (UTC))
Jakub just forgot to send a mail.

Feel free to send patch with your proposed change.

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: fully qualified sysdb names for users and groups

[Pavel Březina]

On 07/06/2016 11:21 PM, Sumit Bose wrote:

On Wed, Jul 06, 2016 at 10:24:26PM +0200, Sumit Bose wrote:

On Wed, Jul 06, 2016 at 09:02:05PM +0200, Jakub Hrozek wrote:

On Wed, Jul 06, 2016 at 06:34:32PM +0200, Jakub Hrozek wrote:

On Wed, Jul 06, 2016 at 11:13:02AM +0200, Lukas Slebodnik wrote:

On (03/07/16 12:50), Jakub Hrozek wrote:

On Fri, Jul 01, 2016 at 11:01:53PM +0200, Jakub Hrozek wrote:

On Fri, Jul 01, 2016 at 03:24:39PM +0200, Jakub Hrozek wrote:

On Wed, Jun 29, 2016 at 07:00:13PM +0200, Jakub Hrozek wrote:

On Wed, Jun 29, 2016 at 04:36:23PM +0200, Sumit Bose wrote:

On Wed, Jun 29, 2016 at 12:05:42PM +0200, Lukas Slebodnik wrote:

On (29/06/16 11:54), Pavel Březina wrote:

On 06/28/2016 06:24 PM, Jakub Hrozek wrote:

Hi,

here is my branch that implements using the fully qualified names in sysdb
for users and groups:
  https://github.com/jhrozek/sssd/commits/sysdb-fqdn

The SSSD code changes are done. There are some downstream tests that are
failing (some sudo and some HBAC tests), but I think I can chase those
down quickly.

But -- I still don't have the upgrade code written and I only tested the
IPA overrides and IPA-AD trust code manually, so there might be a lot of
bugs there. It is also a lot of stuff to review.

With that in mind, I would like to ask other developers for opinions --
should we still pursue pushing this refactoring into 1.14? Or should we
release 1.14.0 first and push this patchset as the first thing into
1.15?


Well there are few places where we told ourselves that it will be fixed as a
side effect of those patches. Given the scale I'm fine with postponing it to
1.15 but than we need to fix those.

However, it still may be better to do it for 1.14 if doable.


+1 for 1.14 if possible


I would prefer this as well. My first tests didn't went too bad. Besides
other things I tested Smartcard and 2-factor authentication which both
worked as expected.

I added 3 fixes to my copy at
https://fedorapeople.org/cgit/sbose/public_git/sssd.git/log/?h=jhrozek_sysdb_fqdn


Thank you for the fixes and checking the code in general, I will check
these out later.


Thanks, merged to the branch.





This first is needed if there is no home-directory defined in AD. The
other 2 I came across while checking if lookups by UPN with alternative
suffix are working. The lookups were working but the output changed
depending on the lookup history. The functionality from the patches is
not present in current master as well so chances are that there might
similar issues here.



About cache updates. I think this is the critical part in the decision
here. If updates are not working or are too complex we cannot commit the
refactoring.


Yes. The most complex part is parsing the qualified names which might
be in any format, given by the full_name_format option. To solve this,
I just pass a name context to the upgrade function.

The upgrade so far doesn't look too bad to me, at least users and groups
from main and trusted domains work:
 
https://github.com/jhrozek/sssd/commit/8492cfeaa3ca789c204e78bfdaebdfdb38c81146
I still need to upgrade override objects and sudo rules because these
contain usernames as well.

The memberof plugin is forcibly disabled during the upgrade and all
attributes are written in a single transaction from the upgrade. I think
this is safer, especially with complex group memberships, where the
memberof plugin might otherwise process a single entry multiple times.


The upgrade is done now. Please review..I tested user and group lookups
also with overrides. I force-pushed the code into my github branch.

I also fixed the timestamp integration tests.





Maybe we should save a copy of the old cache to allow downgrades
without loosing the cache content if something goes wrong with the
update?


Yes. We can call sssctl to do this.


sssctl can do the upgrade now, I just need to add the upgrade to the
specfile..

The only thing remaining in this branch is to fix the failing downstream
tests IMO..


Hi,

I fixed one rather embarassing issue in sudo code related to handling of
the special 'ALL' case. There are issues looking up service accounts, but
I'm not sure if they are related to this patchset or broken in master.,
I will investigate more.

There are some failures in downstram HBAC test, but I can't reproduce
them so far locally. I will keep digging.

Have a nice weekend!


Hi,
I found some other bugs. To ease the review, I pushed them to:
https://github.com/jhrozek/sssd/commits/sysdb-fqdn
I included Pavel's sudo patch as well because otherwise I would do some
changes and then he would overwrite them back :)

So far I kept the patches separate mostly to make it clear where the
bugs were, but I would like to squash them later.

More downstream tests were fixed with these patches, but a new set is
running at the moment.


There are some failures in nss-srv unit test
[ RUN  ] test_nss_getorigbyname
"orig_name@nss_test" != "orig_name"

[SSSD] Re: [PATCH] PAM/KRB5: optional otp and password prompting

[Sumit Bose]

On Wed, Jul 06, 2016 at 10:03:12AM -0400, Nathaniel McCallum wrote:
> On Wed, 2016-07-06 at 15:55 +0200, Sumit Bose wrote:
> > On Wed, Jul 06, 2016 at 09:03:45AM -0400, Nathaniel McCallum wrote:
> > > 
> > > Patch WFM and looks clean. My only question is on the overall
> > 
> > Thank you very much for testing.
> > 
> > > 
> > > prompting.
> > > 
> > > Currently we have:
> > >   * 1FA:
> > >     - "Password"
> > >   * 2FA (required):
> > >     - "First Factor"
> > >     - "Second Factor"
> > >   * 2FA (optional):
> > >     - "First Factor or Password"
> > >     - "Second Factor, press return for Password authentication"
> > > 
> > > This all seems a little verbose to me. It also seems like it
> > > reveals
> > > technical details that don't matter and/or might confuse the user.
> > > 
> > > Is there a reason not to do the following?
> > > 
> > >   * 1FA:
> > >     - "Password"
> > >   * 2FA (required):
> > >     - "Password"
> > >     - "Token
> > > Code"
> > >   * 2FA (optional):
> > >     - "Password"
> > >     - "Token Code (optional)"
> > > 
> > > This seems to me simpler, shorter and more descriptive.
> > > Alternatively:
> > > 
> > >   * 1FA:
> > >     - "Password"
> > >   * 2FA (required):
> > >     - "Password"
> > >     -
> > > "Second Factor"
> > >   * 2FA (optional):
> > >     - "Password"
> > >     - "Second Factor
> > > (optional)"
> > > 
> > > Whatever we decide to do, this patch has my ACK. So feel free to
> > > just
> > > change the string and merge it.
> > 
> > When the 2FA prompting was added in the previous release I think I
> > asked
> > for alternative suggestions as well. My main reasons, especially with
> > the 'First Factor' prompt were first to make the user aware that
> > something new is happen. New in the sense that the user should be
> > prepared to enter the second factor in a new prompt and not put the
> > combined password in the 'Password' prompt as he was used to before.
> > 
> > And second I wasn't sure if users are aware that the first factor is
> > typically their long term password. Iirc the first factor with RSA
> > tokens are typically referred to as PIN and asking for a password
> > might
> > confuse the users.
> > 
> > Maybe we should reach out to UX designers to see what they recommend?
> 
> Let's just go with this:
> 
>  * 1FA:
>    - "Password"
>  * 2FA:
>    - "First Factor"
>    - "Second Factor"
>  * 2FA, optional:
>    - "First Factor"
>    - "Second Factor (optional)"
> 
> We already implement the first two cases, so adding one word is just a
> small change.

New version attached with the prompts changed accordingly.

bye,
Sumit
From b0065408b1950b2ed3a5a5d132383c0cfb1ea144 Mon Sep 17 00:00:00 2001
From: Sumit Bose 
Date: Thu, 26 May 2016 13:20:59 +0200
Subject: [PATCH] PAM/KRB5: optional otp and password prompting

Depending on the available Kerberos pre-authentication methods pam_sss
will prompt the user for a password, 2 authentication factors or both.

Resolves https://fedorahosted.org/sssd/ticket/2988
---
 src/providers/krb5/krb5_child.c | 85 +++--
 src/sss_client/pam_message.h|  2 +
 src/sss_client/pam_sss.c| 14 ++-
 src/sss_client/sss_cli.h|  5 +++
 4 files changed, 101 insertions(+), 5 deletions(-)

diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index 
3b3ebd9a9c943a0aaa28a89338dddba4c045b521..a0a0f74d7e39866828c1c9ee4b18e57c36a30bb9
 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -54,6 +54,7 @@ struct krb5_req {
 char* name;
 krb5_creds *creds;
 bool otp;
+bool password_prompting;
 char *otp_vendor;
 char *otp_token_id;
 char *otp_challenge;
@@ -586,24 +587,87 @@ static krb5_error_code sss_krb5_responder(krb5_context 
ctx,
   krb5_responder_context rctx)
 {
 struct krb5_req *kr = talloc_get_type(data, struct krb5_req);
+const char * const *question_list;
+size_t c;
+const char *pwd;
+int ret;
+krb5_error_code kerr;
 
 if (kr == NULL) {
 return EINVAL;
 }
 
+question_list = krb5_responder_list_questions(ctx, rctx);
+
+if (question_list != NULL) {
+for (c = 0; question_list[c] != NULL; c++) {
+DEBUG(SSSDBG_TRACE_ALL, "Got question [%s].\n", question_list[c]);
+
+if (strcmp(question_list[c],
+   KRB5_RESPONDER_QUESTION_PASSWORD) == 0) {
+kr->password_prompting = true;
+
+if ((kr->pd->cmd == SSS_PAM_AUTHENTICATE
+|| kr->pd->cmd == SSS_PAM_CHAUTHTOK_PRELIM
+|| kr->pd->cmd == SSS_PAM_CHAUTHTOK)
+&& sss_authtok_get_type(kr->pd->authtok)
+ == SSS_AUTHTOK_TYPE_PASSWORD) 
{
+ret = sss_authtok_get_password(kr->pd->authtok, , 
NULL);
+if (ret != EOK) {

[SSSD] Re: [PATCH] LDAP: Lookup services by all protocols unless a protocol is specified

[Sumit Bose]

resend
- Forwarded message from Sumit Bose  -

Date: Wed, 6 Jul 2016 11:13:48 +0200
From: Sumit Bose 
To: sssd-devel@lists.fedorahosted.org
Subject: Re: [SSSD] [PATCH] LDAP: Lookup services by all protocols unless a
protocol is specified
Message-ID: <20160706091348.GD29143@p.Speedport_W_724V_Typ_A_05011603_00_009>
References: <20160705103025.GB24232@hendrix>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160705103025.GB24232@hendrix>
User-Agent: Mutt/1.6.1 (2016-04-27)

On Tue, Jul 05, 2016 at 12:30:25PM +0200, Jakub Hrozek wrote:
> Hi,
> 
> the attached patch makes service lookups great again.
> 
> To reproduce, just run:
> getent service -s sss ldap
> before the patch we would look up ipService="" because DP gives us an
> empty string after the recent DP patches.

> From ba9834637b3cc0d7d98f704ba70f9dcb6f9a70e9 Mon Sep 17 00:00:00 2001
> From: Jakub Hrozek 
> Date: Tue, 5 Jul 2016 12:23:23 +0200
> Subject: [PATCH] LDAP: Lookup services by all protocols unless a protocol is
>  specified
> 
> The DP refactoring changed the way we handle strings from sbus. We no
> longer receive NULL strings, but empty strings instead.
> ---
>  src/providers/ldap/ldap_id_services.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/providers/ldap/ldap_id_services.c 
> b/src/providers/ldap/ldap_id_services.c
> index 
> 77215127b53297d840eaa4d2f35a75eedb085e43..db47e3fc55eea969371d61c3c5ac7f818196f3d5
>  100644
> --- a/src/providers/ldap/ldap_id_services.c
> +++ b/src/providers/ldap/ldap_id_services.c
> @@ -114,7 +114,7 @@ services_get_send(TALLOC_CTX *mem_ctx,
>  ret = sss_filter_sanitize(state, name, _name);
>  if (ret != EOK)  goto error;
>  
> -if (protocol) {
> +if (protocol && protocol[0] != '\0') {
>  ret = sss_filter_sanitize(state, protocol, _protocol);
>  if (ret != EOK)  goto error;
>  }

since the sysdb calls later on in the request expect protocol==NULL as
well I would suggest something like 

 state->sysdb = sdom->dom->sysdb;
 state->name = name;
 state->protocol = protocol;
+if (state->protocol[0] == '\0') {
+state->protocol = NULL;
+}
 state->filter_type = filter_type;
 state->noexist_delete = noexist_delete;


in only use state->protocol in the request.

bye,
Sumit
> -- 
> 2.4.11
> 

> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org


- End forwarded message -
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org