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

David Lehman dlehman at redhat.com
Mon Mar 24 23:14:58 UTC 2014


On Mon, 2014-03-24 at 19:08 -0400, Anne Mulhern wrote:
> 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.

None of the things mentioned as being broken are critical, so I'd be
fine with a single bug to track the followups and a comment in the test
code.

David

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