[stunnel-users] Fwd: Comments/questions to Debian patches

[Peter Pentchev]

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 at mirt.net>
> To: Peter Pentchev <roam at 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

-- 
Peter Pentchev  roam at ringlet.net roam at FreeBSD.org pp at storpool.com
PGP key:        http://people.FreeBSD.org/~roam/roam.key.asc
Key fingerprint 2EE7 A7A5 17FC 124C F115  C354 651E EFB0 2527 DF13
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://www.stunnel.org/pipermail/stunnel-users/attachments/20150423/d215785c/attachment.sig>