On Mon, Aug 15, 2016 at 04:03:17PM +0200, Petr Cech wrote:
On 08/12/2016 04:05 PM, Petr Cech wrote:
> On 08/12/2016 03:36 PM, Jakub Hrozek wrote:
> > On Fri, Aug 12, 2016 at 02:51:21PM +0200, Petr Cech wrote:
> > > On 08/12/2016 11:27 AM, Jakub Hrozek wrote:
> > > > On Wed, Aug 10, 2016 at 08:54:25AM +0200, Petr Cech wrote:
> > > > > Sorry, I experienced some issue with mailing list.
> > > > > So I send it again.
> > > > >
> > > > > -------- Forwarded Message --------
> > > > > Subject: Re: [SSSD] Re: [PATCH SET] AD_PROVIDER:
ad_enabled_domains
> > > > > Date: Tue, 9 Aug 2016 17:29:38 +0200
> > > > > From: Petr Cech <pcech(a)redhat.com>
> > > > > To: sssd-devel(a)lists.fedorahosted.org
> > > > >
> > > > > On 08/09/2016 11:07 AM, Jakub Hrozek wrote:
> > > > > > On Mon, Jul 25, 2016 at 06:18:28PM +0200, Petr Cech wrote:
> > > > > > > > Hello,
> > > > > > > >
> > > > > > > > there is fixed patch set attached.
> > > > > > > >
> > > > > > > > Segmentation fault was caused by wrong pointer
:-(, sorry.
> > > > > > > >
> > > > > > > > This new patch set has new debug message. I am
open to dissccus the
> > > > > > > > debug_level and content of message. Any improving
idea?
> > > > > > > >
> > > > > > > > I hit one issue during testing -- sometimes if I
am connected to
> > > > > > > > subdomain
> > > > > > > > and I enable only sibling subdomain (the master
is added
> > > > > > > > automaticaly) and
> > > > > > > > forest root is not enabled -- I see only master
and sibling not.
> > > > > > > > But if I
> > > > > > > > added sleep for cycle (for using dbg) to
function
> > > > > > > > ad_subdomains_init()
> > > > > > > > everythink is OK.
> > > > > > > > Any idea?
> > > > > > Can you test that case with valgrind? This sounds like
some
> > > > > > uninitilized
> > > > > > variable condition.
> > > > >
> > > > >
> > > > > I didn't run valgrind but I have new information.
> > > > >
> > > > > If you clear the cache and reset sssd, first attempt to obtain
> > > > > information
> > > > > about user from sibling domain fails. The second and the other
> > > > > attempts runs
> > > > > correctly.
> > > > >
> > > > > I see that the sibling domain is enabled. But if I look more
> > > > > carefully there
> > > > > is message in log (gamma.domain.bootes is sibling domain):
> > > > >
> > > > > [sssd[be[beta.domain.bootes]]] [dp_req_new] (0x0020): Unknown
domain:
> > > > > gamma.domain.bootes
> > > > >
> > > > > First attempt should works too but you should wait nearly
exactly 6
> > > > > seconds
> > > > > after restart sssd.
> > > > >
> > > > > New patch set is attached.
> > > >
> > > > I can't start SSSD with these patches:
> > > > (Fri Aug 12 11:25:38 2016) [sssd[be[win.trust.test]]]
> > > > [dp_target_run_constructor] (0x0010): Target [subdomains]
> > > > constructor failed [22]: Invalid argument
> > > > (Fri Aug 12 11:25:38 2016) [sssd[be[win.trust.test]]]
> > > > [dp_load_targets] (0x0020): Unable to load target [subdomains] [22]:
> > > > Invalid argument.
> > > > (Fri Aug 12 11:25:38 2016) [sssd[be[win.trust.test]]] [dp_init]
> > > > (0x0020): Unable to initialize DP targets [1432158209]: Internal
Error
> > > > (Fri Aug 12 11:25:38 2016) [sssd[be[win.trust.test]]]
> > > > [dp_terminate_active_requests] (0x0400): Terminating active data
> > > > provider requests
> > > >
> > > > I have:
> > > > $ git log --oneline origin/master..HEAD
> > > > 3b2f910 TESTS: Adding tests for ad_enabled_domains option
> > > > 7ac9517 AD_PROVIDER: ad_enabled_domains - other then master
> > > > fdbbd30 AD_PROVIDER: ad_enabled_domains - only master
> > > > ebaa14d AD_PROVIDER: Initializing of ad_enabled_domains
> > > > 38989af AD_PROVIDER: Add ad_enabled_domains option
> > > >
> > > > $ git rev-list origin/master..HEAD
> > > > 3b2f9106c2c5bea1681cf1f752fc5f3256a04300
> > > > 7ac9517f78dc4dcde4c4c613ec450a3f3fc8f644
> > > > fdbbd30adf9da7a3c2510029c2e8c3789a3083a0
> > > > ebaa14dd1dd0e4f55a2bc4e647ce848e36970dd2
> > > > 38989afa14bfc89712808867b80e667d34e068b3
> > >
> > > Hello Jakub,
> > >
> > > I wasn't able to reproduce your bug. Is it true that I use F23 for
> > > testing
> > > this patch for historical reasons. I should try it with F24 too.
> > >
> > > I sent whole patch set to CI,
> > >
http://sssd-ci.duckdns.org/logs/job/51/45/summary.html
> > > but I think it is not conclusive because out tests don't contain AD
> > > server.
> > >
> > > I will look at it again. But now I would like finish tests for
> > > netgroups.
> >
> > I don't think it has to do with Fedora version. Maybe my sssd.conf would
> > help:
> >
> > [domain/win.trust.test]
> > ad_domain = win.trust.test
> > krb5_realm = WIN.TRUST.TEST
> > realmd_tags = manages-system joined-with-adcli
> > cache_credentials = True
> > id_provider = ad
> > krb5_store_password_if_offline = True
> > default_shell = /bin/bash
> > ldap_id_mapping = True
> > use_fully_qualified_names = True
> > fallback_homedir = /home/%u@%d
> > ad_enable_gc = false
> > debug_level = 10
> >
> > access_provider = simple
> >
> > #ad_enabled_domains = win.trust.test, siblingdom.win.trust.test
> > #debug_level = 7
> >
> > dyndns_update = false
>
> Thanks,
>
> I see now where's the problem. I didn't try to comment
> ad_enabled_domains in config for long time. If this option missing it
> will crash.
>
> [dp_target_run_constructor] (0x0010): Target [subdomains] constructor
> failed [22]: Invalid argument
>
> I hope it will be easy to fix it.
Hello,
I fixed little bug (wrong return code for missing option)
in ad_get_enabled_domains().
New patch set is attached.
There is still one strange behaviour:
If you clear the cache and reset sssd, first attempt to obtain
information about user from sibling domain fails. The second and the other
attempts runs correctly.
I see that the sibling domain is enabled. But if I look more
carefully there is message in log (gamma.domain.bootes is sibling
domain):
[sssd[be[beta.domain.bootes]]] [dp_req_new] (0x0020): Unknown domain:
gamma.domain.bootes
First attempt should works too but you should wait nearly exactly 6
seconds after restart sssd.
I think this is an unrelated bug where the subdomains take a bit of time
to be discovered. I think the responder was waiting until the subdomains
were autodiscovered, did we break this?
Anyway, it's not related to these patches (and I can't reproduce it
locally either)
I have two last small comments:
From 24d32d0eb12ddc433e64ffd6411e9e13f0067b35 Mon Sep 17 00:00:00
2001
From: Petr Cech <pcech(a)redhat.com>
Date: Fri, 13 May 2016 05:21:07 -0400
Subject: [PATCH 1/5] AD_PROVIDER: Add ad_enabled_domains option
Resolves:
https://fedorahosted.org/sssd/ticket/2828
Did you already have the manpage hunk checked by some native English
speaker?
--- a/src/man/sssd-ad.5.xml
+++ b/src/man/sssd-ad.5.xml
@@ -114,6 +114,28 @@ ldap_id_mapping = False
</varlistentry>
<varlistentry>
+ <term>ad_enabled_domains (string)</term>
+ <listitem>
+ <para>
+ The comma-separated list of the enabled Active
+ Directory domains. This is optional. If provided,
+ SSSD will not contact domains not listed in this
+ option. If not provided, all domains from AD forest
+ are enabled.
In particular, I'm not sure about the double negatives (not contact
domains not listed..)
And normally we list the default value in a separate para.
+ </para>
+ <para>
+ For proper operation, this option should be
+ specified as the lower-case version of the long
+ version of the Active Directory domain.
+ </para>
+ <para>
+ The short domain name (also known as the NetBIOS
+ or the flat name) is autodetected by the SSSD.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term>ad_server, ad_backup_server (string)</term>
<listitem>
<para>
From 807401f380d6f23e4b4594d32ecd45223e602d10 Mon Sep 17 00:00:00
2001
From: Petr Cech <pcech(a)redhat.com>
Date: Mon, 27 Jun 2016 11:51:30 +0200
Subject: [PATCH 4/5] AD_PROVIDER: ad_enabled_domains - other then master
We can skip looking up other domains if
option ad_enabled_domains doesn't contain them.
Resolves:
https://fedorahosted.org/sssd/ticket/2828
---
[...]
static errno_t ad_subdomains_process(TALLOC_CTX *mem_ctx,
struct sss_domain_info *domain,
+ const char **enabled_domains_list,
size_t nsd, struct sysdb_attrs **sd,
struct sysdb_attrs *root,
size_t *_nsd_out,
@@ -493,9 +504,10 @@ static errno_t ad_subdomains_process(TALLOC_CTX *mem_ctx,
size_t i, sdi;
struct sysdb_attrs **sd_out;
const char *sd_name;
+ const char *root_name;
errno_t ret;
- if (root == NULL) {
+ if (root == NULL && (enabled_domains_list == NULL)) {
The parens around enabled_domains_list are not needed, can you drop
them?
/* We are connected directly to the root domain. The
'sd'
* list is complete and we can just use it
*/
The other patches can be considered ACKed.