Skip to content

Commit

Permalink
fixes #40 fixes #45 prevent login replays
Browse files Browse the repository at this point in the history 
The strings passed to prepareForLogin and authenticate are now modified, so the state can be saved back to the user database. Once authenticated, it can't be used again. It doesn't actually work as a timeout, but a login attempt invalidates previous login attempts, and a valid authentication also invalidates re-use of that login.
  • Loading branch information
@davidearl
davidearl committed Dec 8, 2022
1 parent 8be044c commit c697f9b
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 15 deletions.
12 changes: 11 additions & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ enough to put into a convenient JSON form to transport to the server),
and I thought I would share it. Several others have since helped with support for broader
application with fingerprints and Windows Hello.

## Changes 8 Dec 2022

To prevent a possible replay of a login, the user's webauthn string is
now passed by reference to prepareForLogin and authenticate, both of
which modify it. The string should now be saved with the user in your
user after these calls, as well as when registering. This saves and
then cross-checks and clears the challenge data sent to the key, so
that it cannot be re-used.

## Changes from branch 0.1.0

The original code was updated in August 2019 by a number of
Expand All @@ -63,7 +72,8 @@ This requires
* [phpseclib](https://github.com/phpseclib/phpseclib), ditto
* A recent openssl included in PHP ([openssl_verify](http://php.net/manual/en/function.openssl-verify.php)
in particular)
* PHP 5.6 or later (preferably PHP 7)
* PHP 5.6 or later (preferably PHP 7.4 or 8.1; not tested with PHP 8.2, which has some significant changes
to property declarations)

## Example

Expand Down
39 changes: 31 additions & 8 deletions WebAuthn/WebAuthn.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,22 +234,31 @@ public function register($info, $userwebauthn)

/**
* generates a new key string for the physical key, fingerprint
* reader or whatever to respond to on login
* @param string $userwebauthn the existing webauthn field for the user from your database
* reader or whatever to respond to on login.
* You should store the revised userwebauthn back to your database after calling this function
* (to avoid replay attacks)
* @param string &$userwebauthn the existing webauthn field for the user from your database
* @return string to pass to javascript webauthnAuthenticate
*/
public function prepareForLogin($userwebauthn)
public function prepareForLogin(&$userwebauthn)
{
$allow = (object)array();
$allow->type = 'public-key';
$allow->transports = array('usb','nfc','ble','internal');
$allow->id = null;
$allows = array();

$challengebytes = self::randomBytes(16);
$challengeb64 = rtrim(strtr(base64_encode($challengebytes), '+/', '-_'), '=');

if (! empty($userwebauthn)) {
foreach (json_decode($userwebauthn) as $key) {
$webauthn = json_decode($userwebauthn);
foreach ($webauthn as $idx => $key) {
$allow->id = $key->id;
$allows[] = clone $allow;
$webauthn[$idx]->challenge = $challengeb64;
}
$userwebauthn = json_encode($webauthn);
} else {
/* including empty user, so they can't tell whether the user exists or not (need same result each
time for each user) */
Expand All @@ -262,7 +271,7 @@ public function prepareForLogin($userwebauthn)

/* generate key request */
$publickey = (object)array();
$publickey->challenge = self::stringToArray(self::randomBytes(16));
$publickey->challenge = self::stringToArray($challengebytes);
$publickey->timeout = 60000;
$publickey->allowCredentials = $allows;
$publickey->userVerification = 'discouraged';
Expand All @@ -275,15 +284,17 @@ public function prepareForLogin($userwebauthn)

/**
* validates a response for login or 2fa
* requires info from the hardware via javascript given below
* requires info from the hardware via javascript given below.
* You should store the revised userwebauthn back to your database after calling this function
* (to avoid replay attacks)
* @param string $info supplied to the PHP script via a POST, constructed by the Javascript given below, ultimately
* provided by the key
* @param string $userwebauthn the exisiting webauthn field for the user from your
* @param string &$userwebauthn the exisiting webauthn field for the user from your
* database (it's actaully a JSON string, but that's entirely internal to
* this code)
* @return boolean true for valid authentication or false for failed validation
*/
public function authenticate($info, $userwebauthn)
public function authenticate($info, &$userwebauthn)
{
if (! is_string($info)) {
$this->oops('info must be a string', 1);
Expand Down Expand Up @@ -315,6 +326,18 @@ public function authenticate($info, $userwebauthn)
$this->oops("challenge mismatch");
}

/* Does the challenge Correspond to the one we stored for the user? If no challenge is stored, that
implies $userwebauthn was not saved back to the user database after prepareForLogin. It
would be better to produce an error here, but for the purposes of backward compatibility, it'll
allow it, but with a replay vulnerability */
//log(print_r($key->challenge,1).' '.print_r($info->response->clientData->challenge,1));
if (isset($key->challenge) && $key->challenge != $info->response->clientData->challenge) {
$this->oops("you cannot use the same login more than once");
}
/* clear the challenge (but retain it as a property) from each of the keys, so it cannot be re-used */
foreach($webauthn as $idx => $candkey) { $webauthn[$idx]->challenge = ''; }
$userwebauthn = json_encode($webauthn);

/* cross check origin */
$origin = parse_url($info->response->clientData->origin);
if ($this->appid != $origin['host']) {
Expand Down
22 changes: 16 additions & 6 deletions example/index.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ function getuser($username){
return $user;
}

function saveuser($user){
file_put_contents(userpath($user->name), json_encode($user));
}

/* A post is an ajax request, otherwise display the page */
if (! empty($_POST)) {

Expand All @@ -52,7 +56,7 @@ function getuser($username){
case isset($_POST['registerusername']):
/* initiate the registration */
$username = $_POST['registerusername'];
$crossplatform = ! empty($_POST['crossplatform']) && $_POST['crossplatform'] == 'Yes';
$crossplatform = ! empty($_POST['crossplatform']) && $_POST['crossplatform'] == 'Yes';
$userid = md5(time() . '-'. rand(1,1000000000));

if (file_exists(userpath($username))) {
Expand All @@ -64,11 +68,12 @@ function getuser($username){
but you'd probably do that from a user profile page rather than initial
registration. The procedure is the same, just don't cancel existing
keys like this.*/
file_put_contents(userpath($username), json_encode(['name'=> $username,
'id'=> $userid,
'webauthnkeys' => $webauthn->cancel()]));
$user = (object)['name'=> $username,
'id'=> $userid,
'webauthnkeys' => $webauthn->cancel()];
saveuser($user);
$_SESSION['username'] = $username;
$j = ['challenge' => $webauthn->prepareChallengeForRegistration($username, $userid, $crossplatform)];
$j = ['challenge' => $webauthn->prepareChallengeForRegistration($username, $userid, $crossplatform)];
break;

case isset($_POST['register']):
Expand All @@ -81,7 +86,7 @@ function getuser($username){

/* Save the result to enable a challenge to be raised agains this
newly created key in order to log in */
file_put_contents(userpath($user->name), json_encode($user));
saveuser($user);
$j = 'ok';

break;
Expand All @@ -97,6 +102,9 @@ function getuser($username){
people to interrogate your user database for existence */

$j['challenge'] = $webauthn->prepareForLogin($user->webauthnkeys);

/* Save user again, which sets server state to include the challenge expected */
saveuser($user);
break;

case isset($_POST['login']):
Expand All @@ -109,6 +117,8 @@ function getuser($username){
echo 'failed to authenticate with that key';
exit;
}
/* Save user again, which sets server state to include the challenge expected */
saveuser($user);
$j = 'ok';

break;
Expand Down

0 comments on commit c697f9b

Please sign in to comment.