[Freeipa-devel] [PATCH 0017] Add OTP support to ipalib CLI

Petr Viktorin pviktori at redhat.com
Thu Sep 5 10:19:17 UTC 2013


On 09/05/2013 06:38 AM, Nathaniel McCallum wrote:
> On Thu, 2013-09-05 at 00:25 -0400, Nathaniel McCallum wrote:
>> This patch has a few problems that I'd like some help with. There are a
>> few notes here as well.
>>
>> 1. The handling of the 'key' option is insecure. It should probably be
>> treated like a password (hidden from logs, etc). However, in this case,
>> it is binary, so I'm not quite sure how to do that. Passing it as a
>> command line option may be nice for scripting, but is potentially a
>> security problem if it ends up in bash.history. It would also be nice if
>> the encoding were base32 instead of base64, since nearly all the OTP
>> tools use this encoding.

Not only in bash_history; anyone can see command line parameters of 
running programs.
We'll need to modify the framework to support more another password 
parameter type.
The base32 on input/output can be added to that new type.

>> 2. The 'key' option also appears in otp-find. I'd like to suppress this.
>> How?

Use the 'no_search' flag (which is currently undocumented; see below).

>> 3. I had to make the 'id' option optional to make the uuid
>> autogeneration work in otp-add. However, this has the side-effect that
>> 'id' is now optional in all the other commands. This is particularly bad
>> in the case of otp-del, where calling this command with no ID
>> transparently removes all tokens. How can I make this optional for
>> otp-add but required for all other commands?

You'll need to add a new option flag.

1. Add a 'optional_create' flag to the comment in ipalib.parameters.Param.
2. Handle the flag in ipalib.crud.Create.get_options (clone with 
attribute=attribute, required=False)

See the handling of 'ask_create' for exapmles.

Please also document 'no_search' while you're modifying this part of the 
code.

>> 4. otp-import is not implemented. I spent a few hours looking and I
>> didn't find any otp tool that actually uses this xml format for
>> exporting. Should we implement this now or wait until someone can
>> actually export data to us?
>>
>> 5. otp-del happily deletes the last token for a user. How can I find out
>> the dn of the user executing the command? Also, what is the right
>> exception to throw in pre_callback()?

Don't you want to check the token's owner rather than the current user?
You could use LastMemberError here, or make your own error.

>> 6. user-show does not list the associated tokens for this user. Do we
>> care? It is a single search: otp-find --owner npmccallum.
>>
>> 7. otp-add only prints the qr code if the --qrcode option is specified.
>> This is for two reasons. First, and most importantly, the qr code
>> doesn't fit on a standard 24x80 terminal. I wanted to avoid dumping
>> garbage on people's screens by default. Second, you may not always want
>> the qr code output (like for a hard token or manual code entry).

Would it make sense to add --qrcode to otp-show as well? If otp-add is 
the only opportunity to get the QR code, it's is easy to miss.

> 8. If a user is deleted, the user's assigned tokens are left unmodified.
> That is *not* to say they are orphaned. The owner attribute retains a dn
> to an invalid user. This also means that otp-find --owner=deletedUser
> will fail since we can't look up the deleted user. How does dirsrv
> handle this for other relationships?

In the hosts plugin, services are deleted in host_del.pre_callback. You 
could follow that example.


-- 
Petr³




More information about the Freeipa-devel mailing list