Package: polipo
Version: 1.0.4-1

Denis,

You apply the following patch to Debian's polipo:

++++ polipo-1.0.4/log.c
+@@ -80,7 +80,9 @@
+ 
+     if(logFile != NULL && logFile->length > 0) {
+         FILE *f;
++      mode_t mask = umask(026);
+         f = fopen(logFile->string, "a");
++      umask(mask);
+         if(f == NULL) {
+             do_log_error(L_ERROR, errno, "Couldn't open log file %s",
+                          logFile->string);
+@@ -340,7 +342,9 @@
+ {
+     if(logFile) {
+         FILE *f;
++      mode_t mask = umask(026);
+         f = fopen(logFile->string, "a");
++      umask(mask);
+         if(f == NULL) {
+             do_log_error(L_ERROR, errno, "Couldn't reopen log file %s",
+                          logFile->string);

I have thought it over, and I hereby requests that you revert this
patch.  There are just too many reasons why you should not have
applied it.

1. You did apply a patch relating to security without an explicit ack
From upstream.

I hope it is clear from the recent OpenSSL debacle why this must not
be done.

2. You did apply a patch without first trying to get it applied upstream.

You did send me the patch, but only after you applied it.  You should
only ever apply a patch *after* the patch was rejected upstream *and*
you fully understand the reasons why, and believe that these reasons
do not apply to Debian.

Sorry to beat a dead horse, but taking this approach would have
avoided the recent OpenSSL debacle.

3. You changed Polipo's behaviour without documenting it

The Debian binary of Polipo now behaves differently from the upstream
binary.  This will confuse your users and will confuse your friendly
upstream when he tries to help your users with debugging.

What is more, it will create a rather glaring security hole for anyone
who replaces his Debian binary with an upstream binary (which is
something people sometimes do, when they need a more recent version).

4. Your patch, while technically correct, will lead to bugs in the future.

Your patch manipulates the process' *user* mask, which must never be
manipulated by a program.  The umask is a global process feature.

It will cause a rather glaring security hole if I ever decide to use
threads in Polipo.

The proper way to do what you need is to use open(O_CREAT) with the
right permissions (but obeying the umask), then pass the file
descriptor to fdopen.  Of course, the permissions should be
configurable by a config variable.

Any one of the above reasons is enough to ask you to revert this
patch.

Regards,

                                        Juliusz

Attachment: pgpvidOJwf7AS.pgp
Description: PGP signature

Reply via email to