[Freeipa-devel] [PATCH 0053] Implement OTP token importing
Nathaniel McCallum
npmccallum at redhat.com
Fri Jun 13 19:39:20 UTC 2014
I am CC'ing Simo because he wants to review my PBKDF2 implementation.
On Wed, 2014-06-11 at 17:41 -0400, Nathaniel McCallum wrote:
> On Wed, 2014-06-11 at 14:24 +0200, Jan Cholasta wrote:
> > Hi,
> >
> > On 13.5.2014 18:40, Nathaniel McCallum wrote:
> > > On Tue, 2014-05-13 at 12:38 -0400, Nathaniel McCallum wrote:
> > >> This patch adds support for importing tokens using RFC 6030 key
> > >> container files. This includes decryption support. For sysadmin sanity,
> > >> any tokens which fail to add will be written to the output file for
> > >> examination. The main use case here is where a small subset of a large
> > >> set of tokens fails to validate or add. Using the output file, the
> > >> sysadmin can attempt to recover these specific tokens.
> > >>
> > >> This code is implemented as a server-side script. However, it doesn't
> > >> actually need to run on the server. This was done because importing is
> > >> an odd fit for the IPA command framework:
> > >> 1. We need to write an output file.
> > >> 2. The operation may be long-running (thousands of tokens).
> > >> 3. Only admins need to perform this task and it only happens
> > >> infrequently.
> > >
> > > I forgot to put the link to the ticket in the commit message. Fixed.
> >
> > 1) I think you should initialize NSS in ipa_otptoken_import.py, not in
> > the ipa-otptoken-import script.
>
> Fixed.
>
> > 2) The pep8 tool reports a lot of errors in ipa_otptoken_import.py.
>
> Fixed (mostly). The remaining output from pep8 is, I think, entirely
> justifiable.
>
> > 3) Other error messages are in the form "message: %s", I think this one
> > should use that form as well:
> >
> > + if encoding != 'DECIMAL':
> > + raise ValidationError('Unsupported encoding (%s)!' % encoding)
>
> Fixed.
>
> > 4) This is not right:
> >
> > + except:
> > + self.log.warn("Error adding token: " +
> > str(sys.exc_info()[1]))
> >
> > I think it should be something like this instead:
> >
> > except ValidationError, e:
> > self.log.warn("Error adding token: %s", e)
>
> Fixed.
>
> > 5) There is no man page for ipa-otptoken-import.
>
> TODO (tomorrow).
Fixed.
> > 6) Output file is created even when ipa-otptoken-import fails with
> > "Unable to connect to LDAP! Did you kinit?" and other initialization
> > errors, which makes subsequent ipa-otptoken-import fail with "Output
> > file already exists!".
>
> Fixed.
>
> > 7) When a key is specified by reference in Key/KeyReference instead of
> > directly in Key/Data/Secret like in
> > <http://git.savannah.gnu.org/cgit/oath-toolkit.git/tree/pskctool/tests/pskc-figure4.xml>,
> > import fails with "Key not found in token!". I would expect a different
> > error message.
>
> This error is now: Referenced keys are not supported!
>
> > 8) Importing
> > <http://git.savannah.gnu.org/cgit/oath-toolkit.git/tree/pskctool/tests/pskc-figure5.xml>
> > produces this output:
> >
> > /usr/lib/python2.7/site-packages/ipaserver/install/ipa_otptoken_import.py:307:
> > FutureWarning: The behavior of this method will change in future
> > versions. Use specific 'len(elem)' or 'elem is not None' test instead.
> > if data.get('pinpolicy', None):
> > Error adding token: 'NoneType' object has no attribute 'strip'
>
> This now states:
> Error adding token: PINPolicy policy not supported!
> Error adding token: Unsupported token type!
>
> > 9) Using an arbitrary file in -k produces this output:
> >
> > (SEC_ERROR_INVALID_KEY) The key does not support the requested operation.
> > Traceback (most recent call last):
> > File "/usr/sbin/ipa-otptoken-import", line 29, in <module>
> > nss.nss_shutdown()
> > nss.error.NSPRError: (SEC_ERROR_BUSY) NSS could not shutdown. Objects
> > are still in use.
>
> What do you mean by "arbitrary file"? A file that is not the key?
> Like /dev/null? I'm not able to reproduce this.
>
> > 10) Importing
> > <http://git.savannah.gnu.org/cgit/oath-toolkit.git/tree/pskctool/tests/pskc-figure7.xml>
> > and
> > <http://git.savannah.gnu.org/cgit/oath-toolkit.git/tree/pskctool/tests/pskc-figure8.xml>
> > produces this output:
> >
> > Error adding token: object of type 'NoneType' has no len()
>
> Import fails with:
> Derived keys are not currently supported!
> or
> X.509 keys are not currently supported!
>
> It would be nice to support these in the future.
I added support for derived keys. However, pskc-figure7.xml appears to
have a problem. This problem is actually in RFC 6030 itself (where
pskc-figure7.xml comes from). According to the xenc11 standard, all of
these tags should be in the xenc11 namespace (not pkcs5 or undefined as
given in RFC 6030):
<pkcs5:PBKDF2-params>
<Salt>
<Specified>Ej7/PEpyEpw=</Specified>
</Salt>
<IterationCount>1000</IterationCount>
<KeyLength>16</KeyLength>
<PRF/>
</pkcs5:PBKDF2-params>
When fixing this error in pskc-figure7.xml, key derivation works in this
latest patch.
> > 11) Importing
> > <http://git.savannah.gnu.org/cgit/oath-toolkit.git/tree/pskctool/tests/pskc-all.xml>
> > or
> > <http://git.savannah.gnu.org/cgit/oath-toolkit.git/tree/pskctool/tests/pskc-all-signed.xml>
> > produces this output:
> >
> > /usr/lib/python2.7/site-packages/ipaserver/install/ipa_otptoken_import.py:304:
> > FutureWarning: The behavior of this method will change in future
> > versions. Use specific 'len(elem)' or 'elem is not None' test instead.
> > if data.get('maxtransact', None):
> > /usr/lib/python2.7/site-packages/ipaserver/install/ipa_otptoken_import.py:307:
> > FutureWarning: The behavior of this method will change in future
> > versions. Use specific 'len(elem)' or 'elem is not None' test instead.
> > if data.get('pinpolicy', None):
>
> Both of these now output:
> Error adding token: NumberOfTransactions policy not supported!
>
> > 12) Importing
> > <http://git.savannah.gnu.org/cgit/oath-toolkit.git/tree/pskctool/tests/pskc-invalid.xml>
> > succeeds, but it should fail.
>
> This now errors with:
> PSKC file is invalid!
>
> > 13) Importing
> > <http://git.savannah.gnu.org/cgit/oath-toolkit.git/tree/libpskc/examples/pskc-mini.xml>
> > fails, but it should succeed, I think.
>
> I think this should fail in our case since we can't possibly support
> that configuration.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-npmccallum-0053.2-Implement-OTP-token-importing.patch
Type: text/x-patch
Size: 26405 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140613/7ca6b141/attachment.bin>
More information about the Freeipa-devel
mailing list