Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Salt from CredentialModel is wrongly decoded #5

Open
jhomola79 opened this issue Dec 14, 2020 · 3 comments
Open

Salt from CredentialModel is wrongly decoded #5

jhomola79 opened this issue Dec 14, 2020 · 3 comments

Comments

@jhomola79
Copy link

Hi,
I have found a failing test. This way, salt is wrongly base64 decoded to only 15 bytes instead of 16 and verification fails. Probably more Keycloak bug, but maybe can interest you.

@Test
public void testArgon2iVerifyPredefinedHashFromCredentialModel() {
    String rawPassword = "tekvica";
    CredentialModel credentialModel = new CredentialModel();
    credentialModel.setSecretData("{\"value\":\"$argon2i$v=19$m=16,t=2,p=1$a3I5aE5QR3N0U3VGU05peg$fHjVdRNXcS+bS2D4Vu+IHw\",\"salt\":\"a3I5aE5QR3N0U3VGU05peg\"}");
    credentialModel.setCredentialData("{\"hashIterations\":1,\"algorithm\":\"argon2\"}");
    PasswordCredentialModel passwordCredentialModel = PasswordCredentialModel.createFromCredentialModel(credentialModel);
    boolean verified = Argon2Helper.verifyPassword(rawPassword, passwordCredentialModel);
    Assert.assertTrue(verified);
}
@dreezey
Copy link
Owner

dreezey commented Dec 29, 2020

Hi @jhomola79 , thanks for the report.

I have looked into this and indeed when the JSON is serialized to PasswordSecretData, the last element of the byte array seems to get lost, see this line, not entirely sure why but it may be worth reporting this to Keycloak team itself?

I have used java.util.Base64 Utility class in your function to verify with the debugger, and this one seems to properly decode it; byte[16] instead of byte[15] produced by Keycloak's Base64 utility class:

@Test
    public void testArgon2iVerifyPredefinedHashFromCredentialModel() {
        String rawPassword = "tekvica";
        String encodedSalt = "a3I5aE5QR3N0U3VGU05peg";
        byte[] salt = Base64.getDecoder().decode(encodedSalt.getBytes(StandardCharsets.UTF_8));
[X1]    CredentialModel credentialModel = new CredentialModel();
        credentialModel.setSecretData("{\"value\":\"$argon2i$v=19$m=16,t=2,p=1$" + encodedSalt + "$fHjVdRNXcS+bS2D4Vu+IHw\",\"salt\":\"" + encodedSalt + "\"}");
        credentialModel.setCredentialData("{\"hashIterations\":1,\"algorithm\":\"argon2\"}");
        PasswordCredentialModel passwordCredentialModel = PasswordCredentialModel.createFromCredentialModel(credentialModel);
[X2]    boolean verified = Argon2Helper.verifyPassword(rawPassword, passwordCredentialModel);
        Assert.assertTrue(verified);
    }

[X] = breakpoint.
[X1] => salt = byte[16]
[X2] => passwordCredentialModel.secretData.salt = byte[15]

I'll look into bypassing the Keycloak stored salt property, and instead use the one embedded in the encoded Argon2 string:
$argon2i$v=19$m=16,t=2,p=1$a3I5aE5QR3N0U3VGU05peg$fHjVdRNXcS+bS2D4Vu+IHw.

@jhomola79
Copy link
Author

jhomola79 commented Dec 30, 2020

Hi,
yes I did something similar, fully ignore that last, doubled salt data. I also used that 'underscoreunderscoreSALTunderscoreunderscore' from PasswordSecretData to not even need to send salt value there.
Im a bit worried to report to Keycloak, that they have wrongly implemented base64 decoding. : ) If not missed something.

@waweber
Copy link

waweber commented Aug 2, 2021

@jhomola79 @dreezey this issue on the Keycloak jira might be related and might benefit from your input: https://issues.redhat.com/projects/KEYCLOAK/issues/KEYCLOAK-18914

hangy referenced this issue in hangy/scrypt-password-hash-provider Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants