Is this the correct way to implement a HMAC token system?

Question

I'm architecting a basic SSO solution to pass a user between two sites.

Site A handles authentication against a user database and then generates links to Site B which pass a HMAC token comprising a nonce and the user's ID. The HMAC is hashed with a shared secret between the two sites.

The following shows the token generation in PHP.

$sso['nonce'] = hash('sha1', rand() );
$sso['sharedKey'] = 'someVerySecureString';
$sso['hmacKey'] = hash_hmac ( 'sha256', $test['sharedKey'] , $test['nonce'] );
$sso['userId'] = '123456';
$sso['hmac'] = hash_hmac ( 'sha256', $test['userId'] , $test['hmacKey'] );
$sso['url'] = '/?id=' . urlencode($test['userId']) . '&nonce=' . urlencode($test['nonce']) . '&hmac=' . urlencode($test['hmac']);

I'm using the nonce to HMAC the key before transmission.

This is the PHP function on Site B to validate the token:

function validateSSOToken($sharedKey='someVerySecureString') {
 $isValid = false;
 if ( isset( $_GET['id'] ) 
 && isset( $_GET['nonce'] ) 
 && isset( $_GET['hmac'] ) 
 && strlen( $_GET['nonce'] ) == 40 
 && strlen( $_GET['hmac'] ) == 64) {
 $userId = $_GET['id'];
 $nonce = $_GET['nonce'];
 $hmac = $_GET['hmac'];
 // Sign the key with the nonce to get the tmp key
 $hmacKey = hash_hmac ( 'sha256', $sharedKey, $nonce );
 // rebuild the string to sign
 $localHmac = hash_hmac ( 'sha256', $userId , $hmacKey );
 // Compare against the incoming key
 $isValid = $localHmac == $hmac ? true : false;
 } 
 return $isValid;
}

I'm aware of the replay attack vector and will be addressing it in due course.

Could you advise if this is secure? Is there anything else I might look at considering to achieve the same goal?

• mseancole: You are not likely to get too many answers to this as a number of these security questions seem to go unanswered. If after a few days you have not received any helpful advice, I would suggest posting this again on SO with an explanation that no answers were forthcoming here. They may move it here again, but it should still be visible from SO and you might get better answers that way. That being said, you can combine that isset() check. isset( $_GET[ 'id' ], $_GET[ 'nonce' ], $_GET[ 'hmac' ] ).
• Rob Dudley: Thanks for the honest apprisal. First time using CR so I hope I get some feedback. And thanks for the tip on isset(). I've been developing in PHP for almost 10 years now and never realised you could validate multiple vars that way. Guess that just goes to show that it's always worth reading the manual even for functions you think you know!
• Pinoniq: So does the user exist on both sites? or do those sites share the 'user database'?

Answer 1

This looks good.

I do almost exactly the same thing as a password reset mechanism. I am including it here in case it will help you with adding a timer to help prevent replay attacks.

The user receives an email with a link to click to reset their password (the $oldpassword is the nonce because once the password is changed once, it is not valid anymore and the HMAC key is the old password salt (used by the php crypt function)).

$time = time();
$userid = $user->id;
$oldpassword = hash('sha256', $password_info['Password']);
$token = "t={$time}&i={$userid}&o={$oldpassword}";
$verification = hash_hmac('sha256', $token, $password_info['Salt']);
$newpasswordurl = Router::url('user_r')->uri(array(
 't' => $time,
 'i' => $userid,
 'o' => $oldpassword,
 'v' => $verification,
));

The validation is similar.

$userid = $input['i'];
$time = $input['t'];
$expire_time = date_create_from_format('U', $input['t'])->add(date_interval_create_from_date_string(self::$settings['ResetPassword']['LinkValidityTime']));
$oldpassword = $input['o'];
$hash = $input['v'];
$p_info = self::getPasswordParts($OldPassword);
$token = "t=$time&i=$UserID&o=$oldpassword";
$correct_hash = hash_hmac('sha256', $token, $p_info['Salt']);
if ( ($hash != $correct_hash) || ($oldpassword != hash('sha256', $p_info['Password'])) ) {
 throw new ChangePasswordException("That password link is invalid, has expired or has been used.");
}