PASETO authenticator and identifier #538

cnizzardini · 2022-05-27T22:13:50Z

This is a rough draft implementation of PASETO (Platform Agnostic Security Tokens). This supports local symmetric keys and public asymmetric keys, but does not yet have support for keyrings and PASERK which gives functionality similar to JWKS. If there is interest in this I can add support for those items as well as unit tests and docs.

Usage:

Usage is fairly similar to the JWT Authenticator with the addition of a few keys:

        $service->loadIdentifier('Authentication.PasetoSubject');
        $service->loadAuthenticator('Authentication.Paseto', [
            'purpose' => PasetoAuthenticator::LOCAL, // or PasetoAuthenticator::PUBLIC
        ]);

There is also a version which takes an implementation of ProtocolInterface but defaults to the latest version in paseto (currently v4).

Using the public option is a little PITA to generate keys and I'd like to get a shell command added to the main paseto lib to easily generate the public/private key pairs.

Building a local token:

        /** @var ResultInterface $result **/
        $key = SymmetricKey::v4(Security::getSalt());

        $token = (new Builder())
            ->setKey($key)
            ->setSubject($result->getData()->get('id'))
            ->setVersion(new Version4)
            ->setPurpose(Purpose::local())
            // Set it to expire in one day
            ->setIssuedAt()
            ->setNotBefore()
            ->setExpiration(
                (new DateTime())->add(new DateInterval('P01D'))
            )
            // Store arbitrary data
            ->setClaims([
                'claim_data' => 'is encrypted',
            ])
            ->setFooterArray([
                'footer_data' => 'is unencrypted but tamper proof'
            ]);

Reference:

markstory · 2022-05-30T03:17:01Z

src/Authenticator/PasetoAuthenticator.php

+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ * @link          https://cakephp.org CakePHP(tm) Project
+ * @since         1.0.0


You'll likely need 3.0.0 here.

markstory · 2022-05-30T03:17:50Z

src/Authenticator/PasetoAuthenticator.php

+            if (!class_exists(\Cake\Utility\Security::class)) {
+                throw new RuntimeException('PASETO `secretKey` config must be defined');
+            }
+            $this->setConfig('secretKey', \Cake\Utility\Security::getSalt());


Will the application salt always be long enough? Are there length requirements in paseto?

For Symmetric keys between 16 and 64 bytes. sodium_crypto_generichash will throw a SodiumException: unsupported key length exception otherwise, unfortunately this is not well documented here: sodium_crypto_generichash. We could add a validation for this in the authenticator.

For Asymmetric keys it is recommended to use sodium_crypto_sign_keypair to generate the key pairs:

$privateKey = new AsymmetricSecretKey(sodium_crypto_sign_keypair()); $publicKey = $privateKey->getPublicKey(); var_dump($privateKey->encode()); var_dump($publicKey->encode());

This is where I wish the library provided a cli to easily generate those for developers.

This is where I wish the library provided a cli to easily generate those for developers.

We could provide that CLI tool 😄

For Symmetric keys between 16 and 64 bytes. sodium_crypto_generichash will throw a SodiumException: unsupported key length exception otherwise, unfortunately this is not well documented here

That will work. If users get tripped up by the exceptions from libsodium we can always catch errors and rethrow with a better message.

markstory · 2022-05-30T03:18:28Z

src/Authenticator/PasetoAuthenticator.php

+        if ($this->getConfig('returnPayload')) {
+            $array = array_merge(
+                $result->getClaims(),
+                ['footer' => $result->getFooterArray()]


What is the footer used for by application logic?

A token is split into version, purpose, encrypted data, and unencrypted data with dots as the separator.

v4.local.KQVqiaVJbg_V0W9s-ekQMYyjA5sZCSMUFw6WxLJ_6806uE7ceNad9ABgGwgJ_HDN8zLDC_SF9hzFoESX8lt-v_o1LjsahdoOETFlp884ztE-vz5MZq6CfvaVhH4cjT8Wi6p43H95jegzM3BhbjrJpAiUOWISPxbR5Da90WSkCUVSQfMjCzSrTjEQRdTlbBNpjzEOen6f8Vaw-Wl2eqtmzgVDZ5LDcyR9fatO_p_IhH8cFwLXILoORpjFBgeCDXZqr9jaum6zEvAs3XSfWB86RLUM_QD-elM_2DkaCwqS_1btNYKpahRkLpr6ehM8dSKL.eyJmb290ZXJfZGF0YSI6ImlzIHVuZW5jcnlwdGVkIGJ1dCB0YW1wZXIgcHJvb2YifQ

So you can base64 decode the footer segment: eyJmb290ZXJfZGF0YSI6ImlzIHVuZW5jcnlwdGVkIGJ1dCB0YW1wZXIgcHJvb2YifQ to:

{"footer_data":"is unencrypted but tamper proof"}

I assume to give API clients or any client that doesn't have the secret the ability to read non-sensitive data.

cnizzardini · 2022-06-02T21:30:21Z

src/Authenticator/PasetoAuthenticator.php

+        'version' => null,
+        'purpose' => null,
+        'secretKey' => null,
+        //'subjectKey' => IdentifierInterface::CREDENTIAL_JWT_SUBJECT,


I'm thinking for this we will just remove subjectKey and always use sub.

cnizzardini · 2022-06-02T21:31:19Z

src/Identifier/PasetoSubjectIdentifier.php

+     */
+    protected $_defaultConfig = [
+        'tokenField' => 'id',
+        'dataField' => self::CREDENTIAL_JWT_SUBJECT,


Thinking of hardcoding dataField to "sub"

cnizzardini · 2022-06-03T02:59:46Z

src/Authenticator/PasetoAuthenticator.php

+        }
+
+        if (empty($this->getConfig('secretKey'))) {
+            if (!class_exists(\Cake\Utility\Security::class)) {


Don't know how to test this, thinking this should be marked as code coverage ignore.

cnizzardini · 2022-06-03T03:01:22Z

src/Authenticator/PasetoAuthenticator.php

+     */
+    public function getPayload(?ServerRequestInterface $request = null): ?object
+    {
+        if (!$request) {


Not sure I understand this condition, it was borrowed from JwtAuthenticator. How is this scenario possible?

The behavior looks like it wants to reuse the stored payload property. By passing null in, you get the last parsed result.

cnizzardini · 2022-06-07T01:24:23Z

composer.json

        "cakephp/cakephp-codesniffer": "^4.0",
        "firebase/php-jwt": "^6.2",
+        "paragonie/paseto": "^2.4",
+        "paragonie/constant_time_encoding": "^2.2",


This was required because of prefer-lowest on 7.2. The method needed from the package was not included until 2.2 (current is 2.5).

othercorey · 2022-06-07T01:37:59Z

@cnizzardini After fixing the phpstan error, can you squash the commits?

markstory · 2022-06-12T03:38:00Z

src/Authenticator/PasetoAuthenticator.php

+        if ($this->version === null) {
+            throw new RuntimeException('PASETO `version` must be one of: v3 or v4');
+        }
+
+        if (!in_array($this->getConfig('purpose'), [self::PUBLIC, self::LOCAL])) {
+            throw new RuntimeException('PASETO `purpose` config must one of: local or public');
+        }


Are there no reasonably secure defaults we could use? Even 'most secure' is a reasonable defaults. Right now we're forcing a lot of decisions on the developer.

We can default to local and v4.

markstory · 2022-06-12T03:40:21Z

src/Authenticator/PasetoAuthenticator.php

+     */
+    public function getPayload(?ServerRequestInterface $request = null): ?object
+    {
+        if (!$request) {


The behavior looks like it wants to reuse the stored payload property. By passing null in, you get the last parsed result.

markstory · 2022-06-12T03:41:13Z

src/Authenticator/PasetoAuthenticator.php

+    protected function decodeToken(string $token): ?JsonToken
+    {
+        if ($this->getConfig('purpose') === self::PUBLIC) {
+            $receivingKey = AsymmetricSecretKey::fromEncodedString(


This seems like a reasonable default purpose to me.

I was thinking LOCAL should be the default purpose. Local is similar to the default for JWT which uses an HMAC secret.

markstory · 2022-06-12T03:41:37Z

src/Authenticator/PasetoAuthenticator.php

+        switch ($this->getConfig('version')) {
+            case 'v3':
+                return new Version3();
+            case 'v4':


Defaulting to the highest version seems like a good default value as well.

markstory · 2022-06-12T03:42:40Z

src/Command/PasetoCommand.php

+            ->addArgument('purpose', [
+                'help' => 'The PASETO purpose',
+                'required' => true,
+                'choices' => [PasetoAuthenticator::LOCAL, PasetoAuthenticator::PUBLIC],


Would be good to have a default value on these too. Good defaults help users succeed with less effort.

We can set defaults to v4 local here as well.

markstory · 2022-06-12T03:46:15Z

tests/TestCase/Authenticator/PasetoAuthenticatorTest.php

+            case PasetoAuthenticator::PUBLIC:
+                /** @var Builder $token extracted */
+                /** @var AsymmetricSecretKey $privateKey extracted */
+                extract($this->buildPublicToken(new Version4()));


This feels risky to me. Having a method arbitrarily update local scope has not worked out well for us in the past. Can we be more explicit with what is being set in local scope here.

markstory · 2022-06-12T03:52:27Z

tests/TestCase/Authenticator/PasetoAuthenticatorTest.php

+        $authenticator = $this->getMockBuilder(PasetoAuthenticator::class)
+            ->setConstructorArgs([
+                $this->identifiers,
+                [
+                    'purpose' => PasetoAuthenticator::LOCAL,
+                    'version' => 'v4',
+                ],
+            ])
+            ->onlyMethods([
+                'getPayLoad',
+            ])
+            ->getMock();
+
+        $authenticator->expects($this->once())
+            ->method('getPayLoad')
+            ->willReturn((new JsonToken())->setSubject(''));


Does this need to use a mock? There is a lot of code being dedicated to mocking to avoid creating a value we could generate with fewer lines of code.

Could we try having a helper method that generates a key pair and creates an authenticator using that key pair. You would also need a method that generates a token for a given input.

This is a bit of plumbing work to do but in the long term it helps with maintenance as we don't have to worry about changes in a mocking library. Additionally it also gives us more confidence in the tests as we are testing more of the subsytem all together.

othercorey · 2022-06-21T19:23:53Z

@cnizzardini Can you follow up on this?

cnizzardini · 2022-06-21T19:59:08Z

Its on my mind, some personal items getting in the way of freetime dev right now. Gimme a few days.

…

On Tue, Jun 21, 2022 at 3:24 PM othercorey ***@***.***> wrote: @cnizzardini <https://github.com/cnizzardini> Can you follow up on this? — Reply to this email directly, view it on GitHub <#538 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABJ2HTI5QLF42KM6QV7XJTVQIJFLANCNFSM5XFS2CZA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

…y mocks from tests

othercorey · 2022-10-17T07:48:59Z

@markstory can you take another look?

ADmad · 2022-10-18T14:01:33Z

src/Authenticator/PasetoAuthenticator.php

+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ * @link          https://cakephp.org CakePHP(tm) Project
+ * @since         3.0.0


Suggested change

* @since 3.0.0

* @since 2.10.0

ADmad · 2022-10-18T14:02:38Z

src/Authenticator/PasetoAuthenticator.php

+        'tokenPrefix' => 'bearer',
+        'returnPayload' => true,
+        'version' => 'v4',
+        'purpose' => 'local',


Suggested change

'purpose' => 'local',

'purpose' => self::LOCAL,

ADmad · 2022-10-18T14:03:18Z

src/Authenticator/PasetoAuthenticator.php

+    /**
+     * @var \ParagonIE\Paseto\ProtocolInterface|null
+     */
+    private $version;


Suggested change

private $version;

protected $version;

ADmad · 2022-10-18T14:10:10Z

src/Authenticator/PasetoAuthenticator.php

+
+        $this->version = $this->whichVersion();
+
+        if ($this->version === null) {


The whichVersion() method itself can throw an exception instead of returning null.

github-actions · 2023-02-16T02:57:57Z

This pull request is stale because it has been open 30 days with no activity. Remove the stale label or comment on this issue, or it will be closed in 15 days

markstory reviewed May 30, 2022

View reviewed changes

cnizzardini commented Jun 2, 2022

View reviewed changes

cnizzardini changed the base branch from 2.x to 2.next June 2, 2022 23:20

cnizzardini commented Jun 3, 2022

View reviewed changes

cnizzardini commented Jun 7, 2022

View reviewed changes

cnizzardini force-pushed the paseto branch 2 times, most recently from 7ece209 to 3d73585 Compare June 7, 2022 21:43

Adds PASETO authenticator, identifier, and console command.

5cdc740

cnizzardini force-pushed the paseto branch from 3d73585 to 5cdc740 Compare June 7, 2022 21:44

cnizzardini added 2 commits June 7, 2022 17:45

Merge branch '2.next' into paseto

ef54f0e

fixes bad rebase

3fcae0d

markstory reviewed Jun 12, 2022

View reviewed changes

cnizzardini added 3 commits June 21, 2022 17:21

Merge branch '2.next' into paseto

372f3e9

set v4 and local as default version and purpose and remove unnecessar…

281b8ee

…y mocks from tests

increase coverage

689c0a3

cnizzardini requested a review from markstory July 7, 2022 01:50

ADmad reviewed Oct 18, 2022

View reviewed changes

github-actions bot added the stale label Feb 16, 2023

github-actions bot closed this Mar 3, 2023


		$this->version = $this->whichVersion();

		if ($this->version === null) {

Uh oh!

PASETO authenticator and identifier #538

PASETO authenticator and identifier #538

Uh oh!

Conversation

cnizzardini commented May 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cnizzardini Jun 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cnizzardini Jun 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

othercorey commented Jun 7, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

othercorey commented Jun 21, 2022

Uh oh!

cnizzardini commented Jun 21, 2022 via email

Uh oh!

othercorey commented Oct 17, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

cnizzardini commented May 27, 2022 •

edited

Loading

cnizzardini Jun 1, 2022 •

edited

Loading

cnizzardini Jun 1, 2022 •

edited

Loading