0

I was trying to get a hashed password following the code in here. From it, i'm only using, at the moment, the code for the salt method, the hash method and isExpectedPassword method.

I get my password from a text field:

char[] passCharArray = txtPassword.toString().toCharArray();

Then I call the class to get a salt value (I call it Encryptor instead of Passwords like in the original post):

byte[] salt = Encryptor.getNextSalt();

And then I get the hashed password:

byte[] hashedPass = Encryptor.hash(passCharArray, salt);

Using the following code I print the results to see what's going on and the results are commented:

String saltString = Arrays.toString(salt);
System.out.println("SALT: " + saltString);
//SALT: [18, 117, -98, 41, 92, 124, 118, 17, 107, 14, 0, -81, 110, 70, 10, 42]

String hashedPassString = Arrays.toString(hashedPass);
System.out.println("HASHED PASS: " + hashedPassString);
//HASHED PASS: [44, -127, -43, 84, 40, -16, -46, -71, 109, -44, -41, 47, -61, -119, 21, 99, -23, 101, -13, 116, -12, 118, -66, 44, 104, 5, 4, 18, -55, 47, 59, 116]

System.out.println("Passwords match: " + Encryptor.isExpectedPassword(passCharArray, salt, hashedPass));
//Passwords match: false

The below two are the System.out.prints I put in the isExpectedPassword method to see what values that one has upon being called.

//Encryptor pwdHash: [-103, -87, 53, -75, 59, 11, 77, 116, 123, 59, 68, -35, 16, -68, 42, 34, -32, 75, 22, -94, -37, -26, 16, 20, 7, -46, -6, -20, -88, 104, -121, 77]
//Encryptor expectedHash: [44, -127, -43, 84, 40, -16, -46, -71, 109, -44, -41, 47, -61, -119, 21, 99, -23, 101, -13, 116, -12, 118, -66, 44, 104, 5, 4, 18, -55, 47, 59, 116]

So basically, hashedPass (and expectedHash) should be the same as pwdHash, but it's not. I don't understand what I'm doing wrong. Am I missing something in my code? Does something change without my knowledge?

This is my full code, if people want to see the whole thing just in case:

public class Encryptor {

    private static final Random RANDOM = new SecureRandom();
    private static final int ITERATIONS = 10000;
    private static final int KEY_LENGTH = 256;

    private Encryptor(){}

    public static byte[] getNextSalt(){
        byte[] salt = new byte[16];
        RANDOM.nextBytes(salt);
        return salt;
    }

    public static byte[] hash(char[] password, byte[] salt) {
        PBEKeySpec spec = new PBEKeySpec(password, salt, ITERATIONS, KEY_LENGTH);
        Arrays.fill(password, Character.MIN_VALUE);
        try {
            SecretKeyFactory skf = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1");
            return skf.generateSecret(spec).getEncoded();
        } catch (NoSuchAlgorithmException | InvalidKeySpecException e) {
            throw new AssertionError("Error while hashing a password: " + e.getMessage(), e);
        } finally {
            spec.clearPassword();
        }
    }

    public static boolean isExpectedPassword(char[] password, byte[] salt, byte[] expectedHash) {
        byte[] pwdHash = hash(password, salt);

        String s = Arrays.toString(pwdHash);
        System.out.println("Encryptor pwdHash: " + s);

        String s2 = Arrays.toString(expectedHash);
        System.out.println("Encryptor expectedHash: " + s2);

        Arrays.fill(password, Character.MIN_VALUE);
        if (pwdHash.length != expectedHash.length) return false;
        for (int i = 0; i < pwdHash.length; i++) {
            if (pwdHash[i] != expectedHash[i]) return false;
        }
        return true;
    }


}


public class Controller implements Initializable {
    @FXML
    private Button btnLogin;
    //Some private variables

    @FXML
    private AnchorPane ancPane;
    @FXML
    private ImageView imgLogo;
    @FXML
    private Hyperlink hplRegister;
    @FXML
    private TextField txtUsername;
    @FXML
    private TextField txtPassword;

    @Override
    public void initialize(URL url, ResourceBundle resourceBundle) {
        //Some styling

        hplRegister.setOnAction(event -> {
            //Registering event
        });

        btnLogin.setOnAction(event -> {
            try {

                //Loading fxml data
                // I've put the code here just for testing purposes
                // and will not be the final placement.

                char[] passCharArray = txtPassword.toString().toCharArray();

                byte[] salt = Encryptor.getNextSalt();
                byte[] hashedPass = Encryptor.hash(passCharArray, salt);

                String saltString = Arrays.toString(salt);
                System.out.println("SALT: " + saltString);

                String hashedPassString = Arrays.toString(hashedPass);
                System.out.println("HASHED PASS: " + hashedPassString);

                System.out.println("Passwords match: " + Encryptor.isExpectedPassword(passCharArray, salt, hashedPass));

            }catch (Exception e){
                e.printStackTrace();
            }
        });
    }

    //Some getter methods.
}
Doombringer
  • 596
  • 4
  • 19
  • 1
    Your salt isn't the same. `getNextSalt()` will randomly create a salt each call. This will cause your hash to be different every time – locus2k Dec 06 '18 at 21:39
  • Furthermore `TextField.toString` does not return the content of the `TextField`. The result is something like `TextField@5ef8216b[styleClass=text-input text-field]` or `PasswordField@67d95601[styleClass=text-input text-field password-field]` for `PasswordField`. You need to use `getText()` instead. – fabian Dec 06 '18 at 22:20
  • @locus2k That's why I store it in `byte[] salt`, so I don't have to call it again. I'm using the same variable in `byte[] hashedPass` and `Encryptor.isExpectedPassword(passCharArray, salt, hashedPass)`. – Doombringer Dec 06 '18 at 22:38
  • the problem is not `getNextSalt()` or the field. @fabian is right that the text you're getting is not the text you want, but the hash and match should still work. see below for answer – mavriksc Dec 06 '18 at 23:21

1 Answers1

1

The problem is you are wiping out password. when you hash the password you wipe out the char array fill it with blanks this clears passCharArray. When you pass it the second time it's basically checking the hash of the password versus the hash of a blank array. and those definitely do not match.

In a real case you will get salt and hashed password from a db or some other source. The incoming version will not be hashed, and thus cleared, until passed to isExpectedPassword

mavriksc
  • 1,130
  • 1
  • 7
  • 10
  • After fixing `txtPassword.toString().toCharArray();` to `txtPassword.getText().toCharArray();` and commenting off the `Arrays.fill(password, Character.MIN_VALUE);`, it seems to work properly. I don't even know why it's there in the first place. – Doombringer Dec 07 '18 at 01:59
  • it's there because it doesn't want those values just floating around waiting to be logged or by some other means obtained. – mavriksc Dec 07 '18 at 05:09