You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spamassassin.apache.org by do...@apache.org on 2007/01/05 23:41:27 UTC

svn commit: r493217 - in /spamassassin/trunk: lib/Mail/SpamAssassin/Conf.pm lib/Mail/SpamAssassin/Conf/Parser.pm lib/Mail/SpamAssassin/Message/Metadata/Received.pm t/trust_path.t

Author: dos
Date: Fri Jan  5 14:41:27 2007
New Revision: 493217

URL: http://svn.apache.org/viewvc?view=rev&rev=493217
Log:
bug 5259: always trust 127/8 no matter how trusted/internal networks are
          configured, or even if they aren't configured

	  no provision for removing 127/8 from trusted/internal networks is
          provided since there is no reason why they cannot be trusted since
          you must trust the machine SA is running on (which could add local
          hops) and you'd have to first trust another relay that has it's own
          local hops before local hops on that other machine were considered
          trusted/internal

          if you think there's a reason to not consider 127/8 trusted/internal
          (even if it's mail submitted from a web server on the same machine)
          you're almost certainly misunderstading the meaning of
          trusted/internal networks


bug 5235: when trusted_networks are inferred (not manually configured) infer
          the same settings for internal_networks as we would for
          trusted_networks

          this makes the inferral method consistent with how we use the same
	  settings for both trusted and internal networks when only one of
          them are set manually

          this resolves a number of FP issues with tests, such as SPF and a
          number of DNSBL tests, that need to know where the handoff is
          between the remote and local domains


Modified:
    spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm
    spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm
    spamassassin/trunk/lib/Mail/SpamAssassin/Message/Metadata/Received.pm
    spamassassin/trunk/t/trust_path.t

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm?view=diff&rev=493217&r1=493216&r2=493217
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm Fri Jan  5 14:41:27 2007
@@ -623,13 +623,13 @@
 If a network or host address is prefaced by a C<!> the network or host will be
 excluded (or included) in a first listed match fashion.
 
+Note: 127/8 is always included in trusted_networks, regardless of your config.
+
 Examples:
 
-    trusted_networks 192.168/16 127/8		# all in 192.168.*.* and 127.*.*.*
-    trusted_networks 212.17.35.15		# just that host
-    trusted_networks 127.			# all in 127.*.*.*
-    trusted_networks !10.0.1.5 10.0.1/24	# all in 10.0.1.* but not 10.0.1.5
-    trusted_networks 10.0.1/24 !10.0.1.5	# all in 10.0.1.* including 10.0.1.5
+   trusted_networks 192.168/16            # all in 192.168.*.*
+   trusted_networks 212.17.35.15          # just that host
+   trusted_networks !10.0.1.5 10.0.1/24   # all in 10.0.1.* but not 10.0.1.5
 
 This operates additively, so a C<trusted_networks> line after another one
 will result in all those networks becoming trusted.  To clear out the
@@ -674,6 +674,7 @@
       foreach my $net (split (/\s+/, $value)) {
         $self->{trusted_networks}->add_cidr ($net);
       }
+      $self->{trusted_networks_configured} = 1;
     }
   });
 
@@ -687,7 +688,8 @@
     setting => 'clear_trusted_networks',
     code => sub {
       my ($self, $key, $value, $line) = @_;
-      $self->{trusted_networks} = Mail::SpamAssassin::NetSet->new();
+      $self->{trusted_networks} = $self->new_netset();
+      $self->{trusted_networks_configured} = 0;
     }
   });
 
@@ -715,6 +717,8 @@
 Every entry in C<internal_networks> must appear in C<trusted_networks>; in
 other words, C<internal_networks> is always a subset of the trusted set.
 
+Note: 127/8 is always included in internal_networks, regardless of your config.
+
 =cut
 
   push (@cmds, {
@@ -727,6 +731,7 @@
       foreach my $net (split (/\s+/, $value)) {
         $self->{internal_networks}->add_cidr ($net);
       }
+      $self->{internal_networks_configured} = 1;
     }
   });
 
@@ -740,7 +745,8 @@
     setting => 'clear_internal_networks',
     code => sub {
       my ($self, $key, $value, $line) = @_;
-      $self->{internal_networks} = Mail::SpamAssassin::NetSet->new();
+      $self->{internal_networks} = $self->new_netset();
+      $self->{internal_networks_configured} = 0;
     }
   });
 
@@ -2719,8 +2725,10 @@
   $self->{more_spam_to} = { };
   $self->{all_spam_to} = { };
 
-  $self->{trusted_networks} = Mail::SpamAssassin::NetSet->new();
-  $self->{internal_networks} = Mail::SpamAssassin::NetSet->new();
+  $self->{trusted_networks} = $self->new_netset();
+  $self->{internal_networks} = $self->new_netset();
+  $self->{trusted_networks_configured} = 0;
+  $self->{internal_networks_configured} = 0;
 
   # Make sure we add in X-Spam-Checker-Version
   $self->{headers_spam}->{"Checker-Version"} =
@@ -3131,6 +3139,13 @@
     delete $self->{source_file};
     delete $self->{meta_dependencies};
   }
+}
+
+sub new_netset {
+  my ($self) = @_;
+  my $set = Mail::SpamAssassin::NetSet->new();
+  $set->add_cidr ('127/8');
+  return $set;
 }
 
 ###########################################################################

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm?view=diff&rev=493217&r1=493216&r2=493217
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm Fri Jan  5 14:41:27 2007
@@ -934,7 +934,7 @@
   # check that all internal_networks are listed in trusted_networks
   # too.
 
-  if ($ni->get_num_nets() > 0 && $nt->get_num_nets() > 0) {
+  if ($conf->{trusted_networks_configured} && $conf->{internal_networks_configured}) {
     my $replace_nets;
     my @valid_ni = ();
 

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Message/Metadata/Received.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Message/Metadata/Received.pm?view=diff&rev=493217&r1=493216&r2=493217
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Message/Metadata/Received.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Message/Metadata/Received.pm Fri Jan  5 14:41:27 2007
@@ -79,23 +79,26 @@
   # now figure out what relays are trusted...
   my $trusted = $permsgstatus->{main}->{conf}->{trusted_networks};
   my $internal = $permsgstatus->{main}->{conf}->{internal_networks};
+  my $did_user_specify_trust = $permsgstatus->{main}->{conf}->{trusted_networks_configured};
+  my $did_user_specify_internal = $permsgstatus->{main}->{conf}->{internal_networks_configured};
   my $in_trusted = 1;
   my $in_internal = 1;
 
-  if ($trusted->get_num_nets() > 0 && $internal->get_num_nets() > 0) {
-    # good; we can use both reliably.
-  }
-  elsif ($trusted->get_num_nets() <= 0 && $internal->get_num_nets() > 0) {
-    $trusted = $internal;	# use 'internal' for 'trusted'
-  }
-  elsif ($trusted->get_num_nets() > 0 && $internal->get_num_nets() <= 0) {
+  if ($did_user_specify_trust && !$did_user_specify_internal) {
     # use 'trusted' for 'internal'; compatibility with SpamAssassin 2.60
     $internal = $trusted;
+    dbg('conf: internal_networks not configured, using trusted_networks '.
+	'configuration for internal_networks; if you really want '.
+	'internal_networks to only contain the required 127/8 add '.
+	"'internal_networks !0/0' to your configuration");
+  } elsif (!$did_user_specify_trust && $did_user_specify_internal) {
+    # this is required, we only check that internal_networks are in
+    # trusted_networks if trusted_networks were defined
+    $trusted = $internal;
+    dbg('conf: trusted_networks not configured, using internal_networks '.
+	'configuration for trusted_networks');
   }
 
-  my $did_user_specify_trust = ($trusted->get_num_nets() > 0);
-  my $did_user_specify_internal = ($internal->get_num_nets() > 0);
-
   my $IP_PRIVATE = IP_PRIVATE;
   my $LOCALHOST = LOCALHOST;
 
@@ -133,29 +136,6 @@
       $self->make_relay_as_string($relay);
     }
 
-    # trusted_networks matches?
-    if ($in_trusted && $did_user_specify_trust && !$relay->{auth} && !$trusted->contains_ip ($relay->{ip}))
-    {
-      $in_trusted = 0;		# we're in deep water now
-    }
-
-    # internal_networks matches?
-    if ($did_user_specify_internal) {
-      if (!$relay->{auth} && !$internal->contains_ip ($relay->{ip})) {
-	$in_internal = 0;
-      }
-    } else {
-      # if the user didn't specify it, assume we immediately transition
-      # to the external network (the internet) once we leave this host.
-      $in_internal = 0;
-    }
-
-    # note: you can't be in internal networks, but not be in a trusted 
-    # net. (bug 4760)
-    if ($in_internal && !$in_trusted) {
-      $in_trusted = 1;
-    }
-
 # OK, infer the trusted/untrusted handover, if we don't have real info.
 # Here's the algorithm used (taken from Dan's mail):
 # 
@@ -184,7 +164,7 @@
 #    contains the "by" of your trusted relay and the "from" of the first
 #    untrusted relay (which is used for bondedsender testing and so on).
 
-    if ($in_trusted && !$did_user_specify_trust) {
+    if ($in_trusted && !($did_user_specify_trust || $did_user_specify_internal)) {
       my $inferred_as_trusted = 0;
 
       # if the 'from' IP addr is in a reserved net range, it's not on
@@ -205,6 +185,38 @@
       dbg("received-header: cannot use DNS, do not trust any hosts from here on");
 
       if (!$inferred_as_trusted) { $in_trusted = 0; }
+    }
+
+
+    # trusted_networks matches?
+    # if they didn't specify trusted, but did specify internal, we use the
+    # internal config for trusted, so check for either being specified
+    if ($in_trusted && ($did_user_specify_trust || $did_user_specify_internal) &&
+	!$relay->{auth} && !$trusted->contains_ip ($relay->{ip}))
+    {
+      $in_trusted = 0;		# we're in deep water now
+    }
+
+    # internal_networks matches?
+    # if they didn't specify internal, but did specify trusted, we use the
+    # trusted config for internal, so check for either being specified
+    if ($did_user_specify_internal || $did_user_specify_trust) {
+      if (!$relay->{auth} && !$internal->contains_ip ($relay->{ip})) {
+	$in_internal = 0;
+      }
+    } else {
+      # if the user didn't specify any trusted/internal config, everything
+      # we assume as trusted is also internal, just like we'd do if they
+      # specified trusted but not any internal networks
+      $in_internal = $in_trusted;
+    }
+
+    # note: you can't be in internal networks, but not be in a trusted 
+    # net. (bug 4760)
+    # dos: I don't think this can even happen any more, since we copy internal
+    # to trusted above (if trusted isn't configured and internal is)
+    if ($in_internal && !$in_trusted) {
+      $in_trusted = 1;
     }
 
     dbg("received-header: relay ".$relay->{ip}.

Modified: spamassassin/trunk/t/trust_path.t
URL: http://svn.apache.org/viewvc/spamassassin/trunk/t/trust_path.t?view=diff&rev=493217&r1=493216&r2=493217
==============================================================================
--- spamassassin/trunk/t/trust_path.t (original)
+++ spamassassin/trunk/t/trust_path.t Fri Jan  5 14:41:27 2007
@@ -18,7 +18,7 @@
 
 use lib '.'; use lib 't';
 use SATest; sa_t_init("trust_path");
-use Test; BEGIN { plan tests => 24 };
+use Test; BEGIN { plan tests => 51 };
 
 
 use strict;
@@ -27,6 +27,131 @@
 
 # ---------------------------------------------------------------------------
 
+# 127/8 implicitly trusted as default
+q{
+
+  Received: from sender.net (127.0.1.2) by receiver.net
+              with SMTP; 10 Nov 2005 00:00:00 -0000
+
+} => q{
+
+Trusted: [ ip=127.0.1.2 rdns=sender.net helo=sender.net by=receiver.net ident= envfrom= intl=1 id= auth= ]
+Untrusted: 
+
+},
+
+# ---------------------------------------------------------------------------
+
+# 127/8 explicitly trusted
+q{
+
+  trusted_networks 127/8
+  Received: from sender.net (127.0.1.2) by receiver.net
+              with SMTP; 10 Nov 2005 00:00:00 -0000
+
+} => q{
+
+Trusted: [ ip=127.0.1.2 rdns=sender.net helo=sender.net by=receiver.net ident= envfrom= intl=1 id= auth= ]
+Untrusted: 
+
+},
+
+# 127/8 explicitly trusted along with others
+q{
+
+  trusted_networks 127/8 1.2.2.1
+  Received: from sender.net (127.0.1.2) by receiver.net
+              with SMTP; 10 Nov 2005 00:00:00 -0000
+
+} => q{
+
+Trusted: [ ip=127.0.1.2 rdns=sender.net helo=sender.net by=receiver.net ident= envfrom= intl=1 id= auth= ]
+Untrusted: 
+
+},
+
+# ---------------------------------------------------------------------------
+
+# 127/8 explicitly untrusted
+q{
+
+  trusted_networks 1.2/16 !127/8
+  internal_networks 1.2/16 !127/8
+  Received: from sender.net (127.0.1.2) by receiver.net
+              with SMTP; 10 Nov 2005 00:00:00 -0000
+
+} => q{
+
+Trusted: [ ip=127.0.1.2 rdns=sender.net helo=sender.net by=receiver.net ident= envfrom= intl=1 id= auth= ]
+Untrusted: 
+
+},
+
+# ---------------------------------------------------------------------------
+
+# 127/8 implicitly trusted
+q{
+
+  trusted_networks 1.2/16
+  Received: from sender.net (127.0.1.2) by receiver.net
+              with SMTP; 10 Nov 2005 00:00:00 -0000
+
+} => q{
+
+Trusted: [ ip=127.0.1.2 rdns=sender.net helo=sender.net by=receiver.net ident= envfrom= intl=1 id= auth= ]
+Untrusted: 
+
+},
+
+# ---------------------------------------------------------------------------
+
+# 10/8 implicitly trusted by auto-detection
+# note: it should also be internal!
+q{
+
+  Received: from sender.net (10.0.1.2) by receiver.net
+              with SMTP; 10 Nov 2005 00:00:00 -0000
+
+} => q{
+
+Trusted: [ ip=10.0.1.2 rdns=sender.net helo=sender.net by=receiver.net ident= envfrom= intl=1 id= auth= ]
+Untrusted: 
+
+},
+
+# ---------------------------------------------------------------------------
+
+# trusted, then not (which is trusted, we do first match wins)
+q{
+
+  trusted_networks 1.2/16 !1.2/16
+  Received: from sender.net (1.2.3.2) by receiver.net
+              with SMTP; 10 Nov 2005 00:00:00 -0000
+
+} => q{
+
+Trusted: [ ip=1.2.3.2 rdns=sender.net helo=sender.net by=receiver.net ident= envfrom= intl=1 id= auth= ]
+Untrusted:
+
+},
+
+# ---------------------------------------------------------------------------
+
+q{
+
+  trusted_networks 1.2/16
+  Received: from sender.net (1.1.1.2) by receiver.net
+              with SMTP; 10 Nov 2005 00:00:00 -0000
+
+} => q{
+
+Trusted:
+Untrusted: [ ip=1.1.1.2 rdns=sender.net helo=sender.net by=receiver.net ident= envfrom= intl=0 id= auth= ]
+
+},
+
+# ---------------------------------------------------------------------------
+
 q{
 
   trusted_networks 1.1/16
@@ -92,8 +217,8 @@
 # ---------------------------------------------------------------------------
 
 # this should be a lint error; internal_networks is not a subset of trusted.
-# note: "intl=1" in expected, because the error means that the internal_networks
-# line is ignored and the default setting is intl==trusted.
+# note: "intl=0" is expected; even though the internal_networks config is
+# invalid it was still defined by the user, so we do not use trusted for internal
 q{
 
   trusted_networks 1.1/16
@@ -104,7 +229,7 @@
 } => q{
 
 Lint-Error
-Trusted: [ ip=1.1.1.2 rdns=sender.net helo=sender.net by=receiver.net ident= envfrom= intl=1 id= auth= ]
+Trusted: [ ip=1.1.1.2 rdns=sender.net helo=sender.net by=receiver.net ident= envfrom= intl=0 id= auth= ]
 Untrusted:
 
 },
@@ -112,6 +237,8 @@
 # ---------------------------------------------------------------------------
 
 # this should be a lint error; internal_networks is not a subset of trusted.
+# note: "intl=0" is expected; even though the internal_networks config is
+# invalid it was still defined by the user, so we do not use trusted for internal
 q{
 
   trusted_networks 1.1.1/24
@@ -122,7 +249,7 @@
 } => q{
 
 Lint-Error
-Trusted: [ ip=1.1.1.2 rdns=sender.net helo=sender.net by=receiver.net ident= envfrom= intl=1 id= auth= ]
+Trusted: [ ip=1.1.1.2 rdns=sender.net helo=sender.net by=receiver.net ident= envfrom= intl=0 id= auth= ]
 Untrusted:
 
 },
@@ -130,8 +257,8 @@
 # ---------------------------------------------------------------------------
 
 # this should be a lint error; internal_networks is not a subset of trusted.
-# note: "intl=1" in expected, because the error means that the internal_networks
-# line is ignored and the default setting is intl==trusted.
+# note: "intl=0" is expected; even though the internal_networks config is
+# invalid it was still defined by the user, so we do not use trusted for internal
 q{
 
   trusted_networks !1.1.1.1 1.1/16
@@ -142,6 +269,23 @@
 } => q{
 
 Lint-Error
+Trusted: [ ip=1.1.1.2 rdns=sender.net helo=sender.net by=receiver.net ident= envfrom= intl=0 id= auth= ]
+Untrusted:
+
+},
+
+# ---------------------------------------------------------------------------
+
+# this should be a lint error; you can't exclude a network after you've already
+# included it (TODO: it is currently not a lint error, netset just warns about it)
+q{
+
+  trusted_networks 1/8 !1.1.1.2
+  Received: from sender.net (1.1.1.2) by receiver.net
+              with SMTP; 10 Nov 2005 00:00:00 -0000
+
+} => q{
+
 Trusted: [ ip=1.1.1.2 rdns=sender.net helo=sender.net by=receiver.net ident= envfrom= intl=1 id= auth= ]
 Untrusted:
 
@@ -181,10 +325,12 @@
             "clear_trusted_networks\n".
             "clear_internal_networks\n";
 
-  $hdrs =~ s/^\s*(trusted_networks\s+[^\n]*)//gs;
-  if ($1) { $conf .= $1."\n"; }
-  $hdrs =~ s/^\s*(internal_networks\s+[^\n]*)//gs;
-  if ($1) { $conf .= $1."\n"; }
+  if ($hdrs =~ s/^\s*(trusted_networks\s+[^\n]*)//gs) {
+    $conf .= $1."\n";
+  }
+  if ($hdrs =~ s/^\s*(internal_networks\s+[^\n]*)//gs) {
+    if ($1) { $conf .= $1."\n"; }
+  }
 
   tstprefs ($conf);
 
@@ -228,7 +374,7 @@
     print "expected: $expected\n";
     print "got     : $relays\n\n";
 
-    die "dying on first test failure";
+    # die "dying on first test failure";
   }
 
   $status->finish();