Add phpseclib to core

Does this means updater will always work without providing extra login information?

Drupal being able to run a ssh server terrifies me but... its a cool idea.

oh, much better. still scary that we could but hey, its scary what you can do with php. not relying on pecl++ since that's surely not an options for a lot of people that would be using this.

Tagging and subscribing. :-)

1. phpseclib uses an old PEAR style of library inclusion/loading. So either needs careful loading, or an adjustment to our autoloader.

   From http://phpseclib.sourceforge.net/documentation/intro.html#intro_usage :

   set_include_path(get_include_path() . PATH_SEPARATOR . 'phpseclib');
   
   include('Net/SSH2.php');
   

2. The usage examples are relatively sparse, which prolly explains why no one jumped onto this issue with a patch yet:

   http://phpseclib.sourceforge.net/documentation/net.html#net_sftp_example

3. FWIW, despite its age, the library seems to be actively maintained; the repository shows commits from today:
   http://phpseclib.svn.sourceforge.net/viewvc/phpseclib/trunk/phpseclib/

   Of course, SourceForge... SVN... points to dated approaches. Given that the majority of web developers works on other modern platforms and technologies today, it should be allowed to ask who and how many are actually using and contributing to the library, and whether it is an experiment or actually has been battle-tested?

4. This issue doesn't clarify whether anyone actually looked at the library's code and verified that 1) it works, 2) is relatively sane, and 3) is secure. Did anyone do that?

   Also, since it directly ties into server security, did anyone check for security vulnerabilities and arbitrarily posted security issues/reports on the net?

Proper PEAR-style autoloading is supported by PSR-0 and the Symfony class loader, and I think the Composer one as well. However, it looks like the code is doing PHP-style manual inheritance loading, which depends on the include_path() being set. We never use that for good reason. In fact, the code looks to be all PHP4-based. That makes me rather worried that there are some obscure issues that will crop up when we try to run it on modern PHP 5.3.

I have no opinion on the security of the code itself as I have not checked it for that. But sun is right; integrating it sanely makes me uneasy.

On phpseclib's security

In response to the security concerns, all I can say is this. The phpseclib guy seems to have a better track record than anyone else. PEAR's Crypt_RSA was the gold standard for RSA in PHP and contained vulnerabilities. PEAR tried to come out with a Crypt_RSA2, too, and that one, too, was just as vulnerable as the first.

Guess which PHP RSA implementation was found not to possess these vulnerabilities? phpseclib's:

http://www.securityfocus.com/archive/1/520683

On phpseclib's use of PHP4-code

With regard to phpseclib being PHP 4 compatible. So?

The whole point of phpseclib, it seems to me, was extreme portability. It would have been easier to require gmp - far and away the fastest mpint extension for PHP - but the author didn't. Presumably to make it work on as many servers as possible. Note that it does use it if it's available but doesn't require it. Same thing for bcmath. It's available on more servers than gmp but the author still went to insanely great pains to make it work on those few servers that don't have bcmath by writing a pure-PHP arbitrary precision integer arithmetic library.

It would have also been easier to require mcrypt but the author didn't. They use it if it's available but if it's not it does it's own internal thing. Again, presumably to maximize the number of servers it'll work on. Besides, have any of you checked out libmcrypt on sourceforge? The project is dead as all get out. The last commit was eight years ago. I, for one, think that it's good that the phpseclib guy knows enough about symmetric key crypto to re-implement it. If mcrypt is found to have issues who's gonna fix it? The phpseclib guy at least as the requisite knowledge to work around it.

And you know what? It would have been even easier not to make phpseclib contain a pure PHP SSH implementation at all. If the notion of portability is kicked to the sand then you know what? Why not just use PECL's SSH2 extension? If you're gonna dictate how people ought to configure their servers (that they might not have any control over anyway) then where do you draw the line? Why stop at requiring PHP 5.3? Why not require PECL's SSH2 extension as well?

Hell - since you people want to tell people how to set their systems up here's an idea: don't support FTP or local writing. ONLY support SFTP. SFTP is the only secure way to do file transfers anyway! Maybe no one will use Drupal, at that point, but hey - at least ya'll can get all giddy over not using antiquated technologies!

kingkai: You wouldn't happen to be affiliated with phpseclib, would you? Since your account is brand new I have to ask. :-) (Note: That doesn't invalidate your statements at all; it's just good to know where everyone is coming from.)

Re #13, I think you're making a hyperbolic argument. Yes, the purpose of phpseclib is to make it possible to not need those server components. That doesn't mean that it should also be designed to work on ancient versions of PHP. The same argument could be extended to supporting PHP 3 and avoiding classes entirely... PHP 4 is just as dead at this point. (It's been 5 years since the entire PHP community banded together to loudly declare "TO HELL WITH PHP 4!!!" That's an eternity on the web.) The statement could be taken even further to "why do you even need PHP, just provide a C library you can call from any scripting language. That's even more portable!"

More practically, it means that integrating the code into Drupal becomes harder because of those extra include statements. We may need to hack the library to get it to work, which is certainly not a preferable situation. And that in turn leaves me wondering if we might get bitten by other PHP 4 / PHP 5 incompatibilities, which for OO code do exist and are usually just subtle enough to be really really annoying. :-) That's not an illegitimate concern.

It doesn't have to be PHP 5.2-decked out, but practically the way code works has changed since PHP 4. Even PHP 5.2 would be vastly easier for us to cleanly integrate.

Here's what the phpseclib author wrote on the subject of PHP5 compatibility:

http://www.frostjedi.com/phpbb/viewtopic.php?f=46&t=32804

If phpseclib went for PHP3 compatibility it couldn't be object oriented for one. It'd have a totally different API. Maybe the original author thought breaking PHP3 compatibility was worth it for an OOP API but doesn't think the features offered by PHP5 are worth it to break PHP4 compatibility.

And a C library is not that portable on PHP servers. If it was than the PECL libssh2 extension wouldn't be the problem that it is.

As for your question of whether or not I'm affiliated with phpseclib... I am. I'm working on revamping the documentation for it.

Also, what's the problem with setting the include_path? Obviously you don't want to go phpBB2 style and do include($drupal_root_path . '/dirname/filename.ext') at the top of include files lest you get bitten by register_globals but no one's saying that has to be done either.

And what PHP 4 / PHP 5 OOP incompatibilities do you know of? Certainly PHP5 OOP is incompatible with PHP4 OOP but I'm not aware of anything going in the other direction.

I've been using phpseclib for years in PHP5 and have never had any of these phantom issues you're suggesting will happen. And w.r.t. auto-loading... honestly, I think it's a bad idea. It complicates static analysis of code and then you can have multiple auto-loaders overwriting auto-loaders and, honestly, I think it's just a big mess.

Here's what the phpseclib author wrote on the subject of PHP5 compatibility:
http://www.frostjedi.com/phpbb/viewtopic.php?f=46&t=32804

Kingkai can you *please* tell use if you're the author or related to the author of the library.
The quote above is from a forum post made *today* just after the comments in this thread.

It's not bad if that is the case. The same things happen with he Symofiny dudes but at least we know they are connected to the library they are promoting.

As for your question of whether or not I'm affiliated with phpseclib... I am. I'm working on revamping the documentation for it.

@kingkai is not the author.

The pass-by-value vs. pass-by-handle changes in OO from PHP 4 to PHP 5 bit a number of projects. Lots of code works fine, certainly; Any well-written library should have no issue, but I've never used phpseclib myself so I have no idea if it is such a "well written library". (Not intended as an attack; simply pure ignorance on my part, leading to understandable caution.)

As for autoloading vs. include path, this is not the place to go into that minutia in depth. Suffice to say that there are many known issues (mainly performance) with the include path, and most projects have long since moved on from it. Drupal did years ago. The de facto standard for OO code these days is PSR-0-based autloading, which while by no means perfect is what many leading projects have adopted, including Drupal. Using phpseclib right now would require hacking the code so that it could use a PEAR-style (unnamespaced) class loader or else making an exception to our "we don't want to have to manually load code" momentum as well as our avoidance of the include path. These are legitimate blockers that make it more difficult for us to leverage. That's true even if it is the most secure library on the planet.

With regard to pass-by-value vs. pass-by-handle... that's not an issue for phpseclib. Quoting from the source of Math_BigInteger - one of the oldest parts of phpseclib:

     * PHP5 passes objects by reference while PHP4 passes by value.  As such, we need a function to guarantee
     * that all objects are passed by value, when appropriate.  More information can be found here:
     *
     * {@link http://php.net/language.oop5.basic#51624}

I asked the lead developer about PHP5 compatibility and he said that he doesn't even test his code in PHP4 anymore. Quoting from his message to me:

I don't even test in PHP4 anymore - just PHP5. My expectation is that the code would work in PHP4 but if it doesn't those issues can be addressed on a case-by-case basis.

The best example of this is actually bcpowmod(). It's a PHP5-only function. Math_BigInteger used to include PEAR's PHP_Compat to emulate that function on PHP4 but then dropped it:

http://phpseclib.svn.sourceforge.net/viewvc/phpseclib/trunk/phpseclib/Ma...

Note how none of the examples include the PHP_Compat stuff either. It's designed to work on PHP4 but PHP4 users would need to go out of their way to get it working (unlike PHP5 users)

As for auto-loading... phpseclib does use PHP5 magic functions such as __toString() and __clone(). The thing is... those functions are syntactically valid PHP4 code. Maybe you should make a post on the phpseclib support forums and work with the developer to make it support auto-loading for PHP5 and use include's for PHP4. Maybe ya'll could do version_compare(PHP_VERSION, '5.0.0', '>=') and do auto-loading if that's true and include's if it's not. So long as it's syntactically valid PHP4 I don't think the phpseclib dev would have any objections..

@kingkai Autoloading is a bit different and php 5 only. We were part of a big push called Go PHP 5 which helped move pretty much every major host to support php5. As such, we are actually talking about PHP 5.3.3 are our current minimum version for Drupal 8.

the phpseclib author said this about autoloading in the linked forum post:

spl_autoload_register() makes it so that when a non-existant class would be initiated a predefined function is called before displaying an error. The idea is to make it so you don't have to include a bazillion different files. phpseclib includes lots of files but I don't believe those includes to be so unsightly as to justify their removal and replacement with autoloading.

He then also put namespaces and php 5.3 support on his kickstarter. If done correctly that would actually be the sole requirement for our autoloading to work. PSR-0 is fairly handy and might be worth you looking into outside of this discussion.

I will also note that there are a lot of people in the Drupal community and we like helping along good code that will help us. Many of the listed kickstarter items could be taken care of in git PR's and there are even people that could help possibly help with the github conversion(SVN conversion is actually pretty easy compared to the CVS conversion we did). I'm not promising resources but having him join the discussion might be useful if he's interested in engaging our community. Most of the participants here are pretty active in our IRC channels too which might be faster for drawn out discussions.

Out of curiosity, if phpseclib was on github.com would you all be willing to remove the includes and do periodic merges as appropriate?

I don't imagine the lines with the include's have changed since the files were created so removing them and adding a namespace whatever; at the top shouldn't produce any conflicts. Just merge periodically and you should be fine.

That creates a local fork of the library, which we try to avoid. Also, if we start pulling in 3rd party libraries via composer, as many of us want to do, then there would be no real opportunity to run such local forks.

What if a remote fork was created? Then the local copy wouldn't be a local fork - it'd be a direct copy of a remote fork.

https://github.com/phpseclib

phpseclib is on github. Check out the updated documentation too. Still needs some work though.

Quoting an earlier post of mine,

As for your question of whether or not I'm affiliated with phpseclib... I am. I'm working on revamping the documentation for it.

You can see it here:

http://phpseclib.sourceforge.net/new/

It's just a preview though. Work still needs to be done. I didn't post the link because I didn't want to distract from the conversation at hand but now that the cat is out of the bag, what do you think?

kingkai: Providing relevant information about the project and your involvement in it is not a distraction. :-) It's very relevant to the discussion.

I don't think the author is gonna accept that commit unless you pay him, per this:

http://www.frostjedi.com/phpbb/viewtopic.php?f=46&t=32804

The second post says he'll make PHP 5.3 only (which is what PSR-0 requires) for $25k.

I don't understand why a remote fork can't be created? You don't want to do a local fork because, among other things, it'd prevent composer from being used. So just create a remote fork, merging as necessary (maybe when new releases are made maybe?). If you don't want to create a remote fork I can do that.

We don't want to create a remote fork because then someone needs to maintain that remote fork, and that's more work for us. We really try to avoid forking code if we can avoid it. (And when we do fork, it tends to be a rather drastic fork.) I don't know why the author is not willing to adopt a standard that makes life easier and is rapidly becoming widespread throughout the PHP world. Plus, side effects like inline includes are against the recently-adopted PSR-1 spec as well.

PSR-0 also does not require PHP 5.3. If you use PEAR-style autoloading instead, that's PHP 5.2-compatible and PSR-0 compatible. That's what Twig does, for instance.

Having to pay someone to accept improvements seems like a pretty big minus to me :(

Yeah, having read this thread, and the attitude expressed by the phpseclib maintainer in the linked threads, I'm -1 to this.

FTP is less eww if both client and server are on localhost, which is what happens in the Update manager case. Sure, it's sucky if the host has an ftpd listening at all, but that's outside the scope of what we can worry about ourselves. We just provide tools to try to take advantage of the server configurations we find. We can't dictate those server configurations.

And honestly, even if we had phpseclib and were doing SSH most of the time, sites are still screwed if the site admin isn't accessing the Update manager via https. Most hosts I know of charge you extra for https. So, in practice, I don't think sending your ssh password in clear-text over http from your browser to the update manager is any better than just using localhost-only ftp.

Anyway, if someone really wants to integrate this library, they can do it in contrib and they can be responsible for dealing with the problems that seem to be inevitable trying to collaborate with the upstream phpseclib maintainer (and/or maintain their own fork, whatever). However, we have enough problems in core, especially in the Update manager, I don't see a huge win in practice for end users here, and I don't see why it's worth taking on a bunch of new problems.

I'll leave this for someone else to mark "won't fix", but that's definitely my vote.

I think some of the comments about the phpseclib maintainer are uncalled for.

Maybe you all missed it but one of phpseclib's goals is ultra compatibility and intentionally breaking backwards compatibility is not really compatible with that goal.

I respect that the guy isn't compromising his vision. Similarly, I think it's respectable that Drupal isn't compromising it's vision. It shows that both projects have integrity.

If anything I think the Drupal community is in the wrong here. The phpseclib guy is trying to compromise. He moved his repo to github, among other things. You people are trying to bully him into doing it the Drupal Way(tm). If it's not the Drupal Way(tm) it's the wrong way. Because apparently the Drupal Way(tm) is the only way.

I don't know why the author is not willing to adopt a standard that makes life easier and is rapidly becoming widespread throughout the PHP world.

Because it conflicts with his vision for phpseclib? Jesus fucking christ do you people read anything? People have asked me multiple times if I'm the author of phpseclib and I'm not. I'm just working on some updated documentation for it. Because apparently saying anything once isn't enough for you people. So let me just say it one more time: breaking BC is incompatible with the authors vision.

FTP is less eww if both client and server are on localhost, which is what happens in the Update manager case. Sure, it's sucky if the host has an ftpd listening at all, but that's outside the scope of what we can worry about ourselves. We just provide tools to try to take advantage of the server configurations we find. We can't dictate those server configurations.

And honestly, even if we had phpseclib and were doing SSH most of the time, sites are still screwed if the site admin isn't accessing the Update manager via https. Most hosts I know of charge you extra for https. So, in practice, I don't think sending your ssh password in clear-text over http from your browser to the update manager is any better than just using localhost-only ftp.

Can't find fault with those points. Although I think your condemnation of the phpseclib maintainers attitude as well is uncalled for.

Believe it or not, the phpseclib maintainer does have other projects that do use namespaces. Those other projects use namespaces because their goal isn't ultra-compatibility. In my view they're vehicles for him to play around with the latest and greatest PHP has to offer. But that just isn't the goal of phpseclib.

I mean, hell - the phpseclib maintainer has probably written PHP scripts to use PECL's ssh2 extension. Just because you write one codebase with one goal in mind (code using PECL's sshs2 extension is probably written for testing purposes) doesn't mean that's the goal you write all your codebase's with.

You all write Drupal modules. Does that mean that that's all you people can write or that that's what you people prefer to write? Hardly.

Having to pay someone to accept improvements seems like a pretty big minus to me :(

Ah yes. More of the Drupal Way(tm) is the only way. If you're not a Drupal developer you're a bad developer! Drupal group think ftw!

You know what I think disappoints me the most about this topic? The attitudes of pretty much everyone here.

There are good arguments people could make that might have encouraged the phpseclib maintainer to break BC with his code. Like maybe acknowledging his concerns and pointing out that Drupal's adoption would increase the visibility of his project and that if anyone wanted to bring PHP4 compatibility back they could just remove the namespaces.

But it's not good arguments you people chose to use. You people chose to use arguments like "oh, you and your code suck".

I guess you people favor the stick over the carrot. Guess I shouldn't be surprised. This gunboat diplomacy approach you all seem to prefer got most hosts to finally upgrade to PHP5, which has been a plus, for sure. But I guess it's not just hosts you all bully is it?

I think some of the comments about the phpseclib maintainer are uncalled for.

Maybe you all missed it but one of phpseclib's goals is ultra compatibility and intentionally breaking backwards compatibility is not really compatible with that goal.

I respect that the guy isn't compromising his vision. Similarly, I think it's respectable that Drupal isn't compromising it's vision. It shows that both projects have integrity.

If anything I think the Drupal community is in the wrong here. The phpseclib guy is trying to compromise. He moved his repo to github, among other things. You people are trying to bully him into doing it the Drupal Way(tm). If it's not the Drupal Way(tm) it's the wrong way. Because apparently the Drupal Way(tm) is the only way.

@kingkai with all due respect to your experience and perspective, this is not an issue of assigning who is wrong. This is an issue of the possibility of the Drupal community working with the community driving the development of phpseclib in order to find ways were mutual gain can be had. I say mutual because Drupal would be able to benefit from the hard work that has gone into phpseclib, and phpseclib would benefit because we would estudiously file any bugs that we might find, suggest improvements, and collaborate in future development.

I think your charactization of our communication here is needlessly adversarial. IMO there is nothing wrong with free, open, and mutually respectful discussion and even debate over code amongst developers who all share the same goal, making the best code possible. We talk about forking because by the thread that was linked and by your reaction here it seems that there is a lot of resistance accepting the input of third-party projects like Drupal. We value the code that has been written thus far and would like to contribute, but if we're not wanted and if the license allows we'd like to find some way to interoperate.

For our goals, with Drupal 8 having the phpseclib in a format that makes it easier it integrate deeply also makes it easier for us to adopt. And since the PSR-0 format has been agreed upon by so many members of the php community that does not seem like an extreme or uncommon request.

On the money issue, paying folks for core functionality is not in our DNA. Drupal is an open source project and has been developed by folks who generously volunteer their time and expertise. I ask that your respect our culture and not make our refusal to pay a divisive issue.

I say mutual because Drupal would be able to benefit from the hard work that has gone into phpseclib, and phpseclib would benefit because we would estudiously file any bugs that we might fine, suggest improvements, collaborate in future development.

That's a good argument.

I think one thing that might really persuade the phpseclib maintainer is if phpseclib got some additional developers or people providing support. I'm trying to help with the documentation but documentation only helps so much and when I'm done with it I'm pretty much done and phpseclib will go back to being pretty much a one-person shop (as if it isn't already).

My proposal would be to fork phpseclib, make it support PSR-0 and add a whole slew of other features that make the forks features so good that the dev can't refuse them. Maybe ask him for what he'd like and help him out with it. If the maintainer didn't have to stand-alone maybe he wouldn't be so steadfast.

*sigh* not that I really expect anyone to do that. I think phpseclib's most likely fate is this: it'll be abandoned at some point and multiple forks will arise with no clear successor and whatever legacy it - and my documentation could have had - will be lost. Oh well..

Ah yes. More of the Drupal Way(tm) is the only way. If you're not a Drupal developer you're a bad developer! Drupal group think ftw!

I guess you people favor the stick over the carrot. Guess I shouldn't be surprised. This gunboat diplomacy approach you all seem to prefer got most hosts to finally upgrade to PHP5, which has been a plus, for sure. But I guess it's not just hosts you all bully is it?

This is not an efficient or effective way of making your point. You are demonizing an entire community of developers based on the comments of a few. And you are demonizing them based on reactions to the fact that we are trying to figure out ways to working around impediments of interoperating with another php library.

Instead of being hurtful, it would be better to find ways to open a dialog between our communties so we can share perspectives and talk this through.

Within the Drupal community we have a relatively new experience of working with other PHP projects (although we work closely with jQuery projects too). Let me point your attention to how we are adopting Symfony 2 components to drive key functionality in Drupal 8. Already, Drupal developers are contributing patches back to Symfony to fix bugs. We are in frequent communication and work closely with Symfony developers so that we can gain a high level of expertise with Symfony. These are investments in knowledge that will pay off in the future whenever we look to solve problems that Symfony does a good job of solving.

Imagine what would happen if a just a few Drupal developers were to gain that level of expertise with the phpseclib library? The core developer of that library wouldn't have do all the heavy lifting for the rest of eternity.

Besides, collaborating with others can be a wonderously educational, productive, and fun experience. I don't know why one would turn that down.

Fair enough. I apologize.

Also, I didn't see the "collaborate in future development." bit you had written in your last post. I think that's what'd help the most the most. But I think what might help more is showing that the Drupal community is willing to collaborate in current development as well.

Like I said, add PSR-0 support and then add other stuff. Fork it and try to work with the developer. But do more than just add PSR-0 support. PSR-0 support is pretty easy to add - helping develope the codebase in other ways will get the maintainers attention a lot more I think.

edit: I wrote this post before I saw the other one lol.

Anyway, I think your line of reasoning is going to be one the phpseclib developer is much more sympathetic too. I respect your diplomatic approach to the situation!

Woah. There was a proposal to integrate this library into Drupal core. There were some technical hurdles to integrating the library. Various people in here asked "can we collaborate?" and your answer was:

I don't think the author is gonna accept that commit unless you pay him, per this:

http://www.frostjedi.com/phpbb/viewtopic.php?f=46&t=32804

The second post says he'll make PHP 5.3 only (which is what PSR-0 requires) for $25k.

That's the attitude I'm concerned about. I'm not saying he's wrong and we're right, that it's our way or the highway, whatever. I'm just pointing out that it seems like it's going to be hard to collaborate in a mutually beneficial way if the answer is, in effect: "fork my code and good luck, or pay me top dollar for something I don't really agree with, anyway".

And again, I don't see the actual gain in security for most end users to be worth the effort. That's all I'm saying. I'm not personally attacking anyone, nor doubting their abilities. I haven't looked at any of the code. I'm just going on what you're saying and linking to here. And that paints a picture of a project that seems difficult to collaborate with. And I'd rather see us spend our effort on other things. That is all. No one is bullying anyone here.

Cheers,
-Derek

If someone has HTTPS access to their site, I'm assuming they also have SSH access. It might be easier for them to just SSH in and use Drush to update their modules. If this is a feature that people actually want, then it might be best to push that off to contrib. This seems like a support nightmare for Drupal Core.

Therefore, I'm a -1 on this, but I'm still willing to help out an open source project! Instead of donating $25K though, I'll put up a pull request for PHP5 autoloading and Composer support.

FWIW I don't think the bounties the phpseclib maintainer posted should be considered gospel. He also said he'd only switch to github.com before his next release if he was paid $1k and yet he still moved over before his next release. *shrug*

rewritten