Update dependencies, cleanup (#12)

.gitignore

@@ -3,3 +3,4 @@
 /vendor/
 # Suggested for libraries
 composer.lock
+.phpunit.result.cache

.travis.yml

@@ -1,17 +1,17 @@
 language: php
 php:
- - '7.0'
- - '7.1'
  - '7.2'
+ - '7.3'
+ - '7.4snapshot'
 
 # From PHPUnit's config
 install:
  - travis_retry composer install --no-interaction --prefer-source
 
 script:
- - php -d mbstring.func_overload=7 vendor/bin/phpunit --coverage-text --whitelist src/ tests/
+ - php vendor/bin/phpunit --coverage-text --whitelist src/ tests/
  - vendor/bin/phpcs src tests
- - vendor/bin/phpstan analyse --no-progress -l7 src tests
+ - vendor/bin/phpstan analyse .
 
 after_success:
  - travis_retry php vendor/bin/php-coveralls

README.md

@@ -23,6 +23,13 @@ Additional resources:
 * [FIDO U2F Overview](https://fidoalliance.org/specs/fido-u2f-v1.0-nfc-bt-amendment-20150514/fido-u2f-overview.html)
 * [FIDO U2F Javascript API](https://fidoalliance.org/specs/fido-u2f-v1.0-nfc-bt-amendment-20150514/fido-u2f-javascript-api.html)
 
+## Installation
+
+`composer require firehed/u2f`
+
+Note: you **must not** be using the deprecated `mbstring.func_overload` functionality, which can completely break working on binary data.
+The library will immediately throw an exception if you have it enabled.
+
 ## Usage
 
 Usage will be described in three parts: setup, registration, and authentication.

composer.json

@@ -13,13 +13,14 @@
  ],
  "homepage": "https://github.com/Firehed/u2f-php",
  "require": {
- "php": ">=7.0"
+ "php": ">=7.2"
  },
  "require-dev": {
  "php-coveralls/php-coveralls": "^2.0",
- "phpstan/phpstan": "^0.9.2",
- "phpunit/phpunit": "^6.0 || ^7.0",
- "squizlabs/php_codesniffer": "^3.2"
+ "phpstan/phpstan": "^0.11",
+ "phpunit/phpunit": "^8.0",
+ "squizlabs/php_codesniffer": "^3.2",
+ "spatie/phpunit-watcher": "^1.8"
  },
  "autoload": {
  "psr-4": {
@@ -39,5 +40,17 @@
  "name": "Eric Stern",
  "email": "[email protected]"
  }
- ]
+ ],
+ "scripts": {
+ "test": [
+ "@phpunit",
+ "@phpstan",
+ "@phpcs"
+ ],
+ "coverage": "phpunit --coverage-html build; open build/index.html",
+ "autofix": "phpcbf src lib tests db",
+ "phpunit": "phpunit",
+ "phpstan": "phpstan analyse --no-progress .",
+ "phpcs": "phpcs ."
+ }
 }

phpstan.neon

@@ -0,0 +1,6 @@
+parameters:
+ excludes_analyse:
+ - %rootDir%/../../../vendor
+ level: 7
+ includes:
+ - vendor/phpstan/phpstan/conf/bleedingEdge.neon

src/AppIdTrait.php

@@ -19,7 +19,7 @@ public function setAppId(string $appId): self
  }
 
  /**
- * @return the raw SHA-256 hash of the App ID
+ * @return string The raw SHA-256 hash of the App ID
  */
  public function getApplicationParameter(): string
  {

src/AttestationCertificateTrait.php

@@ -5,21 +5,21 @@
 
 trait AttestationCertificateTrait
 {
-
- // Stored base64-encoded
+ /** @var string (binary) */
  private $attest = '';
 
  // Binary string of attestation certificate (from device issuer)
  public function getAttestationCertificateBinary(): string
  {
- return base64_decode($this->attest);
+ return $this->attest;
  }
 
  // PEM formatted cert
  public function getAttestationCertificatePem(): string
  {
+ $data = base64_encode($this->getAttestationCertificateBinary());
  $pem = "-----BEGIN CERTIFICATE-----\r\n";
- $pem .= chunk_split($this->attest, 64);
+ $pem .= chunk_split($data, 64);
  $pem .= "-----END CERTIFICATE-----";
  return $pem;
  }
@@ -28,7 +28,7 @@ public function setAttestationCertificate(string $cert): self
  {
  // In the future, this may make assertions about the cert formatting;
  // right now, we're going to leave it be.
- $this->attest = base64_encode($cert);
+ $this->attest = $cert;
  return $this;
  }
 

src/ClientData.php

@@ -68,6 +68,7 @@ private function validateKey(string $key, array $data)
  public function getChallengeParameter(): string
  {
  $json = json_encode($this, \JSON_UNESCAPED_SLASHES);
+ assert($json !== false);
  return hash('sha256', $json, true);
  }
 

src/ECPublicKeyTrait.php

@@ -7,7 +7,7 @@
 
 trait ECPublicKeyTrait
 {
- // Stored base64-encoded
+ /** @var string (binary) */
  private $pubKey = '';
 
  /**
@@ -18,15 +18,15 @@ public function getPublicKey(): string
  {
  trigger_error('Please use getPublicKeyBinary().', E_USER_DEPRECATED);
 
- return base64_decode($this->pubKey);
+ return $this->getPublicKeyBinary();
  }
 
  /**
  * @return string The decoded public key.
  */
  public function getPublicKeyBinary(): string
  {
- return base64_decode($this->pubKey);
+ return $this->pubKey;
  }
 
  // Prepends the pubkey format headers and builds a pem file from the raw
@@ -67,7 +67,7 @@ public function setPublicKey(string $key): self
  if (strlen($key) !== 65) {
  throw new IDE(IDE::PUBLIC_KEY_LENGTH, '65');
  }
- $this->pubKey = base64_encode($key);
+ $this->pubKey = $key;
  return $this;
  }
 }

src/KeyHandleTrait.php

@@ -4,12 +4,13 @@
 
 trait KeyHandleTrait
 {
+ /** @var string (binary) */
  private $keyHandle;
 
  // Binary string of key handle
  public function getKeyHandleBinary(): string
  {
- return base64_decode($this->keyHandle);
+ return $this->keyHandle;
  }
  // B64-websafe value
  public function getKeyHandleWeb(): string
@@ -20,7 +21,7 @@ public function getKeyHandleWeb(): string
  public function setKeyHandle(string $keyHandle): self
  {
  // TODO: make immutable
- $this->keyHandle = base64_encode($keyHandle);
+ $this->keyHandle = $keyHandle;
  return $this;
  }
 }

src/ResponseTrait.php

@@ -7,17 +7,16 @@
 
 trait ResponseTrait
 {
-
  use KeyHandleTrait;
 
  private $clientData;
- // Stored base64-encoded
+
+ /** @var string (binary) */
  private $signature = '';
 
- // Binary string of signature
  public function getSignature(): string
  {
- return base64_decode($this->signature);
+ return $this->signature;
  }
 
  public function getClientData(): ClientData
@@ -27,7 +26,7 @@ public function getClientData(): ClientData
 
  protected function setSignature(string $signature): self
  {
- $this->signature = base64_encode($signature);
+ $this->signature = $signature;
  return $this;
  }
 

src/Server.php

@@ -5,6 +5,7 @@
 
 use BadMethodCallException;
 use Firehed\U2F\SecurityException as SE;
+use RuntimeException;
 
 class Server
 {
@@ -48,6 +49,17 @@ class Server
  */
  private $signRequests = [];
 
+ public function __construct()
+ {
+ $overload = ini_get('mbstring.func_overload');
+ // @codeCoverageIgnoreStart
+ if ($overload > 0) {
+ throw new RuntimeException(
+ 'The deprecated "mbstring.func_overload" directive must be disabled'
+ );
+ }
+ // @codeCoverageIgnoreEnd
+ }
  /**
  * This method authenticates a `SignResponse` against outstanding
  * registrations and their corresponding `SignRequest`s. If the response's

src/functions.php

@@ -9,7 +9,9 @@
  */
 function fromBase64Web(string $base64): string
 {
- return base64_decode(strtr($base64, '-_', '+/'));
+ $decoded = base64_decode(strtr($base64, '-_', '+/'));
+ assert($decoded !== false);
+ return $decoded;
 }
 
 /**
@@ -21,41 +23,3 @@ function toBase64Web(string $binary): string
 {
  return rtrim(strtr(base64_encode($binary), '+/', '-_'), '=');
 }
-
-// Multibyte string wrappers: hijack calls to `strlen` and `substr` to force
-// 8-bit encoding in the event that `mbstring.func_overload` parameter is
-// non-zero and the mbstring default charset is not 8bit.
-
-/**
- * Identical to `\strlen` except when `mbstring.func_overload` is enabled and
- * set to a multi-byte character set, in which case it retains the
- * non-overloaded behavior.
- *
- * @param string $string The string being measured
- * @return int The length of the string, in bytes
- */
-function strlen(string $string): int
-{
- if (function_exists('mb_strlen')) {
- return \mb_strlen($string, '8bit');
- }
- return \strlen($string);
-}
-
-/**
- * Identical to `\substr` except when `mbstring.func_overload` is enabled and
- * set to a multi-byte character set, in which case it retains the
- * non-overloaded behavior.
- *
- * @param string $string The input string
- * @param int $start The starting point, in bytes
- * @param int $length The length, in bytes
- * @return string The extracted part of the string
- */
-function substr(string $string, int $start, int $length = null): string
-{
- if (function_exists('mb_substr')) {
- return \mb_substr($string, $start, $length, '8bit');
- }
- return \substr($string, $start, $length);
-}

tests/AttestationCertificateTraitTest.php

@@ -96,9 +96,9 @@ public function testFailedCAVerificationFromNoCAs()
  */
  private function getObjectWithYubicoCert()
  {
- $response = RegisterResponse::fromJson(
-  file_get_contents(__DIR__.'/register_response.json')
- );
+ $data = file_get_contents(__DIR__.'/register_response.json');
+ assert($data !== false);
+ $response = RegisterResponse::fromJson($data);
  // Sanity check that the response actually imlements this trait, rather
  // than doing all sorts of magic
  $check = AttestationCertificateTrait::class;

tests/ClientDataTest.php

@@ -23,6 +23,7 @@ public function testFromValidJson()
  'cid_pubkey' => '',
  ];
  $goodJson = json_encode($goodData);
+ assert($goodJson !== false);
  $clientData = ClientData::fromJson($goodJson);
  $this->assertInstanceOf(ClientData::class, $clientData);
  }
@@ -34,6 +35,7 @@ public function testFromValidJson()
  public function testGetChallengeParameter()
  {
  $expected_param = base64_decode('exDPjyyKbizXMAAUNLpv0QYJNyXClbUqewUWojPtp0g=');
+ assert($expected_param !== false);
  // Sanity check
  $this->assertSame(
  32,
@@ -48,6 +50,7 @@ public function testGetChallengeParameter()
  'cid_pubkey' => '',
  ];
  $goodJson = json_encode($goodData);
+ assert($goodJson !== false);
  $clientData = ClientData::fromJson($goodJson);
  $this->assertTrue(
  hash_equals($expected_param, $clientData->getChallengeParameter()),
@@ -89,6 +92,7 @@ public function testTypes(string $type, bool $allowed)
  'cid_pubkey' => '',
  ];
  $json = json_encode($all);
+ assert($json !== false);
  if (!$allowed) {
  $this->expectException(InvalidDataException::class);
  $this->expectExceptionCode(InvalidDataException::MALFORMED_DATA);

tests/ECPublicKeyTraitTest.php

@@ -13,7 +13,7 @@ class ECPublicKeyTraitTest extends \PHPUnit\Framework\TestCase
 
  /**
  * @covers ::setPublicKey
- * @covers ::getPublicKey
+ * @covers ::getPublicKeyBinary
  */
  public function testAccessors()
  {
@@ -46,6 +46,7 @@ public function testGetPublicKeyPem()
  '04b4960ae0fa301033fbedc85c33ac30408dffd6098bc8580d8b66159959d89b9'.
  '31daf1d43a1949b07b7d47eea25efcac478bb5cd6ead0a3c3f7b7cb2a7bc1e3be'
  );
+ assert($key !== false);
  $pem =
  "-----BEGIN PUBLIC KEY-----\r\n".
  "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEtJYK4PowEDP77chcM6wwQI3/1gmL\r\n".