On Wed, 2014-06-25 at 09:53 -0400, Nathaniel McCallum wrote:
> On Wed, 2014-06-25 at 13:35 +0300, Alexander Bokovoy wrote:
> > On Mon, 23 Jun 2014, Nathaniel McCallum wrote:
> > >On Mon, 2014-06-23 at 10:29 +0300, Alexander Bokovoy wrote:
> > >> On Fri, 20 Jun 2014, Nathaniel McCallum wrote:
> > >> >On Thu, 2014-06-19 at 16:30 -0400, Nathaniel McCallum wrote:
> > >> >> This command behaves almost exactly like otptoken-add except:
> > >> >> 1. The new token data is written directly to a YubiKey
> > >> >> 2. The vendor/model/serial fields are populated from the YubiKey
> > >> >>
> > >> >> === NOTE ===
> > >> >> 1. This patch depends on the new Fedora package: python-yubico. If you
> > >> >> would like to help with the package review, please assign yourself 
> > >> >> here:
> > >> >> https://bugzilla.redhat.com/show_bug.cgi?id=1111334
> > >> >
> > >> >New version of the patch. This one works (yay!).
> > >> >
> > >> >1. Because of the dependency on python-yubico, is this feature something
> > >> >we want in core FreeIPA? As a subpackage? Separate project altogether?
> > >> >The only dependency for python-yubico is pyusb.
> > >> I'd prefer to have it integrated but have a separate dummy subpackage
> > >> that pulls in all required dependencies, like, freeipa-tools-yubico. 
> > >> Instead
> > >> of failing when 'ipa otptoken-add-yubikey' is called, please wrap the
> > >> python-yubico import into a code that allows reporting a message back to
> > >> the user advising to install the package.
> > >>
> > >> Who is a supposed user for this command? IPA command line interface isn't
> > >> usually available on enrolled machines even though underlying Python
> > >> modules are all there. Are we talking about admins or just users?
> > >
> > >As discussed on IRC, we are currently hard-coding lots of optional
> > >dependencies. And breaking this apart into subpackages can be solved at
> > >a later point. YubiKey is also a unique case: we don't expect to be
> > >adding many more plugins like this.
> > >
> > >For these reasons, I have kept this as a hard dependency. To ease this
> > >transition, I have added python-yubico to F20 and EL6. You can help with
> > >the update review here:
> > >https://admin.fedoraproject.org/updates/python-yubico-1.2.1-3.fc20
> > >https://admin.fedoraproject.org/updates/python-yubico-1.2.1-3.el6
> > >
> > >> >3. This code currently emits a warning from the call to otptoken-add:
> > >> >WARNING: API Version number was not sent, forward compatibility not
> > >> >guaranteed. Assuming server's API version, 2.89
> > >> >
> > >> >How do I fix this?
> > >> Do not filter 'version' field in options in execute().
> > >
> > >I opted to not filter out version rather than hard-code it (pviktori's
> > >suggestion). This is based on the fact that otptoken-add-yubikey is
> > >tightly integrated with otptoken-add (even using some of the class
> > >attributes from otptoken).
> > >
> > >> >4. I am not sure why I have to delete the summary and value keys from
> > >> >the return dictionary. It would be nice to display this summary message
> > >> >just like otptoken-add.
> > >
> > >I still need help on this.
> > >
> > >> >5. Am I doing the ipatoken(vendor|model|serial) options correctly? These
> > >> >aren't user settable, but we need to pass them from the yubikey
> > >> >(client-side) to the server.
> > >
> > >This is no longer needed since I am doing everything in forward().
> > >However, listing these three as output params causes them to appear
> > >before the token's ID. I don't think this is the right way to output
> > >these. But this seems to me a framework issue.
> > >
> > >> >6. I'm not sure my use of assert or ValueError are correct. What should
> > >> >I do here?
> > >
> > >Still need help here.
> > Fixed this part.
> > 
> > >
> > >> >7. Considering that this is just a specialized invocation of
> > >> >otptoken-add, can't we do this all on the client-side? This is why I had
> > >> >originally used frontend.Local rather than frontend.Command.
> > >> You don't need to implement execute then, only forward, where you'll
> > >> forward your call to the server under otptoken_add name.
> > >>
> > >> Typically in #forward we call super's forward but that is because we
> > >> in Command.forward() we  simply forward the command to the remote 
> > >> backend,
> > >>  using the self.name. In your case we shouldn't really have a separate
> > >> command on the server under the same name, so you'll need to avoid
> > >> calling
> > >>
> > >> So, it should look like this:
> > >>
> > >>     def forward(self, *args, **kw):
> > >>         perform yubikey initialization
> > >>         filter out kw and args, if needed
> > >>         return self.Backend.rpcclient.forward('otptoken_add', *args, 
> > >> **kw)
> > >>
> > >> See service_show implementation for an example.
> > >
> > >Fixed.
> > I'm attaching few fixups to the patch that make it proper reporting for
> > non-Yubikey case and also properly update VERSION.
> > 
> > Provisional ACK.
> 
> Merged.

This patch includes everything above and fixes the missing ./makeapi
run.

Nathaniel
From 3f877142e99c12050def9f56c650eb474320bfff Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum <npmccal...@redhat.com>
Date: Thu, 19 Jun 2014 12:28:32 -0400
Subject: [PATCH] Add the otptoken-add-yubikey command

This command behaves almost exactly like otptoken-add except:
1. The new token data is written directly to a YubiKey
2. The vendor/model/serial fields are populated from the YubiKey
---
 API.txt                            |  12 ++++
 VERSION                            |   4 +-
 freeipa.spec.in                    |   1 +
 ipalib/plugins/otptoken.py         |   2 +-
 ipalib/plugins/otptoken_yubikey.py | 139 +++++++++++++++++++++++++++++++++++++
 5 files changed, 155 insertions(+), 3 deletions(-)
 create mode 100644 ipalib/plugins/otptoken_yubikey.py

diff --git a/API.txt b/API.txt
index fdb780d53fe7e167e0a183926d5c91115d2d131b..3c3b6447fec3c313c3038390ac7317533c530d8b 100644
--- a/API.txt
+++ b/API.txt
@@ -2326,6 +2326,18 @@ option: Str('version?', exclude='webui')
 output: Output('completed', <type 'int'>, None)
 output: Output('failed', <type 'dict'>, None)
 output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
+command: otptoken_add_yubikey
+args: 1,8,1
+arg: Str('ipatokenuniqueid?', cli_name='id', primary_key=True)
+option: Str('description?', cli_name='desc')
+option: Bool('ipatokendisabled?', cli_name='disabled')
+option: Str('ipatokennotafter?', cli_name='not_after')
+option: Str('ipatokennotbefore?', cli_name='not_before')
+option: IntEnum('ipatokenotpdigits?', autofill=True, cli_name='digits', default=6, values=(6, 8))
+option: Str('ipatokenowner?', cli_name='owner')
+option: IntEnum('slot?', cli_name='slot', values=(1, 2))
+option: Str('version?', exclude='webui')
+output: Output('result', None, None)
 command: otptoken_del
 args: 1,2,3
 arg: Str('ipatokenuniqueid', attribute=True, cli_name='id', multivalue=True, primary_key=True, query=True, required=True)
diff --git a/VERSION b/VERSION
index 65e52735079baf0e04f05d295e2b6dd8ad8515f4..f0c2db55658f3ab4cfafffc5735aa77b52bc9cc8 100644
--- a/VERSION
+++ b/VERSION
@@ -89,5 +89,5 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=93
-# Last change: mbasti - New record type added: DLV
+IPA_API_VERSION_MINOR=94
+# Last change: npmaccallum - otptoken-add-yubikey
diff --git a/freeipa.spec.in b/freeipa.spec.in
index ae730c369ae3fac868739de62a144cc611b58481..e56c33d703cc7bf3bb89a244f4edd21aaa7b883a 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -306,6 +306,7 @@ Requires: libipa_hbac-python
 Requires: python-qrcode
 Requires: python-pyasn1
 Requires: python-dateutil
+Requires: python-yubico
 
 Obsoletes: ipa-python >= 1.0
 
diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py
index d834d582a16d95ab08c3f1fe1aef29160c77ae23..7962af0035fb9ac00e68c4b642bb62aa82d498c2 100644
--- a/ipalib/plugins/otptoken.py
+++ b/ipalib/plugins/otptoken.py
@@ -196,7 +196,7 @@ class otptoken(LDAPObject):
         ),
         IntEnum('ipatokenotpdigits?',
             cli_name='digits',
-            label=_('Display length'),
+            label=_('Digits'),
             values=(6, 8),
             default=6,
             autofill=True,
diff --git a/ipalib/plugins/otptoken_yubikey.py b/ipalib/plugins/otptoken_yubikey.py
new file mode 100644
index 0000000000000000000000000000000000000000..b4f21881aed8d545dee256c4ff2d4e6f9048d8bc
--- /dev/null
+++ b/ipalib/plugins/otptoken_yubikey.py
@@ -0,0 +1,139 @@
+# Authors:
+#   Nathaniel McCallum <npmccal...@redhat.com>
+#
+# Copyright (C) 2014  Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from ipalib import _, Str, IntEnum
+from ipalib.errors import  NotFound
+from ipalib.plugable import Registry
+from ipalib.frontend import Command
+from ipalib.plugins.otptoken import otptoken
+
+import os
+
+import yubico
+
+__doc__ = _("""
+YubiKey Tokens
+""") + _("""
+Manage YubiKey tokens.
+""") + _("""
+This code is an extension to the otptoken plugin and provides support for
+reading/writing YubiKey tokens directly.
+""") + _("""
+EXAMPLES:
+""") + _("""
+ Add a new token:
+   ipa otptoken-add-yubikey --owner=jdoe --desc="My YubiKey"
+""")
+
+register = Registry()
+
+@register()
+class otptoken_add_yubikey(Command):
+    __doc__ = _('Add a new YubiKey OTP token.')
+
+    takes_args = (
+        Str('ipatokenuniqueid?',
+            cli_name='id',
+            label=_('Unique ID'),
+            primary_key=True,
+        ),
+    )
+
+    takes_options = Command.takes_options + (
+        IntEnum('slot?',
+            cli_name='slot',
+            label=_('YubiKey slot'),
+            values=(1, 2),
+        ),
+    ) + tuple(x for x in otptoken.takes_params if x.name in (
+        'description',
+        'ipatokenowner',
+        'ipatokendisabled',
+        'ipatokennotbefore',
+        'ipatokennotafter',
+        'ipatokenotpdigits'
+    ))
+
+    has_output_params = Command.has_output_params + \
+        tuple(x for x in otptoken.takes_params if x.name in (
+            'ipatokenvendor',
+            'ipatokenmodel',
+            'ipatokenserial',
+        ))
+
+    def forward(self, *args, **kwargs):
+        # Open the YubiKey
+        try:
+            yk = yubico.find_yubikey()
+        except yubico.yubikey.YubiKeyError, e:
+            raise NotFound(reason=_('No YubiKey found'))
+
+        assert yk.version_num() >= (2, 1)
+
+        # If no slot is specified, find the first free slot.
+        if kwargs.get('slot', None) is None:
+            try:
+                used = yk.status().valid_configs()
+                kwargs['slot'] = sorted({1, 2}.difference(used))[0]
+            except IndexError:
+                raise NotFound(reason=_('No free YubiKey slot!'))
+
+        # Create the key (NOTE: the length is fixed).
+        key = os.urandom(20)
+
+        # Write the config.
+        cfg = yk.init_config()
+        cfg.mode_oath_hotp(key, kwargs['ipatokenotpdigits'])
+        cfg.extended_flag('SERIAL_API_VISIBLE', True)
+        yk.write_config(cfg, slot=kwargs['slot'])
+
+        # Filter the options we want to pass.
+        options = {k: v for k, v in kwargs.items() if k in (
+            'version',
+            'description',
+            'ipatokenowner',
+            'ipatokendisabled',
+            'ipatokennotbefore',
+            'ipatokennotafter',
+            'ipatokenotpdigits',
+        )}
+
+        # Run the command.
+        answer = self.Backend.rpcclient.forward('otptoken_add',
+                                                *args,
+                                                type=u'hotp',
+                                                ipatokenvendor=u'YubiCo',
+                                                ipatokenmodel=unicode(yk.model),
+                                                ipatokenserial=unicode(yk.serial()),
+                                                ipatokenotpalgorithm=u'sha1',
+                                                ipatokenhotpcounter=0,
+                                                ipatokenotpkey=key,
+                                                **options)
+
+        # Suppress values we don't want to return.
+        for k in (u'uri', u'ipatokenotpkey'):
+            if k in answer.get('result', {}):
+                del answer['result'][k]
+
+        # Return which slot was used for writing.
+        answer.get('result', {})['slot'] = kwargs['slot']
+
+        del answer['value']    # Why does this cause an error if omitted?
+        del answer['summary']  # Why does this cause an error if omitted?
+        return answer
-- 
2.0.0

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

Reply via email to