I am looking at some decompiled Java code from an Android app. As a security measure, a signature is passed as a parameter to a number of JSON requests.
The method that generates the signature is as so:
private String getSignature(String s, String s1)
{
try
{
Charset charset = Charset.forName("US-ASCII");
Mac mac = Mac.getInstance("HmacSHA256");
mac.init(new SecretKeySpec(master_key, "HmacSHA256"));
mac.init(new SecretKeySpec(mac.doFinal(charset.encode(s).array()), "HmacSHA256"));
mac.init(new SecretKeySpec(mac.doFinal(charset.encode(s1).array()), "HmacSHA256"));
s = Hex.toHexString(mac.doFinal(charset.encode(serialNumber).array()));
}
// Misplaced declaration of an exception variable
catch (String s)
{
s.printStackTrace();
return null;
}
return s;
}
As you can see, the key of the HMAC is set to the master_key
(a preset byte array). A piece of data is then passed through the hash and used as the key for the HMAC on the next step.
The function is called like so:
s2 = getDateString();
s3 = getSignature(s2, "/xquery/1.0/relay.json");
Is there any reason to chain together the HMAC like this? Is it anymore secure than simply performing HMAC(master_key,s|s1|serialNumber)
?
String
instances in Java, just objects that implementThrowable
. I presume this is an over-simplification of the code you got back from the disassembler.return null;
is of course inexcusable - you don't usenull
to indicate a security failure. What you've got here is an OK security protocol and very shady programming practices. – Maarten Bodewes Jul 11 '15 at 09:03