-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Dear Users,
Peter Pentchev, the Debian package maintainer for the "stunnel4" package, does not reply to my emails. I hope he is still alive and well, just too busy to maintain the package.
I decided to share with you my comments to the patches that are applied to the Debian package. Hopefully, someone will find them useful.
Mike
- -------- Forwarded Message -------- Subject: Comments/questions to Debian patches Date: Tue, 28 Oct 2014 21:13:25 +0100 From: Michal Trojnara Michal.Trojnara@mirt.net To: Peter Pentchev roam@ringlet.net
Hi Peter,
Just a few comments/questions to improve the quality of Debian package. I'll be glad to discuss if you disagree with my opinions.
01-fix-paths.patch
The patch description is quite outdated. Translation from sbin to bin was performed upstream in stunnel 4.21 released 27 Oct 2007. 8-)
I guess: /usr/bin/stunnel -fd 10 \ should be: /usr/bin/stunnel4 -fd 10 \ Probably this should be added to the next patch: 02-rename-binary.patch
05-logrotate-warning-in-sample-conf.patch
Good idea. I'll add it to stunnel 5.07.
08-client-example.patch
I've already added this example in stunnel 5.02. Your patch adds it once again. Just remove it.
10-no-zlib-compression.patch
I'm completely confused by this patch. According to my tests it only makes stunnel reporting different errors when a user tries to enable compression on Debian. Why would anyone need this patch?
11-no-rle-compression.patch
IMHO OpenSSL bugs should be fixed in OpenSSL, and not in stunnel. YMMV
12-restore-pidfile-default.patch
I strongly disagree with this approach, as it breaks configuration file compatibility with the upstream. Debian should instead rewrite stunnel.conf when upgrading from stunnel 4.xx.
14-lsb-init-functions.patch
8-)
15-upstream-systemd-libs.patch
This (and more) will be included in stunnel 5.07.
16-upstream-sslv23-method.patch
This will be included in stunnel 5.07.
Mike
On Tue, Apr 21, 2015 at 11:26:25PM +0200, Michal Trojnara wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Dear Users,
Peter Pentchev, the Debian package maintainer for the "stunnel4" package, does not reply to my emails. I hope he is still alive and well, just too busy to maintain the package.
Right... many, many apologies for that :( Yes, I've been meaning to reply to both your mails for... yeah, for some months now :(
I decided to share with you my comments to the patches that are applied to the Debian package. Hopefully, someone will find them useful.
Mike
- -------- Forwarded Message --------
Subject: Comments/questions to Debian patches Date: Tue, 28 Oct 2014 21:13:25 +0100 From: Michal Trojnara Michal.Trojnara@mirt.net To: Peter Pentchev roam@ringlet.net
Hi Peter,
Just a few comments/questions to improve the quality of Debian package. I'll be glad to discuss if you disagree with my opinions.
01-fix-paths.patch
The patch description is quite outdated. Translation from sbin to bin was performed upstream in stunnel 4.21 released 27 Oct 2007. 8-)
Oof, right, that makes sense, thanks :) I'll reword it a bit.
I guess: /usr/bin/stunnel -fd 10 \ should be: /usr/bin/stunnel4 -fd 10 \ Probably this should be added to the next patch: 02-rename-binary.patch
Grrr, of course it should. Good catch!
05-logrotate-warning-in-sample-conf.patch
Good idea. I'll add it to stunnel 5.07.
Right, I see that you've commented the entries out. We may still keep the "remember to update the logrotate configuration too" warning, we'll see.
08-client-example.patch
I've already added this example in stunnel 5.02. Your patch adds it once again. Just remove it.
True, I noticed that some time back, but the package was already released. Stupid of me :) I'll drop it.
10-no-zlib-compression.patch
I'm completely confused by this patch. According to my tests it only makes stunnel reporting different errors when a user tries to enable compression on Debian. Why would anyone need this patch?
Well, the problem is that stunnel unconditionally adds zlib to the compression stack, no matter what compression algorithm the user has actually chosen. Right now, OpenSSL does not provide any other compression methods, just "zlib" and "rle", but if the user has her own OpenSSL version with a custom compression algorithm built in or added as some sort of extension, trying to use it would fail because stunnel would first try to add "zlib" - and that's not present on Debian.
This patch's purpose is mainly the ssl.c change, the "de-mandatorizing" of COMP_zlib(). The change to the configuration parser is more of a convenience - display an informative error message to the user about the Debian-specific reason that her choice of zlib would not work. That's the main reason why I haven't forwarded this particular patch to you yet - I do believe it to be a Debian-specific thing, and I do think that it's useful for Debian.
11-no-rle-compression.patch
IMHO OpenSSL bugs should be fixed in OpenSSL, and not in stunnel. YMMV
You have a point here, but the error was a bit obscure to diagnose - there were no meaningful error messages, there was nothing to really point out to the bug in OpenSSL's RLE code. IMHO that patch served mostly to provide a more human-readable description of the problem.
That said, wow, they actually fixed it somewhere in OpenSSL 1.0.1... okay, this patch is going away, since run-length compression does work now.
12-restore-pidfile-default.patch
I strongly disagree with this approach, as it breaks configuration file compatibility with the upstream. Debian should instead rewrite stunnel.conf when upgrading from stunnel 4.xx.
I agree with your strong disagreement with this patch. I was kind of uncomfortable adding it in the first place, but the point was to not break compatibility and, well, yes, to avoid rewriting the configuration file in a possibly buggy automated manner :/
My intention has always been to only keep this patch for one Debian release cycle, adding a warning during the system startup so that people might actually see it. Now that Debian 8 (Jessie) is scheduled to be released this weekend (here's hoping!), I will remove the patch in the next upload of stunnel that will target Debian unstable/testing. However, with all due respect (and I do really respect your opinion on this and pretty much all other stunnel- or security-related matters :), the stunnel version in Jessie will have the patch.
14-lsb-init-functions.patch
8-)
15-upstream-systemd-libs.patch
This (and more) will be included in stunnel 5.07.
16-upstream-sslv23-method.patch
This will be included in stunnel 5.07.
Right, these will be dropped in my upcoming update to stunnel-5.16 after Jessie is released.
Thanks a lot, and once again, apologies for my silence :(
G'luck, Peter
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Hi Peter,
10-no-zlib-compression.patch
I'm completely confused by this patch. According to my tests it only makes stunnel reporting different errors when a user tries to enable compression on Debian. Why would anyone need this patch?
Well, the problem is that stunnel unconditionally adds zlib to the compression stack, no matter what compression algorithm the user has actually chosen. Right now, OpenSSL does not provide any other compression methods, just "zlib" and "rle", but if the user has her own OpenSSL version with a custom compression algorithm built in or added as some sort of extension, trying to use it would fail because stunnel would first try to add "zlib" - and that's not present on Debian.
This patch's purpose is mainly the ssl.c change, the "de-mandatorizing" of COMP_zlib(). The change to the configuration parser is more of a convenience - display an informative error message to the user about the Debian-specific reason that her choice of zlib would not work. That's the main reason why I haven't forwarded this particular patch to you yet - I do believe it to be a Debian-specific thing, and I do think that it's useful for Debian.
So the solution that might be merged upstream would be to somehow detect if the OpenSSL library was modified to remove zlib support, and to remove the dependency in this case. Am I right?
12-restore-pidfile-default.patch
I strongly disagree with this approach, as it breaks configuration file compatibility with the upstream. Debian should instead rewrite stunnel.conf when upgrading from stunnel 4.xx.
I agree with your strong disagreement with this patch. I was kind of uncomfortable adding it in the first place, but the point was to not break compatibility and, well, yes, to avoid rewriting the configuration file in a possibly buggy automated manner :/
My intention has always been to only keep this patch for one Debian release cycle, adding a warning during the system startup so that people might actually see it. Now that Debian 8 (Jessie) is scheduled to be released this weekend (here's hoping!), I will remove the patch in the next upload of stunnel that will target Debian unstable/testing. However, with all due respect (and I do really respect your opinion on this and pretty much all other stunnel- or security-related matters :), the stunnel version in Jessie will have the patch.
I see your point. The problem is that users may expect 5.x behavior when stunnel 5.x is installed, and not the backported 4.x behavior.
Backporting 4.x behavior is probably fine as long as it is clearly documented on the manual page.
Right, these will be dropped in my upcoming update to stunnel-5.16 after Jessie is released.
Just be aware of the critical security vulnerability that allows certificate-based access control to be completely bypassed when the "redirect" option is used. Versions 5.00 to 5.12 are vulnerable.
Other important bugfixes include a fix for truncated connections in stunnel 5.09, a fix for handling multiple connect/redirect destinations in stunnel 5.14, and memory leak fixes in stunnel 5.15.
Adopting stunnel 5.16 might be easier than backporting those bugfixes.
Thanks a lot, and once again, apologies for my silence :(
I hope our communication will get better in the future.
Best regards, Mike
-
Michal Trojnara -
Peter Pentchev