Cannot instantiate abstract class 'RSA'

[applibs]

$key = new RSA();
if (!$key->load($raw)) {
    return false;   // Not an unencrypted RSA key.
}

[terrafrost]

You're trying to use phpseclib 2's API with phpseclib 3. That won't work unless you use https://github.com/phpseclib/phpseclib2_compat

To do this with phpseclib 3 you need to do it this way:

use phpseclib3\Crypt\PublicKeyLoader;

$key = PublicKeyLoader::load($raw);

[applibs]

I downloaded last v.3.0.3 and all reported fails are from this library source code. Not from 3rd party lib.

[applibs]

OMG See this?
https://github.com/phpseclib/phpseclib/blob/3.0/phpseclib/File/X509.php#L3694

[applibs]

OK, how you explain this download link https://github.com/phpseclib/phpseclib/archive/3.0.3.zip this code described as wrong?

[terrafrost]

OMG See this?
https://github.com/phpseclib/phpseclib/blob/3.0/phpseclib/File/X509.php#L3694

You found a bug. Congrats!

phpseclib 3 was released a month ago. Anyone thinking such a significantly overhauled version is going to be bug free is an idiot.

[applibs]

It's not about whether someone is an idiot or not. I don't care. In my opinion, it's about using tools that are able to base deficiencies or identify them. The use of such tools is highly effective.

[terrafrost]

OK, how you explain this download link https://github.com/phpseclib/phpseclib/archive/3.0.3.zip this code described as wrong?

The phpseclib project has a Git Hook that with some API that sourceforge has. Maybe it'd be best to remove that hook. The 3.0 release, in particular, is not really meant to be installed without Composer. 2.0 requires an autoloader but it doesn't necessarily need to be Composer. 1.0 doesn't even require an autoloader.

it's about using tools that are able to base deficiencies or identify them. The use of such tools is highly effective.

A lot of these tools have such a high noise to signal ratio that it renders them practically useless. Put another way they have waaaay more false positives than they do legit positives.

Maybe they've gotten better since last I've tried them. I do apologize for having better things to do than trying the same tool over and over and over again, week after week after week, expecting shit to have changed.

And based on a lot of the tickets you've submitted (which, admittedly, I'm still going through) I am seeing a lot of false positives.

[applibs]

Yes sure false positives must be ignored.

[applibs]

OK, close all my reports or I will do it my self.

[terrafrost]

I'm still looking into them. Since you took the time to submit them I feel obligated to look into them now. And you did find some legit issues (most notably, the one mentioned in this ticket).

I apologize if my rhetoric with you, to this point, has been harsh. #1594 put me on the defensive.

[terrafrost]

In reviewing your issues you found two completely legit bugs (new RSA() and instanceof ECDSA [#1593]). You found three legit issues (#1592, #1591, #1590) as well. I wouldn't go so far as to call them bugs but they prob ought to be fixed all the same. And then you found a few non-issues (#1588, #1589). I mean, I guess #1588 is more subjective. It's def not a bug. Maybe the way it's doing things, however, could be rewritten in a way to be more clear? idk.

#1586 and #1587 were based on a misunderstanding (the assumption that phpseclib 3 used the same API as phpseclib 2)

[terrafrost]

Since you, I guess, used a static code analyzer, I'm curious... how many of the issues that it reported to you did you filter out yourself?

Like if it only found 7 issues and of them 5 of them were legit that's not bad at all! If you manually filtered it down from 200 issues to 7 issues then the ratio of false positives to legit issues is waaaay to high for me to want to bother with it. I mean, if I had an infinite amount of time, sure, but as is, time is a very scarce resource.

[applibs]

No be lucky, I have no time. There were so many that I gave up.

[applibs]

I recommend using PHPStorm Free. Edition For Open Source projects like this. Feature - Code inspection tells you much more than I do. It is necessary to work with it carefully and not to accept all the recommendations, because undesirable malfunctions can also occur. After debugging the code with this tool, the project will be perfect. You will be happy with how nice the code will be.

[terrafrost]

No be lucky, I have no time. There were so many that I gave up.

Honestly, that's probably how I'll react.

tbh I don't give two shits about DocBlock comments and if there are 300-400 issues that it finds about them... maybe it takes me two months to fix. But ya know what? That's two months I could spend doing much more relevant stuff like coming up with new API to replace the shit API that X509 currently has. And it may not even be immediately obvious that the issue ultimately is a DocBlock issue. Certainly it wasn't immediately obvious from your tickets - it took some time to make that determination.

Of the tickets you submitted ~30% are on issues I'd actually make a priority to fix, which isn't actually that bad for a static analyzer lol. But even then... what's a better use of my time? Fixing a bunch of bugs that no human being has ever encountered because they're in parts of the code that no one ever utilizes or making the parts of the code that people actually do utilize better?

To think about it another way... according to packagist.org this library was downloaded 2.7 million times in the past 30 day. 99% of those downloads are for the v2 branch. There are ~92 million total downloads, 99% of which are for the v2 branch. 2.0.0 was first released 5 years ago, almost to the day. I'm sure there are bugs in it but if, after all that time and all those downloads, those bugs have yet to be encountered by anyone, then is it really the best use of my time to proactively track all those bugs down?

phpseclib has issues that static analyzers will never find. Issues that require complete redesigns to fix. eg. #479 (comment) or #655 (comment) or #1440 (comment) . The list goes on and on. So what should be a higher priority for me? Fixing known long standing issues like those or going to the ends of the Earth to track down and fix bugs in obscure features that are used by maybe 1% of people? I mean, sure, maybe the issues I've just mentioned aren't bugs, but being bug free doesn't make a project perfect. Indeed, I'd say that a bug free project with a shit API is far from perfect and right now I think phpseclib has a pretty shit API. It didn't seem like a bad API when I came up with it but a lot of the API's are over 10 years old now. I've learned so much more sense then and my sensibilities and the sensibilities of the PHP community have changed a lot in that timespan as well.

[terrafrost]

Anyway, I'm not saying I won't look into it, but rather, that in the grand scheme of phpseclib things, I do have higher priorities and the amount of time I'm going to be willing to invest in sifting through the deluge of issues that the code inspection feature reports on is going to be limited.

[terrafrost]

The shittiest part of phpseclib's current API is, imho, the X509 stuff. It's just god awful. Reading X.509 certs is fine but creating or modifying them is a god awful experience.

I want to make it so that creating an X509 cert works like this:

$x509 = new X509;
echo $x509; // invokes __toString() magic method

That creates a minimally viable X.509 cert. eg.

-----BEGIN CERTIFICATE-----
MEMwOqADAgECAgEAMAMGAQAwADAeFw0xOTA1MjgwMTU1MDVaFw0xOTA1MjgwMTU1
MDVaMAAwBzADBgEAAwAwAwYBAAMA
-----END CERTIFICATE-----

It's a cert with an empty signature because no private key has been provided to create the signature. It's a cert with an empty issuer or subject because none have been provided, etc.

You can modify that cert manually or by calling functions. Here's an example of manual modification:

$x509 = new X509;
$x509['tbsCertificate']['issuer']['rdnSequence'][0][0] = ['type' => 'id-at-organizationName', 'value' => 'phpseclib demo cert'];
echo $x509;

ArrayObject is used to make $x509 accessible as an array.

This method is a bit burdensome, however, so you could, instead do this:

$x509 = new X509;
$x509->setIssuerDN('/O=phpseclib demo cert')
echo $x509;

The problem right now is that the base structure of anything in X.509 isn't set until you call saveX509. It has to be done this way because the X509 class is used to do CSRs and CRLs and SPKACs, etc. This makes for waaay over complicated and hard to maintain code.

With the __debugInfo() magic method you could do this:

$x509 = new X509('...');
print_r($x509);

So the loadX509() call would be removed and instead you'd just pass the X.509 cert to the constructor. This would mean that reloading X.509 certs and having to reset stuff becomes simpler.

As for how signing would work.. not completely sure yet lol. Maybe something like this:

$x509 = new X509;
$x509->setIssuerDN(...);
$x509->setSubjectDN(...);
$private->sign($x509);
echo $x509;

To facilitate $private->sign($x509) I could do if (is_object($message) && $message instanceof X509) in the sign method of every privatekey class (eg. Crypto\EC\PrivateKey.php, etc).

[terrafrost]

Anyway I just mention the X.509 thing because, if I'm gonna spend two months working on something, I'd rather work on that then on fixing bugs that potentially no one would ever hit anyway.

Like if a tree falls in a forest and no one is around to hear it, does it make a sound? If there's a bug in a piece of code that no one ever calls does that bug really exist? The technical answer to both questions is yes but the question has broader implications than that.

I would rather have a perfect API with bugs than a shitty API that's bug free. I would rather have a buggy library that people want to use than a bug free library that no one wants to use. And altho the SSH2 API could certainly use improvement that's not the shittiest API that phpseclib has. That honor belongs to X509!

[applibs]

This is your fight. :-)

[terrafrost]

Yah I know lol. But I do think it might be helpful if people understood my priorities and periodically reevaluating them is never a bad thing! And sometimes just typing up the rationales for said priorities can help elucidate things, even if it's only elucidating things for me lol.

[terrafrost]

3d47673 should fix this - thanks!