[Freeipa-devel] kpasswd and minor fixes
Rob Crittenden
rcritten at redhat.com
Thu Aug 9 21:09:51 UTC 2007
Rob Crittenden wrote:
> Simo Sorce wrote:
>> On Thu, 2007-08-09 at 16:10 -0400, Rob Crittenden wrote:
>>> Simo Sorce wrote:
>>>> Attached my latest work in creating a kpasswd daemon that proxies
>>>> password changes to ldap.
>>>> This make it possible to completely handle password changes with the
>>>> pwd-extop plugin and always use the same codepath.
>>>>
>>>> As I have been traveling the local commit queue grow up and part of
>>>> this
>>>> stuff happened before the directory reorg ...
>>>>
>>>> Patches depend one on top of each other from lower number to higher, I
>>>> omitted any changeset that has already been committed.
>>>>
>>>> Simo.
>>>>
>>> Ignoring freeipa33 and 35...
>>>
>>> The freeipa36 patch is a little odd. It removes a bunch of code the
>>> re-adds it?
>>>
>>> In any case, as a general note I think we need autoconf-enable all of
>>> IPA but it currently defaults to installing in /usr as the prefix.
>>> This patch puts things into /usr/local. So I guess it should go into
>>> /usr as well for the time being.
>>
>> Uhmm I don't think I have touched anything about locations
>> /me scrathes head
>
> In this patch it installs the daemon into /usr/local/sbin rather than
> /usr/sbin. We should be doing our directly hardcoding at least
> consistently IMHO
>
>>> We'll need to update the RPM spec file to had a BuildRequires on
>>> kerberos and openldap (unless we want to link with mozldap).
>>
>> openldap
>>
>>> Should the IPA installer generate the keytab in
>>> FILE:/var/kerberos/krb5kdc/kpasswd.keytab?
>>
>> It should have been there, blame my newbiety with mercurial.
>> /me will never get it right how the merge process really works :/
>>
>>> The realm name is hardcoded into the source. Can this be a cmd-line
>>> or config file option? Ideally it would be read out of
>>> /etc/ipa/ipa.conf.
>>
>> freeipa37.patch fixes this
>>
>>> Is kpasswd a daemon? Should it use syslog for logging?
>>
>> yeah I should changed all fprintf(stderr,.. to something that can choose
>> between syslog and stderr for debugging, it's on my TODO list
>>
>>> How many concurrent connections at a time do we expect for this
>>> service? Should we use poll() instead of select()?
>>
>> it is just for people that change password via the kpasswd protocol. It
>> should be a very low traffic daemon.
>>
>>> The return value of ldap_pwd_change() is unused. How do we know the
>>> change was successful?
>>
>> this is addressed in freeipa37.patch as well
>>
>>> There are places where result_err is set but this will never get into
>>> kpreply: to actually use the result and return something, I presume
>>> to the kerberos client. Instead it goes to done: and frees the
>>> connection.
>>
>> I think I got all of them in freeipa37.patch, but I will recheck
>>
>>> There are cases where the daemon will exit with an error. Are these
>>> really unrecoverable?
>>
>> Some times they are.
>>
>>> I don't know kerberos internals so can't really comment on much of
>>> the code.
>>
>> Np, the code works, so I think I got them right ;-)
>>
>> If you think it is good enough I will push the patch.
>>
>> Simo.
>>
>> Simo.
>>
>
> Well, maybe we should look at freeipa37.patch :-)
>
> rob
Ok, I got all the patches applied, here is a better review.
ntpd is added to README as a requirement but it isn't added to the spec
file nor do we configure it yet (not required for this patch but a
necessary TODO)
The previously mentioned /usr/local/sbin vs /usr/sbin for the
ipa_kpasswd daemon install.
What will the blacklist do to NAT'd addresses? What happens to a kpasswd
request when someone from the same IP is also doing a request? What does
the user see, in other words.
There are still cases in handle_krb_packets() where errors are passed to
done instead of kpreply, such as KRB5_KPASSWD_AUTHERROR.
Should the port be hardcoded in ldap_pwd_change()? I have enough
hardcoding in the code I've done so this isn't a hard stop :-)
rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3245 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20070809/e3ffcf3f/attachment.bin>
More information about the Freeipa-devel
mailing list