Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proof of Concept] - Hypothetical Version 2.x #154

Closed
wants to merge 25 commits into from
Closed

Conversation

paragonie-security
Copy link
Contributor

@paragonie-security paragonie-security commented Sep 26, 2022

See #137 for background

This PoC exists to answer the following questions:

  • What would sodium_compat look like if its minimum version was 7.2?
  • Would it perform better or worse than 1.x?
  • How much code could we delete?

I may follow up with another branch that pins the minimum to 8.1 and uses FFI for scrypt/Argon2 support.

Note: This will never be merged into the master branch (indefinite PHP 5 support is a goal of that branch). Additionally, no tags/releases will be issued from this branch until we're settled on what the minimum version constraint for a sodium_compat v2.x should even be.

What's Been Removed?

  • The old \Sodium\*() API
  • PHP 5 support
    • PHP 7.0 and 7.1 support are gone too! 7.2 is the minimum for this PoC
    • This means no more paragonie/random_compat as a dependency
  • 32-bit Integer Support
  • A lot of boilerplate code (thanks, improved type system!)

What's Been Changed?

  • The APIs now have scalar type declarations, rather than relying on runtime checks

Not much else, really. You should be able to test these changes against your own code without modifying anything since v1.19.0.

How Do These Changes Affect PHP 8.2?

dev-master

PHPUnit 9.5.25 #StandWithUkraine

.......S......S.......S......S.........S.S.....S.S.S.SS....S...  63 / 240 ( 26%)
......................SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS. 126 / 240 ( 52%)
.............................S.......SS..S..................... 189 / 240 ( 78%)
........S........SSSSSSS......SSSSSSSSSSSSSSSSSSSSS             240 / 240 (100%)

Time: 00:44.502, Memory: 14.00 MB

OK, but incomplete, skipped, or risky tests!
Tests: 240, Assertions: 3436, Skipped: 85.

This Branch

PHPUnit 9.5.25 #StandWithUkraine

...............................................................  63 / 185 ( 34%)
.............................................SS..S............. 126 / 185 ( 68%)
................S........SSSSSSS......SSSSSSSSSSSSSSSSSSSSS     185 / 185 (100%)

Time: 00:32.863, Memory: 12.00 MB

OK, but incomplete, skipped, or risky tests!
Tests: 185, Assertions: 2428, Skipped: 32.

Some tests were removed (due to removed support).

We don't need the old libsodium source code in comments anymore.

We're confident our implementation is correct.
@paragonie-security
Copy link
Contributor Author

Q: Does strict-typing impact performance?

Here's a dumb benchmarking script:

<?php
require_once __DIR__ . '/autoload-phpunit.php';
ParagonIE_Sodium_Compat::$disableFallbackForUnitTests = true;

$stop = $start = 0.0;
$i = 0;

$start = microtime(true);
while ($i < 1000) {
    $p = sodium_crypto_core_ristretto255_scalar_random();
    $q = sodium_crypto_core_ristretto255_scalar_random();
    $r = sodium_crypto_core_ristretto255_scalar_add($p, $q);
    $p = sodium_crypto_core_ristretto255_scalar_sub($r, $q);
    $z = sodium_crypto_core_ristretto255_scalar_mul($p, $q);
    $comp = sodium_crypto_core_ristretto255_scalar_complement($p);
    $inv = sodium_crypto_core_ristretto255_scalar_invert($p);
    $neg1 = sodium_crypto_core_ristretto255_scalar_negate($p);
    $bytes = random_bytes(ParagonIE_Sodium_Compat::CRYPTO_CORE_RISTRETTO255_NONREDUCEDSCALARBYTES);
    $red = sodium_crypto_core_ristretto255_scalar_reduce($bytes);

    $alice = sodium_crypto_box_keypair();
    $alice_s = sodium_crypto_box_secretkey($alice);
    $alice_p = sodium_crypto_box_publickey($alice);

    $bob = sodium_crypto_box_keypair();
    $bob_s = sodium_crypto_box_secretkey($bob);
    $bob_p = sodium_crypto_box_publickey($bob);

    $a2b = sodium_crypto_scalarmult($alice_s, $bob_p);
    $b2a = sodium_crypto_scalarmult($bob_s, $alice_p);
    ++$i;
}
$stop = microtime(true);

var_dump($stop - $start);

Results (dev-master)

PS> php .\bench.php
float(0.2294149398803711)
PS> php .\bench.php
float(0.22782397270202637)
PS> php .\bench.php
float(0.22809386253356934)

Results (v2.x branch)

PS> php .\bench.php
float(0.2278280258178711)
PS> php .\bench.php
float(0.2308211326599121)
PS> php .\bench.php
float(0.23272085189819336)

Results (v2.x branch with Strict Typing)

For this test, I added declare(strict_types=1); to all the .php files in scope

PS> php .\bench.php
float(0.22864103317260742)
PS> php .\bench.php
float(0.22912812232971191)
PS> php .\bench.php
float(0.22997093200683594)

Summary

Test Trial 1 Trial 2 Trial 3 Average
master 0.2294149399 0.2278239727 0.2280938625 0.2284442584
v2.x 0.2278280258 0.2308211327 0.2327208519 0.2304566701
v2.x-strict 0.2286410332 0.2291281223 0.229970932 0.2292466958

It seems that any performance penalty incurred by strict types is negligible at best.

@paragonie-security
Copy link
Contributor Author

paragonie-security commented Sep 26, 2022

I forgot to disable ext-sodium, so here's a better benchmarking script.

<?php
declare(strict_types=1);

require_once __DIR__ . '/autoload-phpunit.php';
ParagonIE_Sodium_Compat::$disableFallbackForUnitTests = true;

$stop = $start = 0.0;
$i = 0;

$start = microtime(true);
while ($i < 5) {
    $p = ParagonIE_Sodium_Compat::ristretto255_scalar_random();
    $q = ParagonIE_Sodium_Compat::ristretto255_scalar_random();
    $r = ParagonIE_Sodium_Compat::ristretto255_scalar_add($p, $q);
    $p = ParagonIE_Sodium_Compat::ristretto255_scalar_sub($r, $q);
    $z = ParagonIE_Sodium_Compat::ristretto255_scalar_mul($p, $q);
    $comp = ParagonIE_Sodium_Compat::ristretto255_scalar_complement($p);
    $inv = ParagonIE_Sodium_Compat::ristretto255_scalar_invert($p);
    $neg = ParagonIE_Sodium_Compat::ristretto255_scalar_negate($p);
    $bytes = random_bytes(ParagonIE_Sodium_Compat::CRYPTO_CORE_RISTRETTO255_NONREDUCEDSCALARBYTES);
    $red = ParagonIE_Sodium_Compat::ristretto255_scalar_reduce($bytes);

    $alice = ParagonIE_Sodium_Compat::crypto_box_keypair();
    $alice_s = ParagonIE_Sodium_Compat::crypto_box_secretkey($alice);
    $alice_p = ParagonIE_Sodium_Compat::crypto_box_publickey($alice);

    $bob = ParagonIE_Sodium_Compat::crypto_box_keypair();
    $bob_s = ParagonIE_Sodium_Compat::crypto_box_secretkey($bob);
    $bob_p = ParagonIE_Sodium_Compat::crypto_box_publickey($bob);

    $a2b = ParagonIE_Sodium_Compat::crypto_scalarmult($alice_s, $bob_p);
    $b2a = ParagonIE_Sodium_Compat::crypto_scalarmult($bob_s, $alice_p);
    ++$i;
}
$stop = microtime(true);

var_dump($stop - $start);

Updated Results

Test Trial 1 Trial 2 Trial 3 Average
master 4.158897877 4.164984941 4.245276928 4.189719915
v2.x 3.994076967 3.92305994 3.975117922 3.964084943
v2.x-strict 4.012229204 4.049883127 4.064672947 4.042261759

From v2's perspective, there is a small (1.97%) performance hit for using strict_types. However, the savings from dev-master are still larger (3.64% slower).

If we anchor on dev-master, the total run1time is 94.61% (v2) and 96.48% (v2-strict) what we were seeing before.

Overall, performance is improved with v2.x.

@paragonie-security
Copy link
Contributor Author

I'm closing this pull request. If we decide to continue work on v2.x, the current branch will serve as the basis for this work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant