• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 1092
  • Last Modified:

HMAC implementation to encrypt cookies; Performance and possible multithreading issues

We are in the process of encrypting the application cookies so that it cannot be tampered by the user. Meaning, our application creates certain cookie to store application data. Through some softwares, users have the ability to change the data in the cookie before sending them. We want to avoid this by encyrpting the cookie data; get a token and attach it to the cookie. Next time, when the application gets the cookie, we would remove the token from the cookie, create the token again and see whether the token sent matches the one created now.

Cookie Util class

import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;

import javax.crypto.Mac;
import javax.crypto.spec.SecretKeySpec;

public class CookieUtil {
 
 private static final int CODE_DIGITS = 5;
 private static final String HMAC = "HmacSHA1";
 private static final String SECRET = "WRHLCESS!DDE";
 
 private static byte[] hmac_sha1(byte[] keyBytes, byte[] text)
    throws NoSuchAlgorithmException, InvalidKeyException {
        Mac hmacSha1;
        try {
            hmacSha1 = Mac.getInstance(HMAC);
        } catch (NoSuchAlgorithmException nsae) {
            return new byte[0];
        }
        SecretKeySpec macKey =  
        new SecretKeySpec(keyBytes, "RAW");
        hmacSha1.init(macKey);
        return hmacSha1.doFinal(text);
 }
 
 public static String getCryptographicToken(String text)
   throws InvalidKeyException, NoSuchAlgorithmException{
  byte[] secretKey = SECRET.getBytes();
  byte[] hash = hmac_sha1(secretKey,text.getBytes());
  // Mask the output and get the first codeDigit characters
  // as the cryptographic token
  int offset = hash[hash.length - 1] & 0xf;
        int binary =
                 ((hash[offset] & 0x7f) << 24)  
                 | ((hash[offset + 1] & 0xff) << 16)
                 | ((hash[offset + 2] & 0xff) << 8)
                 | (hash[offset + 3] & 0xff);

        double otp = binary % Math.pow(10, CODE_DIGITS);
        String result = Integer.toString((int)otp);
        while (result.length() < CODE_DIGITS) {
           result = "0" + result;
        }
     return result;
 }
  public static String getCryptographicToken(String text)
    throws InvalidKeyException, NoSuchAlgorithmException{
   byte[] secretKey = SECRET.getBytes();
   byte[] hash = hmac_sha1(secretKey,text.getBytes());
   // Mask the output and get the first codeDigit characters
   // as the cryptographic token
   int offset = hash[hash.length - 1] & 0xf;
         int binary =
                  ((hash[offset] & 0x7f) << 24)  
                  | ((hash[offset + 1] & 0xff) << 16)
                  | ((hash[offset + 2] & 0xff) << 8)
                  | (hash[offset + 3] & 0xff);
 
         double otp = binary % Math.pow(10, CODE_DIGITS);
         String result = Integer.toString((int)otp);
         while (result.length() < CODE_DIGITS) {
            result = "0" + result;
         }
      return result;
  }
 
  public static String seperateTokenFromCookieString(String cookieString){
   
   return cookieString.substring(cookieString.length() - CODE_DIGITS, cookieString.length());
  }
 
  public static String seperateCookieStringFromToken(String cookieString){
   
   return cookieString.substring(0, cookieString.length() - CODE_DIGITS);
  }
}


Code to set the cookie
String cryptToken = CookieUtil.getCryptographicToken(cookieString);
Cookie applicationCookie = new Cookie(applicationCookie, cookieString + cryptToken);


Code to retrieve the cookie
applicationCookie = cookies[i].getValue();
String existingCryptToken = CookieUtil.seperateTokenFromCookieString(applicationCookie);
String generatedToken = CookieUtil.getCryptographicToken(CookieUtil.seperateCookieStringFromToken(applicationCookie));
if(!existingCryptToken.equals(generatedToken)) {
      ...throw exception..
}

With the above implementation, everything works but the performance has degraded as it seems there are small synchronized blocks in two method calls in the hmac_sha1 method above. So, in order to reduce that, we made the following changes in the CookieUtil to put the synchronized blocks to a class-level static block so it is executed only once.

import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;

import javax.crypto.Mac;
import javax.crypto.spec.SecretKeySpec;

import com.schwab.sim.sicom.utilities.exceptions.SIException;


public class CookieUtil {
      
      private static final int CODE_DIGITS = 5;
      private static final String HMAC = "HmacSHA1";
      private static final String SECRET = "WRHLCESS!DDE";
      private static byte[] hmac_sha1 = null;
      private static Mac hmacSha1 = null;
      
      static {
            try {
            hmacSha1 = Mac.getInstance(HMAC);
        } catch (NoSuchAlgorithmException nsae) {
              // Ignore
        }
        SecretKeySpec macKey =  
        new SecretKeySpec(SECRET.getBytes(), "RAW");
        try {
                  hmacSha1.init(macKey);
            } catch (InvalidKeyException e) {
                  // Ignore
            }
      }
      
      public static String getCryptographicToken(String text)
                  throws SIException{
            if(hmacSha1 == null){
                  throw new SIException("Mac object got from the static block was Null");
            }
            byte[] secretKey = SECRET.getBytes();
            Mac mac = null;
            try {
                  mac = (Mac)hmacSha1.clone();
            } catch (CloneNotSupportedException e) {
                  throw new SIException(e.getMessage(), e);
            }
            byte[] hash = mac.doFinal(text.getBytes());
            // Mask the output and get the first codeDigit characters
            // as the cryptographic token
            int offset = hash[hash.length - 1] & 0xf;
        int binary =
                      ((hash[offset] & 0x7f) << 24)  
                      | ((hash[offset + 1] & 0xff) << 16)
                      | ((hash[offset + 2] & 0xff) << 8)
                      | (hash[offset + 3] & 0xff);
        double otp = binary % Math.pow(10, CODE_DIGITS);
          String result = Integer.toString((int)otp);
          while (result.length() < CODE_DIGITS) {
               result = "0" + result;
          }
          return result;
      }
      
      public static String seperateTokenFromCookieString(String cookieString){
            
            return cookieString.substring(cookieString.length() - CODE_DIGITS, cookieString.length());
      }
      
      public static String seperateCookieStringFromToken(String cookieString){
            
            return cookieString.substring(0, cookieString.length() - CODE_DIGITS);
      }
}


The problem is after we moved the code to the static block, sometimes the tokens does not match with load test with 20+ concurrent users; so the application fails for certain random users (the same load test works fine with the first implementation). Could it be a multi-threading issue? Also, is there a better or alternate way to implement this?
0
innumonenu
Asked:
innumonenu
  • 4
  • 2
  • 2
  • +1
3 Solutions
 
objectsCommented:
are you running it in a container?
0
 
CEHJCommented:
I suspect there are two issues here:

a. encoding/decoding the hash digest (code at a v.quick glance doesn't look quite right)
b. creating a digest can be expensive - you could try a different way

Let's take a. first. Have a look at the following:

          public static String byteArrayToHexString(byte[] rawBytes) {
            String charIndex = "0123456789abcdef";
            StringBuffer sb = new StringBuffer(rawBytes.length * 2);
            for (int i = 0; i < rawBytes.length; i++) {
              int ix = (rawBytes[i] >> 4) & 0xF;
              sb.append(charIndex.charAt(ix));
              ix = rawBytes[i] & 0xF;
              sb.append(charIndex.charAt(ix));
            }
            return sb.toString();
          }


                  /*
                   * The reverse of byteArrayToHexString
                   */

      public static byte[] hexStringToByteArray(String s) {
        String charIndex = "0123456789abcdef";
        int stringIndex = 0;
        byte[] bytes = new byte[s.length() / 2];
        s = s.toLowerCase();
        for (int i = 0; i < bytes.length; i++) {
          stringIndex = i * 2;
          byte n = (byte) charIndex.indexOf(s.charAt(stringIndex));
          n <<= 4;
          bytes[i] |= n;
          n = (byte) charIndex.indexOf(s.charAt(stringIndex + 1));
          bytes[i] |= n;
        }
        return bytes;
      }

0
 
objectsCommented:
If its being run in a container you can't do your initialisation in a static block, it needs to be done by a bean in your application context.
0
VIDEO: THE CONCERTO CLOUD FOR HEALTHCARE

Modern healthcare requires a modern cloud. View this brief video to understand how the Concerto Cloud for Healthcare can help your organization.

 
WebstormCommented:
Hi innumonenu,

String concatenation & double computation can slow your code:
         double otp = binary % Math.pow(10, CODE_DIGITS);
         String result = Integer.toString((int)otp);
         while (result.length() < CODE_DIGITS) {
            result = "0" + result;
         }
->
         String result = "0000000000"+Integer.toString((int)otp);
         result=result.substring(result.length()-CODE_DIGITS);

0
 
WebstormCommented:
        String result = "0000000000"+Integer.toString((int)binary);  // instead of otp
         result=result.substring(result.length()-CODE_DIGITS);
0
 
innumonenuAuthor Commented:
We tried a new implementation using XOR operator but still the peformance interms of response time is more. We are yet to try the above options as we thought cryptographic algothirms are inherently time consuming.
0
 
objectsCommented:
> So, in order to reduce that, we made the following changes in the CookieUtil to put the synchronized blocks to a class-level static block so it is executed only once.

I thought u resolved the performance issues
0
 
innumonenuAuthor Commented:
No, we still have performance issue around the implementation. We tried the following implementation along with the result,

1. Cryptographic SHA1 implementation without the static block; create MAC instance per request > No issues with functionality but performance issue
2. Cryptographic SHA1 implementation with static block> Issues with functionality; meaning the tokens generated for requests does not match. Wondering multi threading issues.
3. XOR implementation (take each characeter in the cookie and XOR it with the next characters and generate a token) > No issues with functionality but performance issue
4. Cryptographic MD5 implementation with static block >  No issues with functionality but performance issue

Does any one point to a good sample implementation of a cryptographic implementaion of the cookie without the performance (thoroughput) issue? Requirement it to encrypt\decrypt the cookie to find out whether it is tampered or not? What is the best practice around this?
0
 
objectsCommented:
> 2. Cryptographic SHA1 implementation with static block> Issues with functionality; meaning the tokens generated for
> requests does not match. Wondering multi threading issues.

did you try what I suggested above to try and fix that.


0
 
CEHJCommented:
>>3. XOR implementation

This, although (and because) it's a naive implementation, *should* be the fastest of all
0

Featured Post

Concerto Cloud for Software Providers & ISVs

Can Concerto Cloud Services help you focus on evolving your application offerings, while delivering the best cloud experience to your customers? From DevOps to revenue models and customer support, the answer is yes!

Learn how Concerto can help you.

  • 4
  • 2
  • 2
  • +1
Tackle projects and never again get stuck behind a technical roadblock.
Join Now