0

In my application, I use the users password as the encryption key for encryption media. I am encrypting media using PBEWithMD5AndDES and this works fine with a password stored in shared preferences. Now to achieve a level of security I am removing the password from shared preferences and using a singleton that is only kept alive during the app session (as the app logs out automatically requiring entry of the password). Below is my singleton:

public class Credentials {

private static Credentials dataObject = null;

private Credentials() {
// left blank intentionally
}

public static Credentials getInstance() {
if (dataObject == null)
    dataObject = new Credentials();
return dataObject;
}

private char[] user_password;

public char[] getUser_password() {

return user_password;
}

 public void setUser_password(char[] user_password) {

this.user_password = user_password;
}
}

The password is zeroed out from memory if the app logs out, or is log out by the user or gets destroyed. However at times I am getting a null pointer when trying to retrieve the password.

   char[] pswd = Credentials.getInstance().getUser_password();

What could be causing this? is there any other method I can use except a singleton?

James Wahome
  • 588
  • 11
  • 31
  • 1
    `Credentials.getInstance()` cannot not return `null`. It's that you've never called `setUser_password` on that instance so `Credentials.getInstance().getUser_password()` does. And if you think that you have called `setUser_password` (with a non-null parameter), the problem is that your singleton class isn't thread safe. – Andy Turner Feb 20 '17 at 11:35
  • @James Wahome: Your `password` is null not singleton!! – AndiGeeky Feb 20 '17 at 11:37
  • Add proper check for null object for singleton Instance and values return by its functions. – Chetan Joshi Feb 20 '17 at 11:40
  • @AndyTurner I am sure setUser_password is called, and a value does exit, the problem is that after sometime it becomes null and as such I cannot use it – James Wahome Feb 20 '17 at 11:43
  • "And if you think that you have called setUser_password (with a non-null parameter), the problem is that your singleton class isn't thread safe.". – Andy Turner Feb 20 '17 at 11:43
  • @Chetan checks exit to test if null, thats how I found out its null – James Wahome Feb 20 '17 at 11:43
  • @JamesWahome: You better implement singletons using Enum, if you absolutely have to. Otherwise you will run into situations like this as the implementation above is not thread-safe, and will fall victims to race conditions. – MD Sayem Ahmed Feb 20 '17 at 11:44
  • @MDSayemAhmed enum is not appropriate here, because the value is mutable. Enum values should always be immutable. – Andy Turner Feb 20 '17 at 11:46
  • @AndyTurner which is the best approach to achieve thread saftey ? – James Wahome Feb 20 '17 at 11:47
  • @JamesWahome by far the easiest thing is to eagerly initialize the field: `private static final Credentials dataObject = new Credentials();`. – Andy Turner Feb 20 '17 at 11:49
  • @AndyTurner creating an instance every time I want to use it will make it thread safe and prevent the nulls? – James Wahome Feb 20 '17 at 11:51
  • @AndyTurner: Oh, did not notice that. Then eager initialisation seems to be the way to go. – MD Sayem Ahmed Feb 20 '17 at 11:54
  • @JamesWahome we can check Credentials dataObject ; is null or not and off corse in each case if is null then get instance method initialize this object and return new object of Credentials class.in that case user_password value is null and then you must check null while getting value from it. – Chetan Joshi Feb 20 '17 at 12:46

1 Answers1

-1

Alternatively, you can store the password using built-in Sqlite db, though I'd still recommend you save it encrypted for max protection. You can do this in 4 steps:

2) Create an entity object to store the password:

public class Password {
    int password_id; // will be auto-increamted
    String password;

    public Password(int password_id, String password) {
        this.password_id = password_id;
        this.password = password;
    }
// getter/setters ...
}

2) Create an Sqlite utility object:

public class SQLiteDBAdapter {

    protected static final String DATABASE_NAME = "mydb";
    protected static final int DATABASE_VERSION = 1;

    protected Context context;
    protected static DatabaseHelper mDbHelper;

    public static final String TABLE_PASSWORD = "tbl_password";
    // columns
    public static final String PASSWORD_ID = "_id";
    public static final String PASSWORD = "password";
    // create table string
    private static final String CREATE_TABLE_PASSWORD =
            "CREATE TABLE if not exists " + TABLE_PASSWORD + " ( " +
                    PASSWORD_ID + " INTEGER PRIMARY KEY AUTOINCREMENT, " +
                    PASSWORD + " TEXT NOT NULL);";

    public SQLiteDBAdapter(Context context) {
      context = context.getApplicationContext();
    }

    public SQLiteDatabase openDb() {
      if (mDbHelper == null) {
          mDbHelper = new DatabaseHelper(mContext);
      }
      return mDbHelper.getWritableDatabase();
    }

    protected static class DatabaseHelper extends SQLiteOpenHelper {
      // -------------------------------------------------------------------------------------------
      public DatabaseHelper(Context context) {
          super(context, DATABASE_NAME, null, DATABASE_VERSION);
      }
      // -------------------------------------------------------------------------------------------
      @Override
      public void onCreate(SQLiteDatabase db) {
          db.execSQL(CREATE_TABLE_PASSWORD);
      }
      // -------------------------------------------------------------------------------------------
      @Override
      public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {
          Log.w(TAG, "Upgrading database from version " + oldVersion + " to " +
                  newVersion + ", which will destroy all old data");
          db.execSQL("DROP TABLE IF EXISTS routes");
          onCreate(db);
      }
    }
}

3) Extend an Sqlite object to manipulate the table (CRUD operations):

public class PasswordDbAdapter extends SQLiteDBAdapter {

    private SQLiteDatabase db;

    // these are column corresponding indices
    public static final int INDEX_PASSWORD_ID = 0;  // an auto-increment
    public static final int INDEX_PASSWORD = 1;

    public PasswordDbAdapter(Context context) {
        super(context);
    }

    public void addPassword(String password) {
        db = openDb();
        ContentValues values = new ContentValues();
        values.put(PASSWORD, password);
        db.insert(TABLE_PASSWORD, null, values);
    }

    public void updatePassword(String password) {
        db = openDb();
        ContentValues values = new ContentValues();
        values.put(PASSWORD, password);
        db.update(TABLE_PASSWORD, values, null);
    }

    public void deletePassword() {
        db = openDb();
        db.delete(TABLE_PASSWORD, null, null);
    }

    public boolean isEmpty() {
        db = openDb();
        boolean empty = true;
        Cursor cur = db.rawQuery("SELECT COUNT(*) FROM " + TABLE_PASSWORD, null);
        if (cur != null && cur.moveToFirst()) {
            empty = (cur.getInt (0) == 0);
        }
        cur.close();
        return empty;
    }

    public Password fetchPassword() {   // ok because there's only one password record
        db = openDb();
        Cursor cursor = db.query(TABLE_PASSWORD, new String[]{PASSWORD_ID, PASSWORD},
                null, null, null, null, null, null);
        if (cursor != null &&
            cursor.moveToFirst()) {
            return new Password(
                    cursor.getString(INDEX_PASSWORD_ID),
                    cursor.getInt(INDEX_PASSWORD));
        }
        return null;
    }
}

4) Finally, save/update/retrieve the password as desired:

public class MainActivity extends AppCompatActivity {
    private PasswordDbAdapter passwordDB; 
    @Override
    protected void onCreate(Bundle savedInstanceState) {
        ... 
        // initialize the password db
        passwordDB = new PasswordDbAdapter(this);

        // check if password record exists
        if (passwordDB.isEmpty() {
            // save a new copy
            passwordDB.addPassword("the_password"); // more secure if it is saved encrypted
        } else {
            // update it
            passwordDB.updatePassword("the_password");
        }

    }
    ...
    public String fetchPassword() {
        return passwordDB.fetchPassword();  // or first decrypt it, then return it
    }
}
nkmuturi
  • 248
  • 3
  • 10
  • This is WRONG ....read through the question "Now to achieve a level of security I am removing the password from shared preferences and using a singleton that is only kept alive during the app session" My objective is to store it in runtime memory and not persist it in shared prefs of disk storage. In any case you should never store a password in plain text – James Wahome Feb 20 '17 at 14:47
  • Android singletons can be very problematic. But, see http://stackoverflow.com/questions/16517702/singleton-in-android. Now, concerning using Sqlite, my recommendation is to store the password encrypted. Simpler still, you can hash the password, and use the hash instead. – nkmuturi Feb 20 '17 at 18:49