[Freeipa-devel] [PATCHES] 59-65 SSH public key management

Wed Jan 25 16:19:21 UTC 2012

Jan Cholasta wrote:
> Dne 24.1.2012 23:11, Rob Crittenden napsal(a):
>> Jan Cholasta wrote:
>>> I have updated and rebased the patches:
>>>
>>>
>>> [PATCH] 59 Add LDAP schema for SSH public keys.
>>>
>>> No changes.
>>>
>>> [PATCH] 60 Add LDAP ACIs for SSH public key schema.
>>> Requires patch 59.
>>>
>>> No changes.
>>>
>>> [PATCH] 61 Add support for SSH public keys to user and host objects.
>>> Requires patch 59 and 66.
>>>
>>> Added new virtual attribute for SSH public key fingerprints to both user
>>> and host.
>>>
>>> The ipasshuser and ipasshhost objectclasses are now automatically added
>>> to user and host objects when necessary.
>>>
>>> The --addattr issue is fixed in patch 66.
>>>
>>> [PATCH] 62 Add API initialization to ipa-client-install.
>>>
>>> Changed API context to "cli_installer".
>>>
>>> [PATCH] 63 Move the nsupdate functionality to separate function in
>>> ipa-client-install.
>>>
>>> No changes.
>>>
>>> [PATCH] 64 Update host SSH public keys on the server during client
>>> install.
>>> Requires patch 59, 61, 62, 63, 66 and 67.
>>>
>>> The host SSH public keys are now loaded from a platform specific
>>> location instead of /etc/ssh.
>>>
>>> [PATCH] 65 Configure ssh and sshd during ipa-client-install.
>>> Requires patch 67.
>>>
>>> The configuration files are now looked for in a platform specific
>>> location instead of /etc/ssh
>>>
>>>
>>> Also I have added 2 new patches to the patchset:
>>>
>>>
>>> [PATCH] 66 Base64-decode unicode values in Bytes parameters.
>>>
>>> Fix wrong handling of strings in --setattr/--addattr/--delattr.
>>>
>>> These changes make it possible to use Bytes in
>>> --setattr/--addattr/--delattr without errors.
>>>
>>> It might seem that this patch breaks the API, but it does not. Bytes
>>> parameters are currently used only for certificate attribute of host and
>>> service objects and these attributes are normalized using ipalib.x509
>>> functions, so both raw binary values and base64-encoded values are
>>> accepted. I have checked that old client works with new server without
>>> problems.
>>>
>>> [PATCH] 67 Add SSH service to platform-specific services.
>>>
>>> Add method for getting configuration directory path of a service, so
>>> that a different SSH configuration directory can be specified on
>>> different platforms.
>>>
>>>
>>> Honza
>>>
>>
>> FYI, the schema change in 59.1 didn't apply cleanly in 2.2.
>
> I did all the patches on top of master. Should I rebase them to ipa-2-2?

No, it's fine, was more a heads-up for when we commit the changes than 
anything else. It was an easy merge to do.

>>
>> This patch set lacks a way to upgrade an existing install to support SSH
>> keys.
>
> I will create a patch with the update files.
>
>>
>> Patch 61 you can drop the md5 and sha1 imports and import them from
>> ipalib.compat instead.
>
> Is this OK in ipapython?

It should be, ipa-python and ipalib should be packaged together so I 
think it is safe.

>>
>> Patch 65 should there be a way to set --ssh-trust-dns on master installs?
>
> Possibly. Should I add the ssh-related command-line options of
> ipa-client-install to ipa-server-install as well?

I guess so. It would be an easy option to miss at install time but I 
think its worthwhile. The replica installer would need this as well (and 
man pages).

>>
>> 66 is ACK and I think can be pushed separately.
>>
>> 67 not to be too pedantic but it would read better if the sshd service
>> started on its own line.
>
> I'm not sure I follow.

wellknownservices wraps the screen, it would easier to read. Note that 
it currently wraps the screen with the messagebus service too on the 
first line, it would be nice to fix that too :-)

>>
>> I installed my system with DNS and added VerifyHostKeyDNS to my
>> ssh_config on both my client and server but both sides still said the
>> host key couldn't be found in DNS. Not sure if it is something I
>> did/didn't do or not.
>
> Make sure that both use IPA DNS server and that the SSHFP records exist
> (they should be created automatically in ipa-client-install, or ipa
> host-mod with --updatedns).

Yeah, and AFAIK that was all there. It worked when I tested this the 
last time, I'll chalk this up as my mistake.

>>
>> I like showing just the fingerprint by default, it is much nicer than
>> the whole key.
>
> I think so :-)
>
>>
>> This fails:
>>
>> $ ipa user-mod --delattr ipasshpubkey=<bigkey_not_in_entry> tuser1
>>
>> [Tue Jan 24 16:41:52 2012] [error] ipa: ERROR: non-public:
>> UnicodeDecodeError: 'utf8' codec can't decode byte 0x91 in position 21:
>> invalid start byte
>> [Tue Jan 24 16:41:52 2012] [error] Traceback (most recent call last):
>> [Tue Jan 24 16:41:52 2012] [error] File
>> "/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 230, in
>> wsgi_execute
>> [Tue Jan 24 16:41:52 2012] [error] result = self.Command[name](*args,
>> **options)
>> [Tue Jan 24 16:41:52 2012] [error] File
>> "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 438, in
>> __call__
>> [Tue Jan 24 16:41:52 2012] [error] ret = self.run(*args, **options)
>> [Tue Jan 24 16:41:52 2012] [error] File
>> "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 696, in run
>> [Tue Jan 24 16:41:52 2012] [error] return self.execute(*args, **options)
>> [Tue Jan 24 16:41:52 2012] [error] File
>> "/usr/lib/python2.7/site-packages/ipalib/plugins/baseldap.py", line
>> 1106, in execute
>> [Tue Jan 24 16:41:52 2012] [error]
>> self.process_attr_options(entry_attrs, dn, keys, options)
>> [Tue Jan 24 16:41:52 2012] [error] File
>> "/usr/lib/python2.7/site-packages/ipalib/plugins/baseldap.py", line 784,
>> in process_attr_options
>> [Tue Jan 24 16:41:52 2012] [error] raise
>> errors.AttrValueNotFound(attr=attr, value=delval)
>> [Tue Jan 24 16:41:52 2012] [error] File
>> "/usr/lib/python2.7/site-packages/ipalib/errors.py", line 268, in
>> __init__
>> [Tue Jan 24 16:41:52 2012] [error] self.strerror = ugettext(self.format)
>> % kw
>> [Tue Jan 24 16:41:52 2012] [error] File
>> "/usr/lib/python2.7/site-packages/ipalib/text.py", line 248, in __mod__
>> [Tue Jan 24 16:41:52 2012] [error] return self.__unicode__() % kw
>> [Tue Jan 24 16:41:52 2012] [error] UnicodeDecodeError: 'utf8' codec
>> can't decode byte 0x91 in position 21: invalid start byte
>
> Good catch, will fix (as part of patch 66, so self-NACK on the current
> version).

Ok

rob