zlib in PPP code #374
Comments
|
I added some functions to zlib in order to be able to support "deflate" packet compression more efficiently. If you try to use a standard zlib it won't work properly. If it was possible to use the standard zlib I would have done that from the beginning. |
|
@paulusmack: It has not been integrated since this very old version? If not, maybe good to add a PR here: |
|
The zlib in common is only used for the Solaris kernel modules. I'll ask @carlsonj if it is still needed. There is also a zlib in the pppdump directory which is even older. I don't want getting stuff merged into upstream zlib to be on the critical path for getting a release out. |
|
On 10/27/22 01:42, Paul Mackerras wrote:
The zlib in common is only used for the Solaris kernel modules. I'll ask
@carlsonj <https://github.com/carlsonj> if it is still needed.
There is also a zlib in the pppdump directory which is even older.
I don't want getting stuff merged into upstream zlib to be on the
critical path for getting a release out.
It's needed if the BSD compress algorithm is used on a custom-compiled
version of PPP on Solaris. I don't know of any other source for that in
the Solaris kernel, so removing it would likely disable that feature.
But, on the other hand, I don't know of any serious use of that data
compression feature, so probably not a particularly high priority. Not
really wedded to it.
…--
James Carlson 42.703N 71.076W FN42lq08 ***@***.***>
|
|
I don't mind leaving common/zlib.c there if there is a chance it is still useful to somebody. However, unless someone finds a vulnerability in it, I don't feel like putting in effort to update it either. @Neustradamus note that this code is required in source form here since it is needed for compiling a kernel module. We can't use a system library for this. |
|
@paulusmack: Maybe good to update the code? |
|
I took out the zlib code in pppdump. As to the version for the Solaris kernel modules, I'll let @carlsonj say what to do with it (default to just leaving it as is). |
|
@paulusmack, @carlsonj: The problem is that this zlib code is not secure :/ James, please do an answer, it is not easy to have from you :/ |
What exactly is the insecurity? I know there was a potential out-of-bounds access in the pppdump version of zlib.c, but I thought the version in common/ (which is only used in the Solaris kernel module) was a bit newer, and I don't know if the same problem was found in it. Do you have a reference to a problem in that specific code? In any case, since it's only used on Solaris, it's not a problem for the majority of our user base these days. |
|
@paulusmack: There are CVEs in old Zlib versions: https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=zlib |
|
On 8/22/24 08:43, Neustradamus wrote:
@paulusmack <https://github.com/paulusmack>: There are CVEs in old Zlib
versions: https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=zlib
<https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=zlib>
"There are CVEs" is probably one of the least compelling arguments,
especially given that a quick look over the list shows that they involve
features that are not in use by PPP Deflate compression or are in
applications that have bugs in their own use of zlib.
But actually determining whether any of those CVEs in some manner apply
to anything that PPP might ever do is a substantial amount of detailed
work, and RFC 1979 Deflate was never what one might call "popular" on
any platform. We tested the heck out of it at Sun, putting vast
quantities of data through it, but I don't recall anyone ever using it
on purpose.
I don't have any strong feelings about keeping it at all, but I do think
if you've read this far, you probably have already put way too much work
into this.
…--
James Carlson 42.703N 71.076W FN42lq08 ***@***.***>
|
|
@carlsonj is there still any need to distribute kernel module source for Solaris in this package? If the Solaris kernel source includes the PPP modules then I would think they are probably more up to date than what we have here, so we should just delete the copy in this project. I removed the Linux kernel module source some time ago from this project (in 2008, in fact). I think we could remove modules/* and common/*. None of those files have seen any real changes since 2004, as far as I can tell. Do you agree? |
|
Oh and of course solaris/*, which hasn't had any real changes since 2005. |
|
On 8/22/24 20:15, Paul Mackerras wrote:
@carlsonj <https://github.com/carlsonj> is there still any need to
distribute kernel module source for Solaris in this package? If the
Solaris kernel source includes the PPP modules then I would think they
are probably more up to date than what we have here, so we should just
delete the copy in this project. I removed the Linux kernel module
source some time ago from this project (in 2008, in fact).
I think we could remove modules/* and common/*. None of those files have
seen any real changes since 2004, as far as I can tell. Do you agree?
The only reason I could see for keeping them around would be as an
example of how to implement the modules in STREAMS. That's not at all a
very strong reason, though.
They're still functional, but I agree that they're definitely not
recently updated and that they nominally target an OS that Oracle itself
has all but abandoned. (I'm pretty sure that at one point in time, those
same modules were used on other SVr4 systems like ICL. But, wow, that
was a very long time ago, and I wouldn't be surprised to find some
deviations now.)
I checked with Illumos, which is the remaining OpenSolaris fork that's
in active development (and that is the upstream for multiple different
distributions), and it still has the forked Solaris-native drivers:
https://github.com/illumos/illumos-gate/tree/master/usr/src/uts/common/io/ppp
That includes the native drivers to support other things like PPPoE.
As I recall, we tried hard to maintain the same basic ioctl structure
with the upstream source. I'd have to look carefully at sys-solaris.c to
be able to tell whether or not it'd work with those forked drivers
directly. My guess would be that if any changes are needed at all,
they'd be trivial.
So, if you want to yank them out, feel free.
…--
James Carlson 42.703N 71.076W FN42lq08 ***@***.***>
|
Thanks. Could you have a look at README.sol2 and tell me if anything in there is still useful, please? It was last updated 22 years ago. :/ |
|
On 9/10/24 08:18, Paul Mackerras wrote:
So, if you want to yank them out, feel free.
Thanks.
Could you have a look at README.sol2 and tell me if anything in there is
still useful, please? It was last updated 22 years ago. :/
I see that it's definitely grown stale. I'll take some time and do a
little updating. If you're close to shipping a new version right now,
then don't wait on me. I think I want to look into the driver
compatibility issue (if there indeed is one; I recall that we were just
"worried" about it at the time of of the fork) so that maybe the
solaris/ subdirectory can go as well. There are still Solaris
derivatives around, but there are likely few other STREAMS users out
there these days, and fewer still who really need a fully up-to-date
version of pppd.
…--
James Carlson 42.703N 71.076W FN42lq08 ***@***.***>
|
Great, thanks. If you could also look at PR #515 and check that I'm not removing anything you think we should still distribute, that would be appreciated. |
|
On 9/10/24 20:24, Paul Mackerras wrote:
I see that it's definitely grown stale. I'll take some time and do a
little updating. If you're close to shipping a new version right
now, then don't wait on me. I think I want to look into the driver
compatibility issue (if there indeed is one; I recall that we were
just "worried" about it at the time of of the fork) so that maybe
the solaris/ subdirectory can go as well. There are still Solaris
derivatives around, but there are likely few other STREAMS users out
there these days, and fewer still who really need a fully up-to-date
version of pppd.
Great, thanks.
If you could also look at PR #515
<#515> and check that I'm not
removing anything you think we should still distribute, that would be
appreciated.
That looks comprehensive to me. As I recall the modules/ directory was
just a generic STREAMS implementation that you could use on ancient
machines such as OSF/1. The solaris/ directory was a contribution back
from Sun with a bunch of bug fixes (but without testing on those other
platforms, so likely "Solaris only"). Diffs show just a few changes.
Anyway, ditching it seems fine, and I know of no dependencies with the
rest of the code.
I intend to look into pppd/sys-solaris.c and see what can be done (if
anything) to make it reasonably compatible with the native drivers. The
only thing I remember changing in that area was unit number ("pppN")
selection mechanics.
…--
James Carlson 42.703N 71.076W FN42lq08 ***@***.***>
|
|
The zlib code has been removed. |
There is zlib in /common:
There is zlib in /pppdump:
It is needed to update or to delete from the code?
The text was updated successfully, but these errors were encountered: