On Wed, 2012-07-04 at 23:10 +0300, Alexander Bokovoy wrote:
> On Wed, 04 Jul 2012, Sumit Bose wrote:
> >On Wed, Jul 04, 2012 at 08:57:44PM +0300, Alexander Bokovoy wrote:
> >> Hi,
> >>
> >> when chasing what looked like ccache corruption with Sumit, I've found
> >> yet another issue: use of local stack variable in long-time living code.
> >>
> >> This local stack use was absent in the original patch version and was
> >> proposed by Sumit on one of reviews. It worked for us by luck, it should
> >> not have.
> >>
> >> Hence, the patch that fixes the issue by moving service principal to a
> >> longer term storage (ipasam private struct).
> >>
> >> In order to avoid ccache corruption we also need to move back to
> >> in-memory ccache. When multiple LSASD and smbd processes try to auth
> >> against LDAP in ipasam, they may write to the same ccache (common
> >> ccache) when another process reads from it. This is not what we need.
> >>
> >> And writing to a persistent on-disk ccache is not needed anyway, as
> >> smbldap connections re-authenticate themselves (smbldap connections
> >> expire in few minutes).
> >>
> >> The patch also removes kerberos operations that are not needed when
> >> using memory ccache.
> >>
> >> --
> >> / Alexander Bokovoy
> >
> >This patch works for me and I have no objections using the in-memory
> >ccache, so ACK from my side, but please wait for Simo's answer before
> >pushing.
> >
> >Simo, you were in favour of using the default cache, do you
> >agree as well?
> >
> >Before pushing please fix:
> >
> >ipa_sam.c:3164:8: warning: unused variable 'ccache_name' [-Wunused-variable]
> 
> After talking with Sumit and researching I've changed back to use
> on-disk ccache and avoid re-initing on every callback call.

Thank, this is what I would have asked anyway.

> Attached patch should be good. The only trouble is that you'll need to
> re-run 'ipa-adtrust-install' as smb.conf's parameter we use has been
> changed to a different one. That or you can manually run
> 
> net conf setparm 'global' 'ipasam:principal template' '$FQDN@$REALM'
> 
> where $FQDN is the fully-qualified domain name of the IPA server where
> smbd will run and $REALM is IPA realm name.

We do not need to store this information in the configuration
explicitly, we have all that info already.

So NACK, please remove this configuration option completely and fetch
fqdn from the system (gethostname) and the realm from the krb5 context.

> Note that I had to do this change to avoid manipulating by ipasam:principal
> name as I need both client and server principals to use 
> krb5_get_credentials().
> With this change ipasam will build proper principal names by itself.
> 
> Note that we cannot use use ipasam_privates.realm value because it is
> not available at the point when callback is called. It is fetched from
> ldap later, after callback has been successfully used.

See above, we have all the information we need via simple API calls to
glibc or krb5 libs, we do not need to retrieve that information by other
means.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to