4
$\begingroup$

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)?

$\endgroup$
  • $\begingroup$ You cannot catch String instances in Java, just objects that implement Throwable. 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 use null to indicate a security failure. What you've got here is an OK security protocol and very shady programming practices. $\endgroup$ – Maarten Bodewes Jul 11 '15 at 9:03
  • $\begingroup$ No - this is code verbatim from the disassembler. I ran it through a couple of different ones as well, as I was seeing totally nonsensical output in some location. It would be fair to say the code quality is poor. $\endgroup$ – Cybergibbons Jul 11 '15 at 10:46
4
$\begingroup$

If you are asking if $M(M(M(k, C_1) C_2), s)$ has the same or better security as $M(k, C_1 | C_2 | s)$ then the answer is yes. You can see this as a double key derivation to calculate a new key from $k$ using the constants $C_1$ and $C_2$ as derivation data. Then the resulting key is used to MAC the final serialNumber. $M$ is of course HMAC, $C_1$ and $C_2$ are s and s1 and $s$ represents the serialNumber.

If you look at the HMAC construction you will see that the key elements are repeated in the hashing and there are multiple passes of the underlying hash, but the elements remain the same. If you only perform concatenation then you must make sure that there are no collisions for the concatenated elements (xy|z would be identical to x|yz).


Note that the code uses lossy character encoding (ASCII only represents a small part of the Unicode code set). It should be replaced with UTF-8. Currently the input must be validated beforehand to only contain ASCII, but using UTF-8 in the code would add a layer of defense.


So in the end the protocol is fine. It should however be clearly documented.

| improve this answer | |
$\endgroup$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.