Skip to content

Commit

Permalink
Add permitted algorithms to validation context
Browse files Browse the repository at this point in the history
  • Loading branch information
sop committed Sep 7, 2022
1 parent 6b69a8c commit 3725069
Show file tree
Hide file tree
Showing 5 changed files with 199 additions and 2 deletions.
6 changes: 5 additions & 1 deletion examples/jwe-consume.php
Expand Up @@ -9,6 +9,7 @@
declare(strict_types = 1);

use Sop\CryptoEncoding\PEM;
use Sop\JWX\JWA\JWA;
use Sop\JWX\JWK\RSA\RSAPrivateKeyJWK;
use Sop\JWX\JWT\JWT;
use Sop\JWX\JWT\ValidationContext;
Expand All @@ -21,7 +22,10 @@
$jwk = RSAPrivateKeyJWK::fromPEM(
PEM::fromFile(dirname(__DIR__) . '/test/assets/rsa/private_key.pem'));
// create validation context containing only key for decryption
$ctx = ValidationContext::fromJWK($jwk);
$ctx = ValidationContext::fromJWK($jwk)
// NOTE: asymmetric key derivation algorithms are not enabled by default
// due to sign/encrypt confusion vulnerability!
->withPermittedAlgorithmsAdded(JWA::ALGO_RSA1_5);
// decrypt claims from the encrypted JWT
$claims = $jwt->claims($ctx);
// print all claims
Expand Down
42 changes: 41 additions & 1 deletion lib/JWX/JWT/JWT.php
Expand Up @@ -16,6 +16,7 @@
use Sop\JWX\JWT\Header\Header;
use Sop\JWX\JWT\Header\JOSE;
use Sop\JWX\JWT\Parameter\ContentTypeParameter;
use Sop\JWX\Parameter\Parameter;
use Sop\JWX\Util\Base64;

/**
Expand Down Expand Up @@ -161,7 +162,9 @@ public static function encryptedFromClaims(Claims $claims,
*/
public function claims(ValidationContext $ctx): Claims
{
// check signature or decrypt depending on the JWT type.
// check that the token uses only permitted algorithms
$this->_validateAlgorithms($ctx);
// check signature or decrypt depending on the JWT type
if ($this->isJWS()) {
$payload = self::_validatedPayloadFromJWS($this->JWS(), $ctx);
} else {
Expand Down Expand Up @@ -338,6 +341,43 @@ private function _claimsFromNestedPayload(string $payload,
return $jwt->claims($ctx);
}

/**
* Validate that the token uses only permitted algorithms.
*
* @param ValidationContext $ctx Validation context
*/
private function _validateAlgorithms(ValidationContext $ctx): void
{
$headers = $this->header();
if ($headers->hasAlgorithm()) {
$this->_validateAlgorithmParameter($headers->algorithm(), $ctx);
}
if ($headers->hasEncryptionAlgorithm()) {
$this->_validateAlgorithmParameter($headers->encryptionAlgorithm(), $ctx);
}
if ($headers->hasCompressionAlgorithm()) {
$this->_validateAlgorithmParameter($headers->compressionAlgorithm(), $ctx);
}
}

/**
* Check that given algorithm parameter value is permitted.
*
* @param Parameter $param Header parameter
* @param ValidationContext $ctx Validation context
*
* @throws ValidationException If algorithm is prohibited
*/
private function _validateAlgorithmParameter(Parameter $param,
ValidationContext $ctx): void
{
if (!$ctx->isPermittedAlgorithm($param->value())) {
throw new ValidationException(sprintf(
'%s algorithm %s is not permitted.',
$param->name(), $param->value()));
}
}

/**
* Get validated payload from JWS.
*
Expand Down
108 changes: 108 additions & 0 deletions lib/JWX/JWT/ValidationContext.php
Expand Up @@ -4,6 +4,7 @@

namespace Sop\JWX\JWT;

use Sop\JWX\JWA\JWA;
use Sop\JWX\JWK\JWK;
use Sop\JWX\JWK\JWKSet;
use Sop\JWX\JWT\Claim\RegisteredClaim;
Expand Down Expand Up @@ -72,6 +73,54 @@ class ValidationContext
*/
protected $_allowUnsecured;

/**
* List of permitted algorithms.
*
* By default only asymmetric key derivation algorithms are prohibited.
*
* @var string[]
*/
protected $_permittedAlgoritms = [
// all signature algorithms are safe to use
JWA::ALGO_HS256 => true,
JWA::ALGO_HS384 => true,
JWA::ALGO_HS512 => true,
JWA::ALGO_RS256 => true,
JWA::ALGO_RS384 => true,
JWA::ALGO_RS512 => true,
JWA::ALGO_ES256 => true,
JWA::ALGO_ES384 => true,
JWA::ALGO_ES512 => true,
// unsecured JWS may be used when explicitly allowed
JWA::ALGO_NONE => true,
// all symmetric key derivation algorithms are safe to use
JWA::ALGO_A128KW => true,
JWA::ALGO_A192KW => true,
JWA::ALGO_A256KW => true,
JWA::ALGO_A128GCMKW => true,
JWA::ALGO_A192GCMKW => true,
JWA::ALGO_A256GCMKW => true,
JWA::ALGO_PBES2_HS256_A128KW => true,
JWA::ALGO_PBES2_HS384_A192KW => true,
JWA::ALGO_PBES2_HS512_A256KW => true,
JWA::ALGO_DIR => true,
/* asymmetric key derivation algorithms are subject to
"sign/encrypt confusion" vulnerability, in which the JWT consumer
can be tricked to accept a JWE token instead of a JWS by using
the public key for encryption. */
JWA::ALGO_RSA1_5 => false,
JWA::ALGO_RSA_OAEP => false,
// all encryption algorithms are safe to use
JWA::ALGO_A128CBC_HS256 => true,
JWA::ALGO_A192CBC_HS384 => true,
JWA::ALGO_A256CBC_HS512 => true,
JWA::ALGO_A128GCM => true,
JWA::ALGO_A192GCM => true,
JWA::ALGO_A256GCM => true,
// all compression algorithms are safe to use
JWA::ALGO_DEFLATE => true,
];

/**
* Constructor.
*
Expand Down Expand Up @@ -296,6 +345,65 @@ public function isUnsecuredAllowed(): bool
return $this->_allowUnsecured;
}

/**
* Get self with only given permitted algorithms.
*
* @param string ...$names Algorithm name
*
* @see \Sop\JWX\JWA\JWA
*/
public function withPermittedAlgorithms(string ...$names): self
{
$obj = clone $this;
$obj->_permittedAlgoritms = [];
return $obj->withPermittedAlgorithmsAdded(...$names);
}

/**
* Get self with given algorithms added to the permitted set.
*
* @param string ...$names Algorithm name
*
* @see \Sop\JWX\JWA\JWA
*/
public function withPermittedAlgorithmsAdded(string ...$names): self
{
$obj = clone $this;
foreach ($names as $name) {
$obj->_permittedAlgoritms[$name] = true;
}
return $obj;
}

/**
* Get self with given algorithms removed from the permitted set.
*
* @param string ...$names Algorithm name
*
* @see \Sop\JWX\JWA\JWA
*/
public function withProhibitedAlgorithms(string ...$names): self
{
$obj = clone $this;
foreach ($names as $name) {
$obj->_permittedAlgoritms[$name] = false;
}
return $obj;
}

/**
* Check whether given algorithm is permitted.
*
* @param string $name Algorithm name
*
* @see \Sop\JWX\JWA\JWA
*/
public function isPermittedAlgorithm(string $name): bool
{
return isset($this->_permittedAlgoritms[$name])
&& true === $this->_permittedAlgoritms[$name];
}

/**
* Validate claims.
*
Expand Down
11 changes: 11 additions & 0 deletions test/unit/jwt/JWTTest.php
Expand Up @@ -293,6 +293,17 @@ public function testInvalidJWT()
new JWT('');
}

public function testProhibitedAlgorithm()
{
$jwt = JWT::unsecuredFromClaims(
self::$_claims,
new Header(new JWTParameter(JWTParameter::P_ZIP, 'dummy'))
);
$this->expectException(ValidationException::class);
$this->expectExceptionMessage('zip algorithm dummy is not permitted');
$jwt->claims(new ValidationContext());
}

/**
* @depends testCreateJWS
*/
Expand Down
34 changes: 34 additions & 0 deletions test/unit/jwt/ValidationContextTest.php
Expand Up @@ -3,6 +3,7 @@
declare(strict_types = 1);

use PHPUnit\Framework\TestCase;
use Sop\JWX\JWA\JWA;
use Sop\JWX\JWT\Claim\IssuerClaim;
use Sop\JWX\JWT\Claim\RegisteredClaim;
use Sop\JWX\JWT\Claims;
Expand Down Expand Up @@ -157,4 +158,37 @@ public function testValidateRequiredFail(ValidationContext $ctx)
$this->expectExceptionMessage('failed');
$ctx->validate($claims);
}

/**
* @depends testCreate
*/
public function testAddPermittedAlgorithm(ValidationContext $ctx)
{
$this->assertTrue($ctx->isPermittedAlgorithm(JWA::ALGO_RS256));
$this->assertFalse($ctx->isPermittedAlgorithm('test'));
$ctx = $ctx->withPermittedAlgorithmsAdded('test');
$this->assertTrue($ctx->isPermittedAlgorithm(JWA::ALGO_RS256));
$this->assertTrue($ctx->isPermittedAlgorithm('test'));
}

/**
* @depends testCreate
*/
public function testNewPermittedAlgorithm(ValidationContext $ctx)
{
$this->assertTrue($ctx->isPermittedAlgorithm(JWA::ALGO_RS256));
$ctx = $ctx->withPermittedAlgorithms('test');
$this->assertFalse($ctx->isPermittedAlgorithm(JWA::ALGO_RS256));
$this->assertTrue($ctx->isPermittedAlgorithm('test'));
}

/**
* @depends testCreate
*/
public function testProhibitedAlgorithm(ValidationContext $ctx)
{
$this->assertTrue($ctx->isPermittedAlgorithm(JWA::ALGO_RS256));
$ctx = $ctx->withProhibitedAlgorithms(JWA::ALGO_RS256);
$this->assertFalse($ctx->isPermittedAlgorithm(JWA::ALGO_RS256));
}
}

0 comments on commit 3725069

Please sign in to comment.