Hi!
I've checked in the patch for https://bugs.php.net/bug.php?id=77153,
which disables by default rsh/ssh login functionality in IMAP. I assume
most people neither know such functionality existed nor need it, but
still it's a BC break. The reason why I did it is because IMAP library
does not validate mailbox parameters it sends to the underlying shell
commands, which creates all kinds of unpleasant security scenarios (see
bug for details).
Strictly speaking, such bug is a problem in the library, not PHP
wrapper, since all parsing and mailbox string handling is done inside
the library and it completely opaque to PHP. However, c-client library
has been essentially unsupported for many years (why we're using an
ancient unsupported library is a separate issue which we'd probably want
to address but let's not get distracted) so no fix is probably coming
from that direction. And since imap extension is used by a bunch of
tools and most are not aware underlying library has this vulnerability,
I think disabling this function is a right thing to do. More details in
the bug and in the UPGRADING note.
I've merged patch now since the issue is public (essentially has been
for a while, and was first submitted as
https://bugs.php.net/bug.php?id=76428 but at the time I haven't realized
c-client is not going to be fixed, which is my fault - should have
checked the status of this library). Despite it not being a PHP issue
per se, I think we may still want a CVE for it.
For RMs, please incorporate it into the next release. Maybe not that
urgent for PHP 7.3.0RC6 since it's not a production release anyway.
Please comment if you see any troubles or have any questions about the fix.
Stas Malyshev
smalyshev@gmail.com
Strictly speaking, such bug is a problem in the library, not PHP
wrapper, since all parsing and mailbox string handling is done inside
the library and it completely opaque to PHP. However, c-client library
has been essentially unsupported for many years (why we're using an
ancient unsupported library is a separate issue which we'd probably want
to address but let's not get distracted) so no fix is probably coming
from that direction. And since imap extension is used by a bunch of
tools and most are not aware underlying library has this vulnerability,
I think disabling this function is a right thing to do. More details in
the bug and in the UPGRADING note.
I fully agree with the fix (thanks!), and also that it is a security
issue. However, I don't think it's really a problem in c-client;
actually the PHP wrapper should not have allowed to pass the mailbox
name verbatim, which would only be reasonable in my opinion, if we were
supporting arbitrary drivers (which we don't). And of course, userland
clients should not pass unvalidated input as mailbox name, but as you
said, quite likely at least some developers are not aware that
potentially arbitrary shell commands could be executed this way, and our
docs don't explicitly mention this issue.
For RMs, please incorporate it into the next release. Maybe not that
urgent for PHP 7.3.0RC6 since it's not a production release anyway.
PHP-7.3.0 has already been branched, and PHP-7.2.13RC1 and PHP-7.3.0RC6
have already been tagged without the patch. We probably should re-tag.
--
Christoph M. Becker
Hi!
actually the PHP wrapper should not have allowed to pass the mailbox
name verbatim, which would only be reasonable in my opinion, if we were
supporting arbitrary drivers (which we don't). And of course, userland
PHP does not do anything with mailbox names. In fact, PHP doesn't even
know what mailbox name is (it just passes opaque string to c-client and
hopes c-client can make sense of it, whatever that string does). Thus,
we support everything c-client supports, AFAIK. We could extract the
whole parsing logic from c-client and re-implement it, but I don't think
it's worth the time spent.
--
Stas Malyshev
smalyshev@gmail.com
Hi!
actually the PHP wrapper should not have allowed to pass the mailbox
name verbatim, which would only be reasonable in my opinion, if we were
supporting arbitrary drivers (which we don't). And of course, userlandPHP does not do anything with mailbox names. In fact, PHP doesn't even
know what mailbox name is (it just passes opaque string to c-client and
hopes c-client can make sense of it, whatever that string does). Thus,
we support everything c-client supports, AFAIK. We could extract the
whole parsing logic from c-client and re-implement it, but I don't think
it's worth the time spent.
Btw, is imap on the list to deprecate in 7.x + kill in 8.x? It is really
not maintained well, both c-client and the ext. Would it be possible to
consider it?
best,
Pierre
Den ons. 21. nov. 2018 kl. 06.13 skrev Pierre Joye pierre.php@gmail.com:
Btw, is imap on the list to deprecate in 7.x + kill in 8.x? It is really
not maintained well, both c-client and the ext. Would it be possible to
consider it?
I remember we have spoken about this in the past and it seems that
everytime it comes up, is the argument that we don't provide a real
mail alternative. I mean sure there is lots of userland based
implementations and all, but I still think that PHP should provide
something out of the box, but I do agree that at one point, the
security concerns will start to vastly outweight the unmaintained
library, but its really something that should be discussed for a
future release.
--
regards,
Kalle Sommer Nielsen
kalle@php.net
Den ons. 21. nov. 2018 kl. 06.13 skrev Pierre Joye pierre.php@gmail.com:
Btw, is imap on the list to deprecate in 7.x + kill in 8.x? It is really
not maintained well, both c-client and the ext. Would it be possible to
consider it?I remember we have spoken about this in the past and it seems that
everytime it comes up, is the argument that we don't provide a real
mail alternative. I mean sure there is lots of userland based
implementations and all, but I still think that PHP should provide
something out of the box, but I do agree that at one point, the
security concerns will start to vastly outweight the unmaintained
library, but its really something that should be discussed for a
future release.
To me it is not logical to continue maintaining a wrapper to an
abandoned library just so the functions continue to exist for the sake
of existing.
That seems like a bad policy.
Import the library into core and maintain it or write an alternate from
scratch - IF the functions it provides truly has market demand.
Otherwise remove support. Those seem to me like the only logical
options. Nostalgia is not logical.
I just checked my roundcube install, it does not use this wrapper, so
clearly IMAP connectivity can be obtained through other means.
That makes me feel inclined to believe this wrapper is not needed.
I don't vote, but PHP shouldn't be like OpenSSL was with things like
heartbeat - an extension almost no one used and seemingly no one
maintained yet it was everywhere and vulnerable.
Maybe stick the IMAP wrapper in PECL if someone needs it badly enough to
be willing to maintain the wrapper there.
Less is more, more or less.
Hi!
Import the library into core and maintain it or write an alternate from
scratch - IF the functions it provides truly has market demand.
Nice suggestion if we had a line of volunteers at our doors asking us to
give them something to maintain. We don't. So this is not an option,
it's a happy dream. Realistic option is imap support that we have now or
no imap support at all. Market demand for functionality is always way
exceeds market supply of maintainers, especially long-term maintainers.
If we have somebody coming forward and volunteering to write new imap
extension (or take over c-client maintainership) - excellent, problem
solved. But I don't see it happening now.
Otherwise remove support. Those seem to me like the only logical
options. Nostalgia is not logical.
Functionality is logical. We don't have imap module because people need
to feel nice about keeping imap module. We have imap module because imap
functionality is used. There's a lot of PHP packages using imap on
github, if you look. Can they switch? Probably. But let's not pretend
that the situation is we're having this module in core out of pure
nostalgia. We have it because it's functionality that is needed by people.
--
Stas Malyshev
smalyshev@gmail.com
Den ons. 21. nov. 2018 kl. 06.13 skrev Pierre Joye pierre.php@gmail.com:
Btw, is imap on the list to deprecate in 7.x + kill in 8.x? It is really
not maintained well, both c-client and the ext. Would it be possible to
consider it?I remember we have spoken about this in the past and it seems that
everytime it comes up, is the argument that we don't provide a real
mail alternative. I mean sure there is lots of userland based
implementations and all, but I still think that PHP should provide
something out of the box, but I do agree that at one point, the
security concerns will start to vastly outweight the unmaintained
library, but its really something that should be discussed for a
future release.
One particular issue wrt. removing ext/imap is that the mail
testsuite[1] relies on this extension on Windows, so the tests would
have to be rewritten first.
[1]
https://github.com/php/php-src/tree/php-7.3.0RC5/ext/standard/tests/mail
--
Christoph M. Becker
Am 21.11.2018 um 06:13 schrieb Pierre Joye:
actually the PHP wrapper should not have allowed to pass the mailbox
name verbatim, which would only be reasonable in my opinion, if we were
supporting arbitrary drivers (which we don't). And of course, userland
PHP does not do anything with mailbox names. In fact, PHP doesn't even
know what mailbox name is (it just passes opaque string to c-client and
hopes c-client can make sense of it, whatever that string does). Thus,
we support everything c-client supports, AFAIK. We could extract the
whole parsing logic from c-client and re-implement it, but I don't think
it's worth the time spent.Btw, is imap on the list to deprecate in 7.x + kill in 8.x? It is really
not maintained well, both c-client and the ext. Would it be possible to
consider it?
There was a vote in 2015 if ext/imap should be removed from core:
https://wiki.php.net/rfc/removal_of_dead_sapis_and_exts#extimap
Michael
actually the PHP wrapper should not have allowed to pass the mailbox
name verbatim, which would only be reasonable in my opinion, if we were
supporting arbitrary drivers (which we don't). And of course, userlandPHP does not do anything with mailbox names. In fact, PHP doesn't even
know what mailbox name is (it just passes opaque string to c-client and
hopes c-client can make sense of it, whatever that string does). Thus,
we support everything c-client supports, AFAIK. We could extract the
whole parsing logic from c-client and re-implement it, but I don't think
it's worth the time spent.
libc-client supports arbitrary drivers[1], but the PHP wrapper does
not[2] (presumably for good reasons[3]). Thus, the PHP wrapper could
have expected an array (or some such) instead of a string, assembling
the mailbox name from white-listed elements. This would have been a
much cleaner and more robust API.
Anyhow, this is water under the bridge now, and I think we should issue
a call for maintainership[4] for ext/imap as soon as possible, since
this is not the only issue[5] of this unmaintained[6] extension.
[1] see docs/drivers.txt in the imap-2007f distribution
[2]
https://github.com/php/php-src/blob/php-7.3.0RC5/ext/imap/php_imap.c#L845-L859
[3] see the mail_link() docs in docs/internal.txt
[4] https://wiki.php.net/rfc/umaintained_extensions#proposal
[5]
https://bugs.php.net/search.php?cmd=display&package_name[]=IMAP+related&direction=DESC&limit=30&status=Open
[6]
https://github.com/php/php-src/blob/5a5f3617558b7a278a5c73652087b3d89927ccba/EXTENSIONS#L265
--
Christoph M. Becker
Hi!
Anyhow, this is water under the bridge now, and I think we should issue
a call for maintainership[4] for ext/imap as soon as possible, since
this is not the only issue[5] of this unmaintained[6] extension.
Pierre is listed as maintainer. But given that he is for deprecating it,
I guess we try to find somebody who is willing to revive it (not likely)
and failing that (likely) go the deprecation route probably... Given how
many references to it I see on github though, it's not the best outcome.
--
Stas Malyshev
smalyshev@gmail.com
On Wed, Nov 21, 2018 at 7:27 PM Stanislav Malyshev smalyshev@gmail.com
wrote:
Hi!
Anyhow, this is water under the bridge now, and I think we should issue
a call for maintainership[4] for ext/imap as soon as possible, since
this is not the only issue[5] of this unmaintained[6] extension.Pierre is listed as maintainer. But given that he is for deprecating it,
I guess we try to find somebody who is willing to revive it (not likely)
and failing that (likely) go the deprecation route probably... Given how
many references to it I see on github though, it's not the best outcome.
I'd like to take this one, too.
RoundCube may roll its own, but I know the popular Horde IMP uses this
extensively. I suspect that the extension's going to have the performance
one would desire when dealing with giant mailboxes, but c-client is hard to
deal with. I've had experience with c-client for a while (stretching back
to Pine days), and yet still would like to roadmap our way to a modern
implementation.
bishop
Hi!
Hi! > Anyhow, this is water under the bridge now, and I think we should issue > a call for maintainership[4] for ext/imap as soon as possible, since > this is not the only issue[5] of this unmaintained[6] extension. Pierre is listed as maintainer. But given that he is for deprecating it, I guess we try to find somebody who is willing to revive it (not likely) and failing that (likely) go the deprecation route probably... Given how many references to it I see on github though, it's not the best outcome.I'd like to take this one, too.
That's be great. I see there are 57 open bugs for imap, triaging them
would be a good start.
RoundCube may roll its own, but I know the popular Horde IMP uses this
extensively. I suspect that the extension's going to have the
performance one would desire when dealing with giant mailboxes, but
c-client is hard to deal with. I've had experience with c-client for a
while (stretching back to Pine days), and yet still would like to
roadmap our way to a modern implementation.
Migrating to something more modern (and not dead :) would definitely be
great. May be not that easy though given that one has to keep both APIs
and mailbox formats (at least to some measure) for it to be useful.
Alternative imap implementation may be a good route too...
Stas Malyshev
smalyshev@gmail.com
Zitat von Bishop Bettini bishop@php.net:
On Wed, Nov 21, 2018 at 7:27 PM Stanislav Malyshev smalyshev@gmail.com
wrote:Hi!
Anyhow, this is water under the bridge now, and I think we should issue
a call for maintainership[4] for ext/imap as soon as possible, since
this is not the only issue[5] of this unmaintained[6] extension.Pierre is listed as maintainer. But given that he is for deprecating it,
I guess we try to find somebody who is willing to revive it (not likely)
and failing that (likely) go the deprecation route probably... Given how
many references to it I see on github though, it's not the best outcome.I'd like to take this one, too.
RoundCube may roll its own, but I know the popular Horde IMP uses this
extensively.
Horde isn't using ext/imap since ages, but has developed a PHP-only
IMAP library long ago, that's being used by many other OSS projects too.
I suspect that the extension's going to have the performance
one would desire when dealing with giant mailboxes, but c-client is hard to
deal with. I've had experience with c-client for a while (stretching back
to Pine days), and yet still would like to roadmap our way to a modern
implementation.bishop
--
Jan Schneider
The Horde Project
https://www.horde.org/
Zitat von Bishop Bettini bishop@php.net:
On Wed, Nov 21, 2018 at 7:27 PM Stanislav Malyshev smalyshev@gmail.com
wrote:Hi!
Anyhow, this is water under the bridge now, and I think we should
issue
a call for maintainership[4] for ext/imap as soon as possible, since
this is not the only issue[5] of this unmaintained[6] extension.Pierre is listed as maintainer. But given that he is for deprecating it,
I guess we try to find somebody who is willing to revive it (not likely)
and failing that (likely) go the deprecation route probably... Given how
many references to it I see on github though, it's not the best outcome.I'd like to take this one, too.
RoundCube may roll its own, but I know the popular Horde IMP uses this
extensively.Horde isn't using ext/imap since ages, but has developed a PHP-only
IMAP library long ago, that's being used by many other OSS projects too.
I appreciate the clarification, Jan, thanks. I must have been reading the
h3 docs, instead of current. o.O
The library is indeed very nice, https://github.com/horde/Imap_Client/
Looks like it's faster than native ext/imap, too
https://dev.horde.org/imap_client/
One recent SO answer comparing ext/imap and Horde Imap_Client:
https://stackoverflow.com/q/6659409/2908724
Given the high quality userland option(s) out there, it seems a waste of
effort to revitalize the extension with a fresh imap implementation. We
should be fixing the bugs though, and I'll focus on that. For now let's
just call it actively maintained, but feature frozen?
bishop