[blivet:rhel7/master 1/4] Make crypt tests runnable.

Anne Mulhern amulhern at redhat.com
Mon Mar 24 23:08:14 UTC 2014


What I would like to do for this and the subsequent patch in
this patch set is:

1) Commit the working tests.
2) Put in a FIXME comment in the test file indicating all the things that
we believe need to be fixed, as thoroughly as we can.
3) Fix all the listed things in due course.

I think that it is important to get to the point where our tests are
actually running and are expected to pass, and to not allow the fact that
some of the code does not work as we believe that it should to result
in an indefinite delay.

- mulhern

----- Original Message -----
> From: "David Lehman" <dlehman at redhat.com>
> To: "anaconda patch review" <anaconda-patches at lists.fedorahosted.org>
> Sent: Monday, March 24, 2014 2:34:35 PM
> Subject: Re: [blivet:rhel7/master 1/4] Make crypt tests runnable.
> 
> On Fri, 2014-03-21 at 15:29 -0400, mulhern wrote:
> > If any of these tests seem wrong then the code can be fixed up and the test
> > changed.
> > 
> > A tearDown method was added to close the device in case a test failed.
> > 
> > Some instance fields were used for the names of the devices.
> > 
> > Some aspects of the code that brought about these changes are:
> > * All crypto.luks_* methods raise a ValueError if they are not given a
> > passphrase although they accept a key_file argument.
> 
> This should only be the case when flags.installer_mode is True.
> 
> > * If a device does not exist, CryptSetup raises an IOError.
> > 
> > Some methods that did suprising things were.
> > * is_luks --- it raises an IOError for an unknown device
> 
> I'm not sure what's the best way to handle this. Return False or raise
> an exception? If exception, which one? If we were checking it ourselves
> we could raise ValueError instead of IOError.
> 
> > * luks_format --- raises a ValueError if it gets a keyfile instead of a
> > passphrase
> 
> That's a blivet bug -- it is assuming installer mode is enabled.
> 
> > * luks_remove_key --- raises a CryptoError instead of a RuntimeError
> > * luks_status --- returns ints, not booleans
> 
> It would be nice to fix this in cryptsetup-python, but we could fix it
> in blivet if necessary.
> 
> > 
> > Signed-off-by: mulhern <amulhern at redhat.com>
> > ---
> >  tests/devicelibs_test/crypto_test.py | 140
> >  +++++++++++++++++++++++++----------
> >  1 file changed, 100 insertions(+), 40 deletions(-)
> > 
> > diff --git a/tests/devicelibs_test/crypto_test.py
> > b/tests/devicelibs_test/crypto_test.py
> > index 7ef49fe..5b33a07 100755
> > --- a/tests/devicelibs_test/crypto_test.py
> > +++ b/tests/devicelibs_test/crypto_test.py
> > @@ -8,7 +8,7 @@ import os
> >  class CryptoTestCase(baseclass.DevicelibsTestCase):
> >  
> >      @unittest.skipUnless(os.geteuid() == 0, "requires root privileges")
> > -    def testCrypto(self):
> > +    def testCryptoMisc(self):
> >          _LOOP_DEV0 = self._loopMap[self._LOOP_DEVICES[0]]
> >          _LOOP_DEV1 = self._loopMap[self._LOOP_DEVICES[1]]
> >  
> > @@ -19,13 +19,17 @@ class CryptoTestCase(baseclass.DevicelibsTestCase):
> >          ##
> >          # pass
> >          self.assertEqual(crypto.is_luks(_LOOP_DEV0), -22)
> > -        self.assertEqual(crypto.is_luks("/not/existing/device"), -22)
> > +        self.assertRaisesRegexp(IOError,
> > +            "Device cannot be opened",
> > +            crypto.is_luks,
> > +            "/not/existing/device")
> >  
> >          ##
> >          ## luks_format
> >          ##
> >          # pass
> >          self.assertEqual(crypto.luks_format(_LOOP_DEV0,
> >          passphrase="secret", cipher="aes-cbc-essiv:sha256",
> >          key_size=256), None)
> > +        self.assertEqual(crypto.is_luks(_LOOP_DEV0), 0)
> >  
> >          # make a key file
> >          handle, keyfile = tempfile.mkstemp(prefix="key", text=False)
> > @@ -33,19 +37,29 @@ class CryptoTestCase(baseclass.DevicelibsTestCase):
> >          os.close(handle)
> >  
> >          # format with key file
> > -        self.assertEqual(crypto.luks_format(_LOOP_DEV1, key_file=keyfile),
> > None)
> > +        self.assertRaisesRegexp(ValueError,
> > +           "requires passphrase",
> > +           crypto.luks_format,
> > +           _LOOP_DEV1, key_file=keyfile)
> >  
> >          # fail
> > -        self.assertRaises(crypto.CryptoError, crypto.luks_format,
> > "/not/existing/device", passphrase="secret",
> > cipher="aes-cbc-essiv:sha256", key_size=256)
> > +        self.assertRaisesRegexp(IOError,
> > +           "Device cannot be opened",
> > +           crypto.luks_format,
> > +           "/not/existing/device", passphrase="secret",
> > cipher="aes-cbc-essiv:sha256", key_size=256)
> > +
> >          # no passhprase or key file
> > -        self.assertRaises(ValueError, crypto.luks_format, _LOOP_DEV1,
> > cipher="aes-cbc-essiv:sha256", key_size=256)
> > +        self.assertRaisesRegexp(ValueError,
> > +           "requires passphrase",
> > +           crypto.luks_format,
> > +           _LOOP_DEV1, cipher="aes-cbc-essiv:sha256", key_size=256)
> >  
> >          ##
> >          ## is_luks
> >          ##
> >          # pass
> >          self.assertEqual(crypto.is_luks(_LOOP_DEV0), 0)    # 0 = is luks
> > -        self.assertEqual(crypto.is_luks(_LOOP_DEV1), 0)
> > +        self.assertEqual(crypto.is_luks(_LOOP_DEV1), -22)
> >  
> >          ##
> >          ## luks_add_key
> > @@ -53,49 +67,96 @@ class CryptoTestCase(baseclass.DevicelibsTestCase):
> >          # pass
> >          self.assertEqual(crypto.luks_add_key(_LOOP_DEV0,
> >          new_passphrase="another-secret", passphrase="secret"), None)
> >  
> > -        # make another key file
> > -        handle, new_keyfile = tempfile.mkstemp(prefix="key", text=False)
> > -        os.write(handle, "area51")
> > -        os.close(handle)
> > -
> > -        # add new key file
> > -        self.assertEqual(crypto.luks_add_key(_LOOP_DEV1,
> > new_key_file=new_keyfile, key_file=keyfile), None)
> > -
> >          # fail
> > -        self.assertRaises(crypto.CryptoError, crypto.luks_add_key,
> > _LOOP_DEV0, new_passphrase="another-secret",
> > passphrase="wrong-passphrase")
> > +        self.assertRaisesRegexp(crypto.CryptoError,
> > +           "luks add key failed",
> > +           crypto.luks_add_key,
> > +           _LOOP_DEV0, new_passphrase="another-secret",
> > passphrase="wrong-passphrase")
> >  
> >          ##
> >          ## luks_remove_key
> >          ##
> >          # fail
> > -        self.assertRaises(RuntimeError, crypto.luks_remove_key,
> > _LOOP_DEV0, del_passphrase="another-secret", passphrase="wrong-pasphrase")
> > +        self.assertRaisesRegexp(crypto.CryptoError,
> > +           "luks remove key failed",
> > +           crypto.luks_remove_key,
> > +           _LOOP_DEV0, del_passphrase="another-secret",
> > passphrase="wrong-pasphrase")
> >  
> >          # pass
> >          self.assertEqual(crypto.luks_remove_key(_LOOP_DEV0,
> >          del_passphrase="another-secret", passphrase="secret"), None)
> >  
> > -        # remove key file
> > -        self.assertEqual(crypto.luks_remove_key(LOOP_DEV1,
> > del_key_file=new_keyfile, key_file=keyfile), None)
> > +        # fail
> > +        self.assertRaisesRegexp(IOError,
> > +           "Device cannot be opened",
> > +           crypto.luks_open,
> > +           "/not/existing/device", "another-crypted", passphrase="secret")
> > +
> > +        # no passhprase or key file
> > +        self.assertRaisesRegexp(ValueError,
> > +           "luks_format requires passphrase",
> > +           crypto.luks_open,
> > +           _LOOP_DEV1, "another-crypted")
> > +
> > +        self.assertRaisesRegexp(IOError,
> > +           "Device cannot be opened",
> > +           crypto.luks_status,
> > +           "another-crypted")
> > +        self.assertRaisesRegexp(IOError,
> > +           "Device cannot be opened",
> > +           crypto.luks_close,
> > +           "wrong-name")
> > +
> > +        # cleanup
> > +        os.unlink(keyfile)
> > +
> > +class CryptoTestCase2(baseclass.DevicelibsTestCase):
> > +
> > +    def __init__(self, *args, **kwargs):
> > +        """Set up the names by which luks knows these devices."""
> > +        super(CryptoTestCase2, self).__init__(*args, **kwargs)
> > +        self._names = { self._LOOP_DEVICES[0]: "crypted",
> > +           self._LOOP_DEVICES[1]: "encrypted" }
> > +
> > +    def tearDown(self):
> > +        """Close all devices just in case they are still open."""
> > +        import blivet.devicelibs.crypto as crypto
> > +        for name in self._names.values():
> > +            try:
> > +                crypto.luks_close(name)
> > +            except (IOError, crypto.CryptoError):
> > +                pass
> > +        super(CryptoTestCase2, self).tearDown()
> > +
> > +    @unittest.skipUnless(os.geteuid() == 0, "requires root privileges")
> > +    def testCryptoOpen(self):
> > +        _LOOP_DEV0 = self._loopMap[self._LOOP_DEVICES[0]]
> > +        _LOOP_DEV1 = self._loopMap[self._LOOP_DEVICES[1]]
> > +
> > +        _name0 = self._names[self._LOOP_DEVICES[0]]
> > +        _name1 = self._names[self._LOOP_DEVICES[1]]
> > +
> > +        import blivet.devicelibs.crypto as crypto
> >  
> >          ##
> > -        ## luks_open
> > +        ## luks_format
> >          ##
> >          # pass
> > -        self.assertEqual(crypto.luks_open(_LOOP_DEV0, "crypted",
> > passphrase="secret"), None)
> > -        self.assertEqual(crypto.luks_open(_LOOP_DEV1, "encrypted",
> > key_file=keyfile), None)
> > +        self.assertEqual(crypto.luks_format(_LOOP_DEV0,
> > passphrase="secret", cipher="aes-cbc-essiv:sha256", key_size=256), None)
> > +        self.assertEqual(crypto.luks_format(_LOOP_DEV1,
> > passphrase="hidden", cipher="aes-cbc-essiv:sha256", key_size=256), None)
> >  
> > -        # fail
> > -        self.assertRaises(crypto.CryptoError, crypto.luks_open,
> > "/not/existing/device", "another-crypted", passphrase="secret")
> > -        self.assertRaises(crypto.CryptoError, crypto.luks_open,
> > "/not/existing/device", "another-crypted", key_file=keyfile)
> > -        # no passhprase or key file
> > -        self.assertRaises(ValueError, crypto.luks_open, _LOOP_DEV1,
> > "another-crypted")
> > +        ##
> > +        ## luks_open
> > +        ##
> > +        # pass
> > +        self.assertEqual(crypto.luks_open(_LOOP_DEV0, _name0,
> > passphrase="secret"), None)
> > +        self.assertEqual(crypto.luks_open(_LOOP_DEV1, _name1,
> > passphrase="hidden"), None)
> >  
> >          ##
> >          ## luks_status
> >          ##
> >          # pass
> > -        self.assertEqual(crypto.luks_status("crypted"), True)
> > -        self.assertEqual(crypto.luks_status("encrypted"), True)
> > -        self.assertEqual(crypto.luks_status("another-crypted"), False)
> > +        self.assertEqual(crypto.luks_status(_name0), 2)
> > +        self.assertEqual(crypto.luks_status(_name1), 2)
> >  
> >          ##
> >          ## luks_uuid
> > @@ -110,19 +171,18 @@ class CryptoTestCase(baseclass.DevicelibsTestCase):
> >          ## luks_close
> >          ##
> >          # pass
> > -        self.assertEqual(crypto.luks_close("crypted"), None)
> > -        self.assertEqual(crypto.luks_close("encrypted"), None)
> > +        self.assertEqual(crypto.luks_close(_name0), None)
> > +        self.assertEqual(crypto.luks_close(_name1), None)
> >  
> > -        # fail
> > -        self.assertRaises(crypto.CryptoError, crypto.luks_close,
> > "wrong-name")
> >          # already closed
> > -        self.assertRaises(crypto.CryptoError, crypto.luks_close,
> > "crypted")
> > -        self.assertRaises(crypto.CryptoError, crypto.luks_close,
> > "encrypted")
> > -
> > -        # cleanup
> > -        os.unlink(keyfile)
> > -        os.unlink(new_keyfile)
> > -
> > +        self.assertRaisesRegexp(IOError,
> > +           "Device cannot be opened",
> > +           crypto.luks_close,
> > +           "crypted")
> > +        self.assertRaisesRegexp(IOError,
> > +           "Device cannot be opened",
> > +           crypto.luks_close,
> > +           "encrypted")
> >  
> >  def suite():
> >      return unittest.TestLoader().loadTestsFromTestCase(CryptoTestCase)
> 
> 
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> 


More information about the anaconda-patches mailing list