Constructing and validating a Gigya signature

Go To StackoverFlow.com

10

I wrote a method to verify a gigya signature against a specified timestamp and UID, based on Gigya's instructions for constructing a signature. Here is Gigya's psuedo code for doing that:

string constructSignature(string timestamp, string UID, string secretKey) {
    // Construct a "base string" for signing
    baseString = timestamp + "_" + UID;
    // Convert the base string into a binary array
    binaryBaseString = ConvertUTF8ToBytes(baseString);
    // Convert secretKey from BASE64 to a binary array
    binaryKey = ConvertFromBase64ToBytes(secretKey);
    // Use the HMAC-SHA1 algorithm to calculate the signature 
    binarySignature = hmacsha1(binaryKey, baseString);
    // Convert the signature to a BASE64
    signature = ConvertToBase64(binarySignature);
    return signature;
}

[sic]

Here's my method (exception handling omitted):

public boolean verifyGigyaSig(String uid, String timestamp, String signature) {

    // Construct the "base string"
    String baseString = timestamp + "_" + uid;

    // Convert the base string into a binary array
    byte[] baseBytes = baseString.getBytes("UTF-8");

    // Convert secretKey from BASE64 to a binary array
    String secretKey = MyConfig.getGigyaSecretKey();
    byte[] secretKeyBytes = Base64.decodeBase64(secretKey);

    // Use the HMAC-SHA1 algorithm to calculate the signature 
    Mac mac = Mac.getInstance("HmacSHA1");
    mac.init(new SecretKeySpec(secretKeyBytes, "HmacSHA1"));
    byte[] signatureBytes = mac.doFinal(baseBytes);

    // Convert the signature to a BASE64
    String calculatedSignature = Base64.encodeBase64String(signatureBytes);

    // Return true iff constructed signature equals specified signature
    return signature.equals(calculatedSignature);
}

This method is returning false even when it shouldn't. Can anyone spot something wrong with my implementation? I'm wondering if there could be an issue with the caller or gigya itself - "Your method checks out" is a valid answer.

I'm using Apache Commons' Base64 class for encoding.

Further (somewhat redundant) info on signatures is also found in Gigya's FAQ, in case that helps.

To clarify this further: uid, timestamp, and signature are all being taken from cookies set by gigya. In order to verify these aren't being spoofed, I'm taking uid and timestamp, and making sure signature can be reconstructed using my secret key. The fact that it fails when it shouldn't indicates a bug/format issue at some point in the process, either with my method, in the front-end, or with gigya itself. The purpose of this question is essentially to rule out a bug in the above method.

Note: I've also tried URL-encoding uid:

String baseString = timestamp + "_" + URLEncoder.encode(uid, "UTF-8");

Though I wouldn't think this would matter since it's just an integer. The same goes for timestamp.

Update:

The underlying issue has been solved, however the question itself remains open. See my answer for more details.

Update 2:

It turns out I was confused about which of apache's Base64 classes I was using - my code was not using the Commons Codec version but the Commons Net version. This confusion arose from my project's large amount of third-party libraries and my ignorance of the many Base64 implementations over the years from Apache libraries - a situation I now realize Commons Codec was meant to address. Looks like I'm late to the party when it comes to encoding.

After switching in Commons Codec's version, the method behaves correctly.

I'm going to award the bounty to @erickson since his answer was spot on, but please upvote both answers for their excellent insight! I'll leave the bounty open for now so they get the attention they deserve.

2012-04-05 18:28
by Paul Bellora
So, are you saying that if you extract calculation of the signature into sign(uid, timestamp) (everything except the return line), it will return different results for the exact same input when called multiple times - Vlad 2012-04-12 20:19
No, I'm saying that the result of constructing the sig from uid and timestamp, based on gigya's instructions, is not matching signature, which they are passing me - Paul Bellora 2012-04-12 20:22
@Vlad - Edited to try and clarify the question - Paul Bellora 2012-04-12 20:29
What is the point of this binaryBaseString = ConvertUTF8ToBytes(baseString);? binaryBaseString is never used in the gigya pseudocode...within the scope we are seeing. But in your function, you actually do use it ...mac.doFinal(baseBytes); then you turn the returned byte array into the calculatedSignature.. - Youssef G. 2012-04-12 20:54
@YoussefG. - Seems like a typo in their documentation. Nice eyes. I assume they meant binarySignature = hmacsha1(binaryKey, binaryBaseString);Paul Bellora 2012-04-12 20:59
have you tired "UTF8"? I doubt it will fix it, but its the low hanging fruit - Youssef G. 2012-04-12 21:04
@PaulBellora could you provide sample values of uid, timestamp and signature? Do they contain any non-alphanumeric characters - Vlad 2012-04-12 23:06
Does it fail 100% of the time - erickson 2012-04-14 00:12
Also, it looks like there are differences in the Base64 encoding depending on which version of the library you are using. What version of commons-codec are you using - erickson 2012-04-16 17:06
@erickson - I'm sorry for neglecting this post in the last couple days. After resolving the real world problem (see my answer) I had limited time to look into what went wrong here. Please see my update above - Paul Bellora 2012-04-16 20:44
As a general rule, because of possible base64 incompatibilities, I would avoid comparing the encoded strings and instead compare the decoded signatures. This avoids problems with padding and trailing garbage - Old Pro 2012-04-18 20:38
@OldPro - Excellent point - Paul Bellora 2012-04-18 23:06


5

Well I finally heard back from gigya yesterday regarding this issue, and it turns out their own server-side Java API exposes a method for handling this use case, SigUtils.validateUserSignature:

if (SigUtils.validateUserSignature(uid, timestamp, secretKey, signature)) { ... }

Today I was able to verify that this call is behaving correctly, so that solves the immediate issue and turns this whole post into a kind of a facepalm moment for me.

However:

I'm still interested in why my own home-rolled method doesn't work (and I have a bounty to award anyway). I'll examine it again this coming week and compare it with the SigUtils class file to try and figure out what went wrong.

2012-04-14 20:58
by Paul Bellora
I just had a look at the documentation on their custom implementation of Base64. Quoth they: The encoder produces the same output as the Sun one except that the Sun's encoder appends a trailing line separator if the last character isn't a pad. Unclear why but it only adds to the length and is probably a side effect. Both are in conformance with RFC 2045 though. Commons codec seem to always att (sic) a trailing line separator. Hm. I wonder how tested their encoder actually is.. - MrGomez 2012-04-15 07:53


10

I would take a close look at your Base-64 encoding and decoding.

Are you using a third-party library for this? If so, which one? If not, can you post your own implementation or at least some sample input and output (representing bytes with hexadecimal)?

Sometimes there are differences in the "extra" Base-64 characters that are used (substituting characters for '/' and '+'). Padding can also be omitted, which would cause string comparison to fail.


As I suspected, it is the Base-64 encoding that is causing this discrepancy. However, it is trailing whitespace that is causing the problem, not differences in padding or symbols.

The encodeBase64String() method that you are using always appends CRLF to its output. The Gigya signature does not include this trailing whitespace. Comparing these strings for equality fails only because of this difference in whitespace.

Use encodeBase64String() from the Commons Codec library (instead of Commons Net) to create a valid signature.

If we factor out the signature computation, and test its result against the Gigya SDK's verifier, we can see that removing the CRLF creates a valid signature:

public static void main(String... argv)
  throws Exception
{
  final String u = "";
  final String t = "";
  final String s = MyConfig.getGigyaSecretKey();

  final String signature = sign(u, t, s);
  System.out.print("Original valid? ");
  /* This prints "false" */
  System.out.println(SigUtils.validateUserSignature(u, t, s, signature));

  final String stripped = signature.replaceAll("\r\n$", "");
  System.out.print("Stripped valid? ");
  /* This prints "true" */
  System.out.println(SigUtils.validateUserSignature(u, t, s, stripped));
}

/* This is the original computation included in the question. */
static String sign(String uid, String timestamp, String key)
  throws Exception
{
  String baseString = timestamp + "_" + uid;
  byte[] baseBytes = baseString.getBytes("UTF-8");
  byte[] secretKeyBytes = Base64.decodeBase64(key);
  Mac mac = Mac.getInstance("HmacSHA1");
  mac.init(new SecretKeySpec(secretKeyBytes, "HmacSHA1"));
  byte[] signatureBytes = mac.doFinal(baseBytes);
  return Base64.encodeBase64String(signatureBytes);
}
2012-04-13 05:23
by erickson
+1. Based on the API calls, I'm going to assume the Base64 library being used here is Apache Commons' <code>Base64</code>. To my knowledge, it is indeed RFC4648 compliant, but you're absolutely correct about the padding and alphabet differences being able to cause a regression. Given Gigya's at least feigned Java support, I'm going to assume parity was at least considered... but the pseudocode flaw does not inspire confidence - MrGomez 2012-04-13 06:58
@MrGomez was correct about me using Apache Commons' Base64 class. I'll update the question with that information. +1 for the suggestion - Paul Bellora 2012-04-13 23:45
As noted in my updated answer, my previous comment was misinformed - Paul Bellora 2012-04-16 20:47
Very interesting update about the appended CRLF. I spoke to my team today about migrating our use in general of Commons Net's Base64 to Commons Codec's version. Unfortunately both have been used variously, and to make it worse our Codec lib is out of date (I know you mentioned subtle differences between versions). So it may be a long road to using a fully compliant and up-to-date Base64 across our codebase. Thanks again for the help, including the whitespace demonstration - Paul Bellora 2012-04-17 02:36
Ah, Commons Net, not Commons Codec. That's the key here. You'll note this is called out in the documentation of their custom Base64 implementation, and, as mentioned and benchmarked in my answer, the two (Commons Codec and Gigya) should be comparable - MrGomez 2012-04-17 10:19
These kinds of problems are why I would generally compare decoded binary blobs rather than base64 string encodings of them - Old Pro 2012-04-18 20:41
Interesting. This is a case where my +1 is going to cause the half-bounty to auto-grant to a question other than my own, unless the OP gets back in time. I think that's the most correct resolution -- you absolutely deserve it for the psychic debugging I completely missed. Sincerely: great job! : - MrGomez 2012-04-19 19:33
@MrGomez Thanks for the kind words - erickson 2012-04-19 21:29


7

Code review time! I love doing these. Let's check your solution and see where we fall.

In prose, our goal is to underscore-connect a timestamp and UID together, coerce the result from UTF-8 into a byte array, coerce a given Base64 secret key to into a second byte array, SHA-1 the two byte arrays together, then convert the result back to Base64. Simple, right?

(Yes, that pseudocode has a bug.)

Let's step through your code, now:

public boolean verifyGigyaSig(String uid, String timestamp, String signature) {

Your method signature here is fine. Though obviously, you'll want to make sure your created timestamps and the ones you're verifying are using the exact same format (otherwise, this will always fail) and that your Strings are UTF-8 encoded.

(Further details about how String encodings work in Java)

    // Construct the "base string"
    String baseString = timestamp + "_" + uid;

    // Convert the base string into a binary array
    byte[] baseBytes = baseString.getBytes("UTF-8");

This is fine (reference a, reference b). But, in the future, consider using StringBuilder for String concatenation explicitly, instead of relying on compiler-time optimizations to support this feature.

Note the documentation up to this point is inconsistent on whether to use "UTF-8" or "UTF8" as your charset identifier. "UTF-8" is the accepted identifier, though; I believe "UTF8" is kept for legacy and compatibility purposes.

    // Convert secretKey from BASE64 to a binary array
    String secretKey = MyConfig.getGigyaSecretKey();
    byte[] secretKeyBytes = Base64.decodeBase64(secretKey);

Hold it! This breaks encapsulation. It's functionally correct, but it would be better if you passed this as a parameter to your method than pulling it in from another source (thus coupling your code, in this case, to the details of MyConfig). Otherwise, this, too, is fine.

    // Use the HMAC-SHA1 algorithm to calculate the signature 
    Mac mac = Mac.getInstance("HmacSHA1");
    mac.init(new SecretKeySpec(secretKeyBytes, "HmacSHA1"));
    byte[] signatureBytes = mac.doFinal(baseBytes);

Yep, this is correct (reference a, reference b, reference c). I've nothing to add here.

    // Convert the signature to a BASE64
    String calculatedSignature = Base64.encodeBase64String(signatureBytes);

Correct, and...

    // Return true iff constructed signature equals specified signature
    return signature.equals(calculatedSignature);
}

... correct. Ignoring the caveats and implementation notes, your code checks out procedurally.

I would speculate on a few points, though:

  1. Are you UTF-8 encoding your input String for your UID or your timestamp, as defined here? If you've failed to do so, you're not going to get the results you expect!

  2. Are you sure the secret key is correct and properly encoded? Make sure to check this in a debugger!

  3. For that matter, verify the whole thing in a debugger if you have access to a signature generation algorithm, in Java or otherwise. Failing this, synthesizing one will help you check your work because of the encoding caveats raised in the documentation.

The pseudocode bug should be reported, as well.

I believe checking your work here, especially your String encodings, will expose the correct solution.


Edit:

I checked their implementation of Base64 against Apache Commons Codec's. Test code:

import org.apache.commons.codec.binary.Base64;
import static com.gigya.socialize.Base64.*;

import java.io.IOException;

public class CompareBase64 {
    public static void main(String[] args) 
      throws IOException, ClassNotFoundException {
        byte[] test = "This is a test string.".getBytes();
        String a = Base64.encodeBase64String(test);
        String b = encodeToString(test, false);
        byte[] c = Base64.decodeBase64(a);
        byte[] d = decode(b);
        assert(a.equals(b));
        for (int i = 0; i < c.length; ++i) {
            assert(c[i] == d[i]);
        }
        assert(Base64.encodeBase64String(c).equals(encodeToString(d, false)));
        System.out.println(a);
        System.out.println(b);
    }
}

Simple tests show that their output is comparable. Output:

dGhpcyBpcyBteSB0ZXN0IHN0cmluZw==
dGhpcyBpcyBteSB0ZXN0IHN0cmluZw==

I verified this in a debugger, just in case there might be whitespace I can't detect in visual analysis and the assert didn't hit. They're identical. I also checked a paragraph of lorem ipsum, just to be sure.

Here's the source code for their signature generator, sans Javadoc (author credit: Raviv Pavel):

public static boolean validateUserSignature(String UID, String timestamp, String secret, String signature) throws InvalidKeyException, UnsupportedEncodingException
{
    String expectedSig = calcSignature("HmacSHA1", timestamp+"_"+UID, Base64.decode(secret)); 
    return expectedSig.equals(signature);   
}

private static String calcSignature(String algorithmName, String text, byte[] key) throws InvalidKeyException, UnsupportedEncodingException  
{
    byte[] textData  = text.getBytes("UTF-8");
    SecretKeySpec signingKey = new SecretKeySpec(key, algorithmName);

    Mac mac;
    try {
        mac = Mac.getInstance(algorithmName);
    } catch (NoSuchAlgorithmException e) {
        return null;
    }

    mac.init(signingKey);
    byte[] rawHmac = mac.doFinal(textData);

    return Base64.encodeToString(rawHmac, false);           
}

Changing your function signature in line with some of the changes I made above and running this test case causes both signatures to be validated correctly:

// Redefined your method signature as: 
//  public static boolean verifyGigyaSig(
//      String uid, String timestamp, String secret, String signature)

public static void main(String[] args) throws 
  IOException,ClassNotFoundException,InvalidKeyException,
  NoSuchAlgorithmException,UnsupportedEncodingException {

    String uid = "10242048";
    String timestamp = "imagine this is a timestamp";
    String secret = "sosecure";

    String signature = calcSignature("HmacSHA1", 
              timestamp+"_"+uid, secret.getBytes());
    boolean yours = verifyGigyaSig(
              uid,timestamp,encodeToString(secret.getBytes(),false),signature);
    boolean theirs = validateUserSignature(
              uid,timestamp,encodeToString(secret.getBytes(),false),signature);
    assert(yours == theirs);
}

Of course, as reproduced, the problem is with Commons Net, whereas Commons Codec appears to be fine.

2012-04-13 02:25
by MrGomez
+1 Thanks for the comprehensive review! Your points are all well taken and I'll look into them when I have a chance. And Re: StringBuilder as I understand it the compiler will optimize concatenation within a single statement so it shouldn't matter in this case - Paul Bellora 2012-04-13 23:52
@PaulBellora To my understanding, that only applies to String literals. The reason for this is the compiler is able to perform the concatenation and store the value in a single String instance, instead of attempting to perform said concatenation on each pass. Unfortunately, you can't get away with this optimization with variables (as in your example), because there's no easily unrolled representation of them for the complete space of String on either side of the concatenation. Therefore, StringBuilder is appropriate here for the efficiency gains, if that makes sense. : - MrGomez 2012-04-14 15:04
I didn't mean they would be optimized into one string, which as you say is impossible. What I meant was that as long as the concatenation takes place in one statement (ie without a loop), then the compiler will optimize that to use a StringBuilder instead. So my statement and yours effectively become the same bytecode. I haven't verified this myself by examining bytecode but it's what the linked answer, and other answers on SO have indicated - Paul Bellora 2012-04-14 15:37
@PaulBellora Ah! Excuse my misunderstanding. You are correct, according to the documentation, so that bears a quick answer update. : - MrGomez 2012-04-14 15:48
@PaulBellora Answer updated with reference implementation straight from the Gigya source code and a simple test case. This should allow you, once and for all, to trace where your problem occurred. : - MrGomez 2012-04-16 23:05
Ads