Jared Johnson wrote:
> 
> Index: plugins/virus/clamdscan
> ===================================================================
> --- plugins/virus/clamdscan   (revision 961)
> +++ plugins/virus/clamdscan   (working copy)
> @@ -24,8 +24,8 @@
>  =item * Enable the "AllowSupplementaryGroups" option in clamd.conf.
>  
>  =item * Change the permissions of the qpsmtpd spool directory to 0750 (this 
> -will emit a warning when the qpsmtpd service starts up, but can be safely
> -ignored).
> +will emit a warning when the qpsmtpd service starts up, unless/until you
> +write '0750' to the 'spool_perms' configuration file).
>  
>  =item * Make sure that all directories above the spool directory (to the
>  root) are g+x so that the group has directory traversal rights; it is not
> Index: lib/Qpsmtpd.pm
> ===================================================================
> --- lib/Qpsmtpd.pm    (revision 961)
> +++ lib/Qpsmtpd.pm    (working copy)
> @@ -529,18 +529,15 @@

Please try and wrap your lines at 80 characters.
>    
>      $Spool_dir =~ /^(.+)$/ or die "spool_dir not configured properly";
>      $Spool_dir = $1; # cleanse the taint
> +    my $Spool_perms  = $self->config('spool_perms') || '0700';
>  
> -    # Make sure the spool dir has appropriate rights
> -    if (-e $Spool_dir) {
> -      my $mode = (stat($Spool_dir))[2];
> -      $self->log(LOGWARN, 
> -          "Permissions on spool_dir $Spool_dir are not 0700")
> -        if $mode & 07077;
> +    if (-d $Spool_dir) { # Make sure the spool dir has appropriate rights
> +      $self->log(LOGWARN,"Permissions on spool_dir $Spool_dir are not 2750")

    I thought the point was to make the spool perms configurable.

> +        unless sprintf('%o',(stat($Spool_dir))[2] & 07777) eq $Spool_perms;

    Why are you doing a string comparison here?  Much cleaner to just
    do it numeric.  

> +    } else { # Or create it if it doesn't already exist
> +      mkdir($Spool_dir,oct $Spool_perms) or die "Could not create spool_dir 
> $Spool_dir: $!";
>      }
>  
> -    # And finally, create it if it doesn't already exist
> -    -d $Spool_dir or mkdir($Spool_dir, 0700) 
> -      or die "Could not create spool_dir $Spool_dir: $!";
>    }
>      
>    return $Spool_dir;


I think the code flow was cleaner in general before.

Reply via email to