[PATCHES] Run PAC responder and subdomains provider for IPA domains

[Sumit Bose]

Hi,

the following two patches fix https://fedorahosted.org/sssd/ticket/1613
by running the PAC responder and the subdomains provider for an IPA
domain with being explicitly configured, i.e. the sssd.conf file
created by ipa-client-install does not have to be changed if trusts are
enabled in the server. Please see commit messages for further details.

bye,
Sumit

[Jakub Hrozek]

On Tue, Nov 13, 2012 at 01:43:25PM +0100, Sumit Bose wrote:
> Hi,
>
> the following two patches fix https://fedorahosted.org/sssd/ticket/1613
> by running the PAC responder and the subdomains provider for an IPA
> domain with being explicitly configured, i.e. the sssd.conf file
> created by ipa-client-install does not have to be changed if trusts are
> enabled in the server. Please see commit messages for further details.
>
> bye,
> Sumit

Hi,

The functionality seems to be working as expected for both a client enrolled
with an IPA server with trusts and without. I only have a couple of comments:

Maybe it would be nice to split the first patch into two - one that
includes the new functions add_string_to_list and string_in_list and one
that uses them in monitor. I don't require this change, though.

Given that the pac responder is only compiled on systems with reasonably
new libkrb5, I think the pac service should only be added for
configurations that have the pac service compiled in.

Is there a way to disable the automatic enabling of pac responder for
environments that only want to run the minimal required set of services?
Maybe we could have a separate ticket to not spawn pac responder if the
subdomains_provider is explicitly set to none. This doesn't block
inclusion of new patches either.

There is a compilation warning:
src/monitor/monitor.c: In function ‘add_implicit_services’:
src/monitor/monitor.c:855:5: warning: passing argument 2 of
‘string_in_list’ from incompatible pointer type [enabled by default]
In file included from src/monitor/monitor.c:22:0:
./src/util/util.h:542:6: note: expected ‘const char **’ but argument is
of type ‘char **’

I asked our docs expert to check the man page paragraph in the second
patch, here is what she proposed:
+    <refsect1 id='subdomains_provider'>
+        <title>SUBDOMAINS PROVIDER</title>
+        <para>
+            The IPA subdomains provider behaves slightly differently
+            if it is configured explicitly or implicitly.
+        </para>
+        <para>
+            If the option 'subdomains_provider = ipa' is found in the
+            domain section of sssd.conf, the IPA subdomains provider is
+            configured explicitly, and all subdomain requests are sent to the
+            IPA server if necessary.
+        </para>
+        <para>
+            If the option 'subdomains_provider' is not set in the domain
+            section of sssd.conf but there is the option 'id_provider = ipa,' the IPA subdomains
+            provider is configured implictly. In this case, if a subdomain
+            request fails and indicates that the server does not support
+            subdomains, i.e. is not configured for trusts, the IPA subdomains
+            provider is disabled. After an hour or after the IPA provider goes
+            online, the subdomains provider is enabled again.
+        </para>
+    </refsect1>

[Sumit Bose]

On Tue, Nov 13, 2012 at 05:23:13PM +0100, Jakub Hrozek wrote:
> On Tue, Nov 13, 2012 at 01:43:25PM +0100, Sumit Bose wrote:
> > Hi,
> >
> > the following two patches fix https://fedorahosted.org/sssd/ticket/1613
> > by running the PAC responder and the subdomains provider for an IPA
> > domain with being explicitly configured, i.e. the sssd.conf file
> > created by ipa-client-install does not have to be changed if trusts are
> > enabled in the server. Please see commit messages for further details.
> >
> > bye,
> > Sumit
>
> Hi,
>
> The functionality seems to be working as expected for both a client enrolled
> with an IPA server with trusts and without. I only have a couple of comments:

Thank you for the review.
> Maybe it would be nice to split the first patch into two - one that
> includes the new functions add_string_to_list and string_in_list and one
> that uses them in monitor. I don't require this change, though.

done
> Given that the pac responder is only compiled on systems with reasonably
> new libkrb5, I think the pac service should only be added for
> configurations that have the pac service compiled in.

done, I hope the way I tried to avoid ifdef's in the main code is not
too pedantic.
> Is there a way to disable the automatic enabling of pac responder for
> environments that only want to run the minimal required set of services?
> Maybe we could have a separate ticket to not spawn pac responder if the
> subdomains_provider is explicitly set to none. This doesn't block
> inclusion of new patches either.

Simo had the idea to add an option for the PAM responder to
enable/disable the PAC responder as well. I'm not sure if he already
opened a ticket for this. I would prefer this approach because there
might be setups where you do not want/need the subdomains provider but
the PAC responder.
> There is a compilation warning:
> src/monitor/monitor.c: In function ‘add_implicit_services’:
> src/monitor/monitor.c:855:5: warning: passing argument 2 of
> ‘string_in_list’ from incompatible pointer type [enabled by default]
> In file included from src/monitor/monitor.c:22:0:
> ./src/util/util.h:542:6: note: expected ‘const char **’ but argument is
> of type ‘char **’

fixed
> I asked our docs expert to check the man page paragraph in the second
> patch, here is what she proposed:
>
> <refsect1 id='subdomains_provider'>
>    <title>SUBDOMAINS PROVIDER</title>
>    <para>
>        The IPA subdomains provider behaves slightly differently
>        if it is configured explicitly or implicitly.
>    </para>
>    <para>
>        If the option 'subdomains_provider = ipa' is found in the
>        domain section of sssd.conf, the IPA subdomains provider is
>        configured explicitly, and all subdomain requests are sent to the
>        IPA server if necessary.
>    </para>
>    <para>
>        If the option 'subdomains_provider' is not set in the domain
>        section of sssd.conf but there is the option 'id_provider = ipa,' the IPA subdomains
>        provider is configured implictly. In this case, if a subdomain
>        request fails and indicates that the server does not support
>        subdomains, i.e. is not configured for trusts, the IPA subdomains
>        provider is disabled. After an hour or after the IPA provider goes
>        online, the subdomains provider is enabled again.
>    </para>
>
> </refsect1>

added the new version.

New patches attached.

bye,
Sumit
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[Jakub Hrozek]

On Tue, Nov 13, 2012 at 10:04:25PM +0100, Sumit Bose wrote:
> On Tue, Nov 13, 2012 at 05:23:13PM +0100, Jakub Hrozek wrote:
> > On Tue, Nov 13, 2012 at 01:43:25PM +0100, Sumit Bose wrote:
> > > Hi,
> > >
> > > the following two patches fix https://fedorahosted.org/sssd/ticket/1613
> > > by running the PAC responder and the subdomains provider for an IPA
> > > domain with being explicitly configured, i.e. the sssd.conf file
> > > created by ipa-client-install does not have to be changed if trusts are
> > > enabled in the server. Please see commit messages for further details.
> > >
> > > bye,
> > > Sumit
> >
> > Hi,
> >
> > The functionality seems to be working as expected for both a client enrolled
> > with an IPA server with trusts and without. I only have a couple of comments:
>
> Thank you for the review.
> > Maybe it would be nice to split the first patch into two - one that
> > includes the new functions add_string_to_list and string_in_list and one
> > that uses them in monitor. I don't require this change, though.
>
> done
> > Given that the pac responder is only compiled on systems with reasonably
> > new libkrb5, I think the pac service should only be added for
> > configurations that have the pac service compiled in.
>
> done, I hope the way I tried to avoid ifdef's in the main code is not
> too pedantic.

Works for me :-)
> > Is there a way to disable the automatic enabling of pac responder for
> > environments that only want to run the minimal required set of services?
> > Maybe we could have a separate ticket to not spawn pac responder if the
> > subdomains_provider is explicitly set to none. This doesn't block
> > inclusion of new patches either.
>
> Simo had the idea to add an option for the PAM responder to
> enable/disable the PAC responder as well. I'm not sure if he already
> opened a ticket for this. I would prefer this approach because there
> might be setups where you do not want/need the subdomains provider but
> the PAC responder.

I couldn't find the ticket so I filed
https://fedorahosted.org/sssd/ticket/1642
> > There is a compilation warning:
> > src/monitor/monitor.c: In function ‘add_implicit_services’:
> > src/monitor/monitor.c:855:5: warning: passing argument 2 of
> > ‘string_in_list’ from incompatible pointer type [enabled by default]
> > In file included from src/monitor/monitor.c:22:0:
> > ./src/util/util.h:542:6: note: expected ‘const char **’ but argument is
> > of type ‘char **’
>
> fixed
> > I asked our docs expert to check the man page paragraph in the second
> > patch, here is what she proposed:
> >
> > <refsect1 id='subdomains_provider'>
> >    <title>SUBDOMAINS PROVIDER</title>
> >    <para>
> >        The IPA subdomains provider behaves slightly differently
> >        if it is configured explicitly or implicitly.
> >    </para>
> >    <para>
> >        If the option 'subdomains_provider = ipa' is found in the
> >        domain section of sssd.conf, the IPA subdomains provider is
> >        configured explicitly, and all subdomain requests are sent to the
> >        IPA server if necessary.
> >    </para>
> >    <para>
> >        If the option 'subdomains_provider' is not set in the domain
> >        section of sssd.conf but there is the option 'id_provider = ipa,' the IPA subdomains
> >        provider is configured implictly. In this case, if a subdomain
> >        request fails and indicates that the server does not support
> >        subdomains, i.e. is not configured for trusts, the IPA subdomains
> >        provider is disabled. After an hour or after the IPA provider goes
> >        online, the subdomains provider is enabled again.
> >    </para>
> >
> > </refsect1>
>
> added the new version.
>
> New patches attached.

Ack to all three patches.