|
From: Christian B. <pos...@cb...> - 2013-08-18 19:03:22
|
Hello,
Am Dienstag, 30. Juli 2013 schrieb Lars Engelhard:
> The MySQL table fetchmail (of database postfixadmin) was enhanced by
> some attributes - you may check those in detail below this message.
> The GUI was extended and the ADDITION/fetchmail.pl is now fetching the
> newly added attributes and using them to either do a MD5 fingerprint
> SSL check (for those servers with self-signed-certificates) or do a
> SSL certificate validation. For SSL certificate validation, the
> certificate path can be set directly.
>
> We thought that you may like to add these features in your official
> project to increase security through SSL using fetchmail.
Yes, those additions make sense :-) - thanks for your patch!
In general your changes look good, but there are some details I
modified:
In fetchmail.pl, you added
$cmd.=" dropdelivered";
to the fetchmail config which seems unrelated and is not a good idea for
general usage (it drops "Delivered-To" headers).
You also added "-L /var/log/fetchmail" to the fetchmail parameters.
Since fetchmail shouldn't run as root, I doubt this will work for most
people (and the logfile needs to be pre-created).
I won't include those two changes, but would accept a patch that adds
config options for the additional parameters (so you can easily add them
yourself without editing the script).
In fetchmail.php, you added the fields
+ "sslcertck" => array(1, 1, 'bool' ),
+ "sslcertpath" => array(1, 1, 'text' ),
+ "sslfingerck" => array(1, 1, 'bool' ),
+ "sslfingerprint" => array(1, 1, 'text' ),
but you did not add any validation to the text fields, which means a
user could inject random fetchmail parameters (I like "mda rm -rf /"
most ;-)
I'll set sslcertpath and sslfingerprint to be only editable if
$CONF['fetchmail_extra_options'] == 'YES' (this config option comes with
a security warning already ;-)
If you have some code to validate the input in those fields, I can relax
the restrictions (would make sense for the fingerprint) and make it
available for everybody.
Oh, and the "sslfingerck" field looks superfluous - I'll change the code
so that it checks if "sslfingerprint" is filled - if yes, it checks the
fingerprint, if it's empty, it (obviously) doesn't.
(Yes, there's the corner case "disable checking, but store the
fingerprint for later usage, but I doubt this is needed in practise ;-)
OTOH, it might cause some confusion, people could forget to set the
checkmark etc.)
Any objections to the changes I did?
I don't hope so, because that's what I just commited to SVN trunk r1519
;-) - but if you have a good reason, I can do another commit.
> MySQL:
> ALTER TABLE`fetchmail` ADD `sslcertck` BOOLEAN NOT NULL DEFAULT '0'
> AFTER `usessl` ,
We use tinyint(1), but that doesn't change much. (You might want to
adjust your table.)
> ADD `sslcertpath` VARCHAR( 254 ) NOT NULL AFTER `sslcertck` ,
> ADD `sslfingerprint` VARCHAR( 254 ) NOT NULL AFTER `sslcertpath`
Added as varchar(255) - also doesn't change much.
The fingerprint can't contain non-ascii chars, to I made the field
latin1.
The sslcertpath can in theory point to "/home/foo/blöde Zertifikate", so
I made it utf8. (BTW: seeing this example, does having a space in the
sslcertpath break anything?)
Regards,
Christian Boltz
--
Danke an alle, die mir bei der Geburt geholfen haben, das Baby brennt ;)
[CD-Brenner-Einrichtung - Thorsten von Plotho-Kettner in suse-linux]
|
