Re: man page patch https://fedorahosted.org/sssd/ticket/2683
by Jakub Hrozek
On Fri, Nov 20, 2015 at 01:31:10PM -0500, Dan Lavu wrote:
> Arguably just 'git diff' is the git formatted way, =D
> but here you go.
We prefer to attach the output of git-format-patch to the e-mail :-)
>
> From 8b2d160abbf6eb6ff894ddb2e687983168d1da51 Mon Sep 17 00:00:00 2001
> From: Dan Lavu <dlavu(a)redhat.com>
> Date: Thu, 19 Nov 2015 06:36:25 -0500
> Subject: [PATCH] Man page edit for
>
> https://fedorahosted.org/sssd/ticket/2683
> ---
> src/man/sssd.conf.5.xml | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
> index 573f421a7d885d28d5dbc03423e6c6dd84d7b8fd..dcf66b205aaf2bc49c7ca94f67f5239acf6aa9fc 100644
> --- a/src/man/sssd.conf.5.xml
> +++ b/src/man/sssd.conf.5.xml
> @@ -509,6 +509,10 @@ subdomain_inherit = ldap_purge_cache_timeout
> <para>
> Default: none
> </para>
> + <para>
> + Note: This option only works with the IPA and AD
> + provider.
> + </para>
> </listitem>
> </varlistentry>
> </variablelist>
> --
> 2.5.0
I fixed the indentation here:
https://github.com/jhrozek/sssd/commit/3e6404f7cfd493a7599a0b48b5b074c22c...
And ran the patch through CI together with Dan's other patch:
http://sssd-ci.duckdns.org/logs/job/34/02/summary.html
ACK
8 years, 4 months
[PATCHES] ldap: skip sdap_save_grpmem() if ignore_group_members is set
by Sumit Bose
Hi,
please find attched the patches I came up with so far to fix
https://fedorahosted.org/sssd/ticket/2868.
sdap_save_grpmem() is used to refresh the members of groups in the cache
based on the data received from LDAP, external entries which are
retained and the primary group stored in the user objects. If
ignore_group_members is set to True there is of course no member data
from LDAP and calling sdap_save_grpmem() in this case would remove all
members entries which where created during initgroups lookups.
So if ignore_group_members is True the members list of groups are
managed only indirectly by the initgroups calls and the list should not
be modified during group lookup to avoid loosing information. This is
what the first patch tries to achieve. Since I think sdap_save_grpmem()
shouldn't be called at all in this case I added the SSSDBG_CRIT_FAILURE
which should help to identify in which cases sdap_save_grpmem() is
called to see if it is valid or not.
This leads to a question I didn't found a good answer so far.
sdap_save_grpmem() is only called from sdap_save_groups() and only "if
(twopass && !populate_members)". I have to admit that I'm currently not
sure about the meaning and usage of the populate_members boolean. The
commit message introducing it says
"""
commit c4e120cec00807ef4fe7a0b7af96f5b0eece8a4b
Author: Ralf Haferkamp <rhafer(a)suse.de>
Date: Mon Oct 11 17:13:58 2010 +0200
Shortcut for save_group() to accept sysdb DNs as member attributes
Addtional parameter "populate_members" for save_group() and save_groups()
to indicate that the "member" attribute of the groups is populated with
sysdb DNs of the members (instead of LDAP DNs).
"""
while there is the call in sdap_get_groups_done():
ret = sdap_save_groups(state, state->sysdb, state->dom, state->opts,
state->groups, state->count,
!state->dom->ignore_group_members, NULL,
state->lookup_type == SDAP_LOOKUP_SINGLE,
&state->higher_usn);
where populate_members is directly related to ignore_group_members which
is imo not related to the original use case. populate_members might not
be a good choice of name in the original commit but I wonder if the
meaning has changed over time?
The second patch just avoids a use-less group lookup if the primary
group is already in the cache.
bye,
Sumit
8 years, 5 months
[PATCH] p11: enable ocsp checks
by Sumit Bose
Hi,
this patch switches on the Online Certificate Status Protocol (OCSP)
checks while validation the certificate. This is done by calling
CERT_EnableOCSPChecking() before doing the validation. The main part of
the patch makes this configurable.
Since I expect that certificate validation will need more tuning option
in future I didn't add a new option of switch OCSP off and on but a more
generic one called 'certificate_verification' which accepts a comma
separated list of parameters. Currently only 'no_ocsp' is supported.
Currently this option is tested indirectly because I generated the test
certificates with IPA they contain the OCSP data. But since the OCSP
check, which is now on by default, requires on-line access to the
referenced OCSP server the OCSP check must be disabled for the
unit-test.
bye,
Sumit
8 years, 5 months
[PATCH] p11: check if cert is valid before selecting it
by Sumit Bose
Hi,
currently the p11_child does not continue to search for more
certificates if the first suitable certificate cannot be verified. With
this patch p11_child will continue until a valid certificate is found
(or all are checked).
As said before I'm working on improving the handling of the certificates
for the unit-test and would prefer to not spend time to add a test for
this case with the old scheme. If you agree I would open a ticket to
remind me to add tests for this case with the new scheme.
bye,
Sumit
8 years, 5 months
[PATCH] BUILD: Install polkit rules file only on platforms that support it
by Jakub Hrozek
The attached patch tries to detect if the build platform has the rules.d
polkit directory and only installs the file from contrib if so. As an
additional improvement we might add a config option that specifies the
directory directly, but I don't think it's needed at the moment.
I ran a local mock build of epel-6 and fedora and those passed. Now the
patch is in CI to check Debian.
Sorry again for the careless review of the first patch.
8 years, 5 months
[PATCH] TEST: Fix warning: cast from pointer to integer of different size
by Lukas Slebodnik
ehlo,
you can see warnings on 32 bit machine and cmocka tests.
So the simplest reproducer is to use mock
mock --root fedora-21-i386 --resultdir . --rebuild ./sssd-1.13.90-0.fc23.src.rpm
I plan to send patch for cmocka. Therefore there is #ifndef will_return_ptr.
I would like to avoid prefixed version with sss_ because
it might be useful for other cmocka user as well.
LS
8 years, 5 months
Design Discussion: IPA sudo schema
by Pavel Březina
https://fedorahosted.org/sssd/wiki/DesignDocs/SUDOIPASchema
= IPA sudo schema support =
Related ticket(s):
* https://fedorahosted.org/sssd/ticket/1108
Related design document(s)
* https://fedorahosted.org/sssd/wiki/DesignDocs/SUDOCachingRules
== Problem statement ==
SSSD supports only standard sudo ldap schema at the moment. This has a
drawback of having the need to run compat plugin that converts IPA sudo
schema into the standard one. Once SSSD has support for IPA schema
administrators administrators can disable sudo compat tree which will
result in performance improvement.
== Use cases ==
* compat plugin may be disabled when using IPA sudo provider
== IPA sudo schema ==
IPA sudo schema is rather different than the standard one. This section
contains the description of this schema together with ldap containers
where sudo rules are stored. A relevant standard attribute is noted when
possible. '''RDN is marked in bold'''. Attributes that hold dn are
marked in italic.
==== cn=sudocmds,cn=sudo,$dc ====
This container contains definition of single commands that may be
present in sudo rules.
* objectClass = ipasudocmd
* '''ipaUniqueID'''
* sudoCmd ~ sudoCommand
* ''memberOf'' (dn of sudo command group)
==== cn=sudocmdgroups,cn=sudo,$dc ====
This container contains definition of command groups that may be present
in sudo rules.
* objectClass = ipasudocmdgroup
* ipaUniqueID
* '''cn'''
* ''member'' (dn of sudo command)
==== cn=sudorules,cn=sudo,$dc ====
This container contains definition of sudo rules.
* objectClass = ipasudorule
* '''ipaUniqueID'''
* cn
* ipaEnabledFlag
* ipaSudoOpt ~ sudoOption
* ''ipaSudoRunAs'' ~ sudoRunAsUser (dn of user or group of users)
* ''ipaSudoRunAsGroup'' ~ sudoRunAsGroup (dn of group)
* ''memberAllowCmd'' (dn of sudo command or command group)
* ''memberDenyCmd'' (dn of sudo command or command group)
* ''memberHost'' ~ sudoHost (dn of ipa enrolled machine or hostgroup)
* ''memberUser'' ~ sudoUser (dn of user or group of users)
* hostMask (ip/mask)
The following attibures have a special meaning and can contain only
value "all". For example if cmdCategory is present, it is equivalent to
sudoCommand=ALL.
* cmdCategory ~ sudoCommand
* hostCategory ~ sudoHost
* ipaSudoRunAsGroupCategory ~ sudoRunAsGroup
* ipaSudoRunAsUserCategory ~ sudoRunAsUser
* userCategory ~ sudoUser
The following attributes are used to contain external objects not known
to IPA nor SSSD. Since SSSD by design provides rules only to users and
groups known to it, we can safely ignore those attributes.
* externalHost
* externalUser
* ipaSudoRunAsExtGroup
* ipaSudoRunAsExtUser
== Overview of the solution ==
We will again use rules, smart and full refresh similar to what we do in
ldap provider. Since we are working with three containers, it is not
very simply to translate everything at once into current standard sudo
schema that we use inside SSSD, because it would make changes in
commands and command groups hard to propagate. Instead we will keep
command and command groups stored separately and translate it into
sudoCommand in responder on the fly.
We will take advantage of using an IPA server and translate dn into
names by parsing dn when possible.
== Implementation details ==
==== Full refresh ====
* download everything under cn=sudo,$dc that applies to this host
* store only commands and command groups that are present in at least
one rule
* convert what possible to sudo schema but leave references to commands
and command groups for further processing in responder
==== Smart refresh ====
* download everything under cn=sudo,$dc that applies to this host newer
than last usn value
* if new command or command group is downloaded store it only if it is
present in changed rule
* if a rule contains command or command group that is not yet present in
sysdb, fetch it with dereference or single lookup
==== Rules refresh ====
* refresh expired rules and commands and command groups that are present
in those rules
=== Configuration changes ===
* new option sudo_schema = [ipa|native]
=== How To Test ===
* existing tests can be used, only switching ldap server for IPA
=== Authors ===
* Pavel Březina <pbrezina(a)redhat.com>
8 years, 5 months
Replace hexadecimal debug level code in log messages with a text tag
by Petr Cech
Hello all,
I would like to work on:
https://fedorahosted.org/sssd/ticket/2808
However, there are two different opinions.
Ticket description says:
........................
Currently a log message of SSSD like e.g.
# (Mon Jun 29 22:10:59 2015) [sssd[be[lab.runlevelone.lan]]]
[be_get_account_info] (0x0200): Got request for [0x1001][1][name=dlavu]
has the debug level given by '(0x0200)'. For the inexperienced reader it
is hard to see if the message indicates an error or is only some
informational messages.
It would be good to replace them which short text tags which better
indicates the character and severity of the message. E.g. something like
ERR_CRIT, INFO_DATA, TRACE_FUNC ...
-----
Jakub said:
One issue that I realized is that we might want to add the string
representation in addition to hexa to avoid breaking existing scripts.
But on the other hand, I'm not sure if we should care too much about
keeping compatibility of debug logs...
And Lukas said:
I use very often hexadecimal representation for filtering huge log file.
So it should not be replaced. It's much simpler (and shorter) to write
regex for hexadecimal numbers.
-----
In my opinion, we should write numerical values if and only if they
represent values (uid for example), not enum constant.
In this situation I feel that filtering is important.
I suggest use:
# (Mon Jun 29 22:10:59 2015) [sssd[be[lab.runlevelone.lan]]]
[be_get_account_info] (0x0200 | INFO_FUNC_DATA): Got request for
[0x1001][1][name=dlavu]
What is your opinion, please?
Regards
Petr
8 years, 5 months
