Is there any good reason to chain HMACs in this manner? 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)?
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.