[V3] SSH login slower in V3 with key whereas it takes less than 1s in V2

[MrVerifit]

We have been using V2 of your library in our WordPress plugin but we recently received multiple reports of error by some Hosting Providers when user try to update PHP to version 8

Column 18, Line 803
Column 30, Line 835
Column 41, Line 838
Column 36, Line 843
Column 28, Line 851
Column 28, Line 860
Column 18, Line 862
Column 18, Line 1159
Column 36, Line 1186
Column 28, Line 1191
Column 18, Line 1201
Column 125, Line 1663
Column 14, Line 1734
Column 14, Line 1735
Column 18, Line 1739
Column 86, Line 1837
Column 122, Line 1837
Column 185, Line 1837
Column 221, Line 1837
Column 259, Line 1837
Column 298, Line 1837
Column 32, Line 1838
Column 32, Line 1839
Column 31, Line 1844
Column 82, Line 1844
Column 14, Line 1849
~/vendor/phpseclib/phpseclib/phpseclib/Crypt/Base.php

Column 26, Line 66
Column 26, Line 109
Column 53, Line 109
~/vendor/phpseclib/phpseclib/phpseclib/Crypt/Random.php

V2 does work well for us till PHP 8.2 but these reports can be related to security issues and we couldn't risk, so we decided to use V3 of phpseclib. On upgradation, we found out that V3 is performing slow as compared to V2. During our inspection for sftp login, we found issue with BigInteger::powMod (or BigInteger::modPow) which is taking all the time during signing method. We already tried forcing BCMath (default in V2), but it is still slower.

Here are some benchmarks

V2
Time to login via sFTP: 0.56785297393799s
Mode: "BCMath", "OpenSSL"

and

V3
Time to login via sFTP: 14.192477941513s
Mode: "PHP64", "OpenSSL"

Time to login via sFTP: 6.0693409442902s
Mode: "BCMath", "OpenSSL"

Time to login via sFTP: 4.7939910888672s
Mode: "BCMath", "DefaultEngine"

PHP: 8.0.10
OpenSSL: 1.1.1f 31 Mar 2020
BCMath: enabled

Let me know if you require more information. Thanks

[terrafrost]

Does terrafrost@41cb11e help?

[terrafrost]

That said, I would expect v3 to be a little slower because, for example, PublicKeyLoader::load('...') is basically brute forcing every possible key format (ECDSA and DSA included). Also, v3 has more abstraction, which probably is gonna slow things down to an extent. But that shouldn't result in slow downs as significant as you're seeing and I think the above code change should help with that.

[MrVerifit]

Does terrafrost@41cb11e help?

Thanks, Yes this helped a lot and the performance is now significantly faster 👌🥳

V3 with Fix
Time to login via sFTP: 2.3652219772339s
Mode: "PHP64", "OpenSSL"

Time to login via sFTP: 0.91955590248108
Mode: "BCMath", "OpenSSL"

[terrafrost]

One thing that's interesting is how BCMath is faster for you then PHP64. phpseclib uses the PHP64 engine preferentially over the BCMath engine. In light of your analysis I updated https://github.com/phpseclib/benchmarks/blob/master/test.php to work with phpseclib 3.0 and ran benchmarks on PHP 8.1 vs 8.2.

So with PHP 8.1 x64 with BCMath that benchmark script takes between 0.46s and 0.49s. With PHP64 on PHP 8.1 x64 it takes 0.38 and 0.40s. So, clearly, PHP64 is faster.

With PHP 8.2 x64, in contrast, both PHP64 and BCMath take between 0.06s and 0.09s.

For fun I tried it on PHP 7.0 x64, as well. On that one PHP64 takes between 0.56s - 0.58s whereas BCMath takes between 0.89s and 0.91s.

TLDR it might be worthwhile to default to BCMath on PHP 8.2 whilst still defaulting to PHP64 on PHP 8.1 and earlier..

[MrVerifit]

I don't know what could be causing this.
I am using the following sftp-docker image:
https://hub.docker.com/r/atmoz/sftp

And this is my private key used during tests:

-----BEGIN RSA PRIVATE KEY-----
MIIJKAIBAAKCAgEAusdgRuScVVHQMcp4gWeaKcJROGZm42KXQndULY57sVuYW7fL
9FJlNoPaA4XNHlLXMwS1exJZjLGfRs/7h7UnI+Af0t2sqh7/oyf5yrYceZ7fmEUc
o5HQ3J81i0OLy1BhwIXUKbYtBYumAzYZv/toplkEsYTCATxOMdeWv93BbALwTpBY
C8PZatae/QUJbwvThCtOatPct3AQ/m6hQ2cWuTN5tt3Gdg/P6FQRZR1dopwUbIaf
WVtUTxy8zOahEhsht8Ne8J5wAmaNBijOAVngLoB5W+t5f1U0C+Z9l8CXaNVHXQQE
oAjy4rAu/giY3WCTNK5F/EAlHY7T0ATxzEANbVAe1Lxy7EgCtWNd13Qr0+yV92D1
3IQaoK5muLnNPGYITwxjyj8hkVJ9rNepCsHz9ZPj21zu+7SZr9Uk5IWmwGDqAnE7
cW65Cmqj8Fd+rQYiq4SClRVBK7gRJSP5YmwJ0lVR32px7LYf47kG/xgK2yONxrqY
AV/uJAM0/NtB2nX4HPU/7qMwX402WGuDCFIy11XVMXmLuJChwfDlL0up0+TUyffb
BMYn7xvBIVIlO07ZT+KYvNme/F4i7lvOMYfFB7cRjhXgpLqPphaZ6AGjVme1r3UD
V+d0XJRZFYzZH1+RV6DcGVwfN+su425INX60OMpDVRr9PFdVABg6TDyj24kCAwEA
AQKCAgBDdAV9FMqwtV8u3CQw961hL2ANsk2uCMj/pACugyqAnsejN/lcPvV2fp+Z
bhTpwpkdP7IaxEG64drXw/zewblk52gWyx+0QIWp8qHvZ11ZPtsZLrxtBhgb1RON
O9OclWb4FFzOw4/21LwbpMof3zen+7YKNhuqPpEuxuEWYYuc6mLGvLJPbWdzghbg
4ZTKmcbFkK49Pk4ToVFB0cpDgVNfhwuJ0wX9VTCMgu/VSImsVBm8J9IcfzMvbv1w
MkoxM8M1jkUXOODSvR5o/wuGyWKA6LzrSVsTdT3anl71EwSbQ4sGOJFDCnhNDbuq
2of6SwpemCGGO9QrnI06Ls70KJ2wIpeJFxIWq4S6gbAXllC8PXC8BQGhDa+3a0Ci
Bu4gPRgfRyJXvnOrZjWZK9i3LRvMXQNvCqSTqjHXFk7Njcm+uiyAgz4rEeVIEdol
QRRUngvgCwWy6V5Vs6cfFbrcieoMcm68SC5CdLft25EvYkAu0OpLaUq1dSssWkZr
t77XhuqKNkISKcUB/vo5WihU5yhcpm00bTVdPJBq9uPAmJg8wmWMklBXrRy1wNgV
l/IlkDrAeQuhSZfTrz75YWRHGS0LvQ1FU/1BwJ7CMV39PaG5z/jbdXVcD/M85bzT
fSMndasr+GWoRsnNMjvCawc5d7gQZukkt8nTuc4vp2TCNbV+0QKCAQEA0tqVdG78
eJ3zQBE+UtXqqg+w9z6SWRBFmaemEyiohc0R5tUm1vSD1EiUYt0ktFUSVfFVRqpP
pNnOCjw1UStC22Do/cAGU5Oyb3ZDJo57QCZsMI8P0xzEHy5HqRRIckLGPN6Uiqho
e2c7CdRh+PQyHVNBaSYPzaCIi3N5xYQaTsdjBF3xk0xG3vaNWjeGNcSvNpwQfm49
dC4lA4FJvVHXIKk285tNE3GqKNaEykDYSNi9+mqsShIasXHvEs6zx1woxyhBxtu2
/wE6V+CM92SN8ghXmgwBRLI1QDJwSctuN36FSMpzAh64NbLwjm21xrv7l3ox+Ll0
m+AYD5mI+z/gywKCAQEA4sUuSw2F1hsvh4Q5L2/Bj+pU41EIJyNk9OcUTLx4EJkT
tOuglgsc4ATnozMYvYExHEoMcjX68nxq8QxhMQdcbLWtP2e1LOF7jfQDg3iXVZO9
SOZY1crxPtesUlWcEM9UIHaYM2QpNeShuWAYx4ZF2UOaWXgv9hDYtzInKpE9FXj3
irsLmdROoeqOqk4+fj051J0rmRmlqcfxIPnnEQnsH7zTnOv6Wp6zu/MmzlSkwsPG
WLYm1D5MVU9tby7nj/6O9d75qJfswrCG0+D1U+TMKYBllq9UJ85uiFp2xh5MpITZ
1WP55ExXxljlmuoRqdlCh/VFkBZmeeOmQOlYJb9OewKCAQAVk0xLt7eZ1/+2BI2u
CDmU4ToocnyjJGkaye7ScwybdFpqllD9GdQLesnEPACveyz/IkypiG/R3H+ayIJk
JdsXe5kbrfR3xNszUJSac2XlMIlNhrYF0iG3VxL6GBs5pd+HBveIIKdgSTXfUZ4c
zU/7dDNzQPPJBK40JsLFFTlj6j8ViU9Y1StDUVMxchEHm3KaUenWJg2fv8EF1Mro
1mD+9JGs561BWhYhS6IW1//JpHP8FcI/ag7VLWVbNU9sDOAXUOU6Je1ZtpRzvYRv
JBjqNTRRjPzsPJf6U67tviUvBIE664xNITuUMxUuXceF6GJcI6LNYpyt/oY3wmwj
2Wt1AoIBAA+LDF1AlAfU3TEgsvq5Yj8HBMJ/71sfZfATVqTb1i7tDxi7fcpLALOq
ImZhPjTfAgq6WJgLcyPju0DCZHcD1iXtXe6WoShuXBWiwArm3RiUg8jXYQdmXZq9
FaafD49nZpDDEDOVje2PujYGjCxA29DEfXPcdQ0con4MBfs48ULjg7QSlfz5PFqE
xTqPGmF0uN4F19MqmtxERBbKijf0wiZ6vtZmpkZH0nHzfJlv28stuA6kjYzGqFzA
kPOp9107SXk4y0vu/d9qUhN7u9UAPa4qiswFQHNqBtF5weJaeU5dfMDPlXU8dle2
CCbSHFvT9Gd1PEtf/a4lGGQimlQcBLkCggEBANJi7onCVO4uy0TECHw1pemyxS+8
tXdG7A6KLE1AB6u4m8C7viVR4c7uijW5arS1FmNNahXQ5LAMu9PTp7FcuwyYKSUA
QppnSe+I2YhkiZrEZwBUZieUIKx1+0nkcHij4pErSF0Tmls2yauiSlQtLJn2fi/m
buqFclTJmtmQMggKs6g9B5NFIbkXzfTue8ckkD+6eyG2sf4znHDQKUG5ESBiXOEy
GZGXa/FRFHa/TLYel01CjqLNmyA2tXqCoAYh/kSVUkkn7Md1+Yk8mlpdCfDW4/wM
B8giW1DNQwnoCgq1vFuEkCREv1YOVkClw7vJXk2uP5bBZ8vIk+E/kubFOCE=
-----END RSA PRIVATE KEY-----

Since we can't control servers of users, once this fix is released, I am thinking about either providing a explicit option to users within our plugin to whether prefer BCMath over PHP64 or prefer PHP64 over BCMath while always prefering GMP over others. Or maybe automate this selection during connection test.

Thank you for your quick response and fix.

[terrafrost]

So I wanted to do some more testing before possibly making BCMath prioritized over PHP64 and... altho I said in an earlier post that BCMath was faster than PHP64 on PHP 8.2 I was not able to duplicate that when trying to do so just now.

I created a heatmap showing the fastest and slowest speeds at http://phpseclib.com/docs/speed . Per that PHP64 is consistently faster than BCMath BUT, when OpenSSL is being used, the difference is pretty negligible. Consequently, I thought I'd just go ahead and make it use BCMath over PHP64 / PHP32 when OpenSSL is available but then Github Actions kept on timing out on the unit tests.

I'd still like to be able to better understand why, on your system, BCMath is outperforming PHP64, but none-the-less, I've released a new version of 3.0 that includes this fix.

Thanks!