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

[Nathaniel McCallum]

On Thu, 2013-09-05 at 12:19 +0200, Petr Viktorin wrote:
> 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.

To clarify, by scripting I meant calling this from a python script. In
this case, the argument wouldn't show up in the argv. Sorry my wording
wasn't clear here.

The primary case where this will apply is in otp-import (if we implement
it). We will parse the XML and call self.api.Command.otp_add() for each
token found, including the key.

So it would be good to have this option available in python but not the
shell.

> >> 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.

If the executing user has permissions to remove another user's token, we
assume they are an admin and know what they are doing. So this case only
arises if the executing user is removing his/her own tokens. For
instance:

if executor == owner and tokenCount(owner) == 1:
  raise LastMemberError()

> >> 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.

Only admins have permission to read 'key' from LDAP after creation.
Standard users can create the OTP token object with a 'key', but cannot
read back the key. Hence, the URI is only available at creation
(provisioning) time.