[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