[gparted] Change to insert or replace PasswordRAMStore::store() interface (#795617)



commit 957216f06c80c83b07a49123ac7e4df147fa477a
Author: Mike Fleetwood <mike fleetwood googlemail com>
Date:   Mon Apr 23 17:15:23 2018 +0100

    Change to insert or replace PasswordRAMStore::store() interface (#795617)
    
    Replace the insert() method (which reports an error when inserting a
    password with a key which already exists) with the store() method which
    replaces or inserts the password depending on whether the key already
    exists or not respectively.  There is also an optimisation that nothing
    is changed if the password to be replaced is the same as the one already
    stored.  The code in Win_GParted::open_encrypted_partition() is
    simplified now it doesn't have to implement this pattern of behaviour
    itself.
    
    Bug 795617 - Implement opening and closing of LUKS mappings

 include/PasswordRAMStore.h     |    2 +-
 src/PasswordRAMStore.cc        |   12 ++++++++-
 src/Win_GParted.cc             |   21 ++---------------
 tests/test_PasswordRAMStore.cc |   46 ++++++++++++++++++++++++++++++---------
 4 files changed, 49 insertions(+), 32 deletions(-)
---
diff --git a/include/PasswordRAMStore.h b/include/PasswordRAMStore.h
index 5c77175..c6b6ff4 100644
--- a/include/PasswordRAMStore.h
+++ b/include/PasswordRAMStore.h
@@ -40,7 +40,7 @@ friend class PasswordRAMStoreTest;  // To allow unit testing PasswordRAMStoreTes
                                     // access to private methods.
 
 public:
-       static bool insert( const Glib::ustring & key, const char * password );
+       static bool store( const Glib::ustring & key, const char * password );
        static bool erase( const Glib::ustring & key );
        static const char * lookup( const Glib::ustring & key );
 
diff --git a/src/PasswordRAMStore.cc b/src/PasswordRAMStore.cc
index e84cf18..917bc2a 100644
--- a/src/PasswordRAMStore.cc
+++ b/src/PasswordRAMStore.cc
@@ -203,9 +203,17 @@ static PWStore single_pwstore;
 
 // PasswordRAMStore public methods
 
-bool PasswordRAMStore::insert( const Glib::ustring & key, const char * password )
+bool PasswordRAMStore::store( const Glib::ustring & key, const char * password )
 {
-       return single_pwstore.insert( key, password );
+       const char * looked_up_pw = single_pwstore.lookup( key );
+       if ( looked_up_pw == NULL )
+               return single_pwstore.insert( key, password );
+
+       if ( strcmp( looked_up_pw, password ) == 0 )
+               return true;
+
+       return    single_pwstore.erase( key )
+              && single_pwstore.insert( key, password );
 }
 
 bool PasswordRAMStore::erase( const Glib::ustring & key )
diff --git a/src/Win_GParted.cc b/src/Win_GParted.cc
index 69d93f9..0abf31e 100644
--- a/src/Win_GParted.cc
+++ b/src/Win_GParted.cc
@@ -2465,28 +2465,13 @@ bool Win_GParted::open_encrypted_partition( const Partition & partition,
        bool success = ! Utils::execute_command( cmd, pw, output, error );
        hide_pulsebar();
        if ( success && pw != NULL )
-       {
-               // Replace the password just successfully used to open the LUKS mapping if
-               // it is different to the one already saved, or there is no saved
-               // password.
-               const char * stored_pw = PasswordRAMStore::lookup( partition.uuid );
-               if ( stored_pw != NULL            &&
-                    strcmp( pw, stored_pw ) == 0    )
-               {
-                       PasswordRAMStore::erase( partition.uuid );
-                       stored_pw = NULL;
-               }
-               if ( stored_pw == NULL )
-               {
-                       PasswordRAMStore::insert( partition.uuid, pw );
-               }
-       }
+               // Save the password just entered and successfully used to open the LUKS
+               // mapping.
+               PasswordRAMStore::store( partition.uuid, pw );
        else if ( ! success )
-       {
                // Erase the password used during for the failure to open the LUKS
                // mapping.
                PasswordRAMStore::erase( partition.uuid );
-       }
 
        message = ( success ) ? "" : _("Failed to open LUKS encryption");
        return success;
diff --git a/tests/test_PasswordRAMStore.cc b/tests/test_PasswordRAMStore.cc
index 7b8c41b..7c96b60 100644
--- a/tests/test_PasswordRAMStore.cc
+++ b/tests/test_PasswordRAMStore.cc
@@ -127,7 +127,7 @@ TEST_F( PasswordRAMStoreTest, SinglePassword )
 {
        // Test a single password can be stored, looked up and erased (and zeroed).
        pw = "password";
-       EXPECT_TRUE( PasswordRAMStore::insert( "key-single", pw.c_str() ) );
+       EXPECT_TRUE( PasswordRAMStore::store( "key-single", pw.c_str() ) );
 
        looked_up_pw = PasswordRAMStore::lookup( "key-single" );
        EXPECT_STREQ( pw.c_str(), looked_up_pw );
@@ -139,17 +139,41 @@ TEST_F( PasswordRAMStoreTest, SinglePassword )
        EXPECT_TRUE( mem_is_zero( protected_mem, ProtectedMemSize ) );
 }
 
-TEST_F( PasswordRAMStoreTest, DuplicatePassword )
+TEST_F( PasswordRAMStoreTest, IdenticalPassword )
 {
-       // Test storing a password with a duplicate key fails (and single password can be
-       // erased and zeroed).
+       // Test a password can be saved twice with the same key and looked up (and the
+       // single password can be erased and zeroed).
        pw = "password";
-       EXPECT_TRUE( PasswordRAMStore::insert( "key-single", pw.c_str() ) );
+       EXPECT_TRUE( PasswordRAMStore::store( "key-single", pw.c_str() ) );
+
+       EXPECT_TRUE( PasswordRAMStore::store( "key-single", pw.c_str() ) );
 
        looked_up_pw = PasswordRAMStore::lookup( "key-single" );
        EXPECT_STREQ( pw.c_str(), looked_up_pw );
 
-       EXPECT_FALSE( PasswordRAMStore::insert( "key-single", pw.c_str() ) );
+       looked_up_len = strlen( looked_up_pw );
+       EXPECT_TRUE( PasswordRAMStore::erase( "key-single" ) );
+       EXPECT_TRUE( mem_is_zero( looked_up_pw, looked_up_len ) );
+
+       EXPECT_TRUE( mem_is_zero( protected_mem, ProtectedMemSize ) );
+}
+
+TEST_F( PasswordRAMStoreTest, ReplacePassword )
+{
+       // Test a password can be saved and looked up, then saved again with a different
+       // password using the same key and looked up (and the single replaced password
+       // is erased and zeroed).
+       pw = "password";
+       EXPECT_TRUE( PasswordRAMStore::store( "key-single", pw.c_str() ) );
+
+       looked_up_pw = PasswordRAMStore::lookup( "key-single" );
+       EXPECT_STREQ( pw.c_str(), looked_up_pw );
+
+       pw = "password2";
+       EXPECT_TRUE( PasswordRAMStore::store( "key-single", pw.c_str() ) );
+
+       looked_up_pw = PasswordRAMStore::lookup( "key-single" );
+       EXPECT_STREQ( pw.c_str(), looked_up_pw );
 
        looked_up_len = strlen( looked_up_pw );
        EXPECT_TRUE( PasswordRAMStore::erase( "key-single" ) );
@@ -166,7 +190,7 @@ TEST_F( PasswordRAMStoreTest, OneHundredPasswordsForwards )
        for ( i = 0 ; i < 100 ; i ++ )
        {
                pw = gen_passwd( i );
-               EXPECT_TRUE( PasswordRAMStore::insert( gen_key(i), pw.c_str() ) );
+               EXPECT_TRUE( PasswordRAMStore::store( gen_key(i), pw.c_str() ) );
        }
 
        for ( i = 0 ; i < 100 ; i ++ )
@@ -196,7 +220,7 @@ TEST_F( PasswordRAMStoreTest, OneHundredPasswordsBackwards )
        for ( i = 0 ; i < 100 ; i ++ )
        {
                pw = gen_passwd( i );
-               EXPECT_TRUE( PasswordRAMStore::insert( gen_key(i), pw.c_str() ) );
+               EXPECT_TRUE( PasswordRAMStore::store( gen_key(i), pw.c_str() ) );
        }
 
        for ( i = 0 ; i < 100 ; i ++ )
@@ -224,7 +248,7 @@ TEST_F( PasswordRAMStoreTest, LongPassword )
        // [Implementation knowledge: size]
        pw = std::string( ProtectedMemSize-1, 'X' );
 
-       EXPECT_TRUE( PasswordRAMStore::insert( "key-long", pw.c_str() ) );
+       EXPECT_TRUE( PasswordRAMStore::store( "key-long", pw.c_str() ) );
 
        looked_up_pw = PasswordRAMStore::lookup( "key-long" );
        EXPECT_STREQ( pw.c_str(), looked_up_pw );
@@ -242,7 +266,7 @@ TEST_F( PasswordRAMStoreTest, TooLongPassword )
        // [Implementation knowledge: size]
        std::string pw = std::string( ProtectedMemSize, 'X' );
 
-       EXPECT_FALSE( PasswordRAMStore::insert( "key-too-long", pw.c_str() ) );
+       EXPECT_FALSE( PasswordRAMStore::store( "key-too-long", pw.c_str() ) );
        EXPECT_TRUE( mem_is_zero( protected_mem, ProtectedMemSize ) );
 
        looked_up_pw = PasswordRAMStore::lookup( "key-too-long" );
@@ -260,7 +284,7 @@ TEST_F( PasswordRAMStoreTest, TotalErasure )
        for ( i = 0 ; i < 100 ; i ++ )
        {
                pw = gen_passwd( i );
-               EXPECT_TRUE( PasswordRAMStore::insert( gen_key(i), pw.c_str() ) );
+               EXPECT_TRUE( PasswordRAMStore::store( gen_key(i), pw.c_str() ) );
        }
        EXPECT_FALSE( mem_is_zero( protected_mem, ProtectedMemSize ) );
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]