Patchwork PROBLEM: Netfilter time matching matches all packets when time start and time stop is the same

login
register
mail settings
Submitter Henry Lee
Date July 31, 2013, 8:42 a.m.
Message ID <CAK+MRYuFUCvkker9sq7LFa42t=vQ7FhWwzz37Mtw3sLMZTJ+EQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/263627/
State Not Applicable
Headers show

Comments

Henry Lee - July 31, 2013, 8:42 a.m.
Dear sirs:

I've found a bug when running the netfilter time matching module. Here
is the description.

[1.] One line summary of the problem:
Netfilter time matching matches all packets when time start and time
stop is the same

[2.] Full description of the problem/report:
When I used "-m time --timestart 01:00 --timestop 01:00", the netfitler
matches all day packets, which in my point of view, looks unnatural.

[3.] Keywords (i.e., modules, networking, kernel):
modules, networking, netfilter

[4.] Kernel information

[4.1.] Kernel version (from /proc/version):
Linux version 3.8.0-27-generic (buildd@roseapple) (gcc version 4.7.3
(Ubuntu/Linaro 4.7.3-1ubuntu1) ) #40-Ubuntu SMP Tue Jul 9 00:17:05 UTC 2013

I tested this on 3.8.0, in which the xt_time.c should be the newest.

[4.2.] Kernel .config file:
Irrelevant.

[5.] Most recent kernel version which did not have the bug:
None.

[6.] Output of Oops.. message (if applicable) with symbolic information
      resolved (see Documentation/oops-tracing.txt)
Irrelevant.

[7.] A small shell script or example program which triggers the
      problem (if possible)
iptables -I INPUT -m time --timestart 01:00 --timestop 01:00 -j DROP

[8.] Environment
[8.1.] Software (add the output of the ver_linux script here)
Irrelevant.

[8.2.] Processor information (from /proc/cpuinfo):
Irrelevant.

[8.3.] Module information (from /proc/modules):
Module                  Size  Used by
xt_time                12661  0
xt_tcpudp              12603  1
iptable_filter         12810  1
xt_multiport           12597  1
ipt_MASQUERADE         12759  1
iptable_nat            12810  1
nf_conntrack_ipv4      14487  1
nf_defrag_ipv4         12729  1 nf_conntrack_ipv4
nf_nat_ipv4            13263  1 iptable_nat
nf_nat                 25867  3 ipt_MASQUERADE,nf_nat_ipv4,iptable_nat
nf_conntrack           83275  5
ipt_MASQUERADE,nf_nat,nf_nat_ipv4,iptable_nat,nf_conntrack_ipv4
ip_tables              26995  2 iptable_filter,iptable_nat
8021q                  24000  0
garp                   14354  1 8021q
stp                    12976  1 garp
llc                    14552  2 stp,garp
ipt_ULOG               17410  1
x_tables               29803  7
xt_time,ipt_ULOG,ip_tables,xt_tcpudp,ipt_MASQUERADE,xt_multiport,iptable_filter
...
The rest are irrelevant.

[8.4.] Loaded driver and hardware information (/proc/ioports, /proc/iomem)
Irrelevant.

[8.5.] PCI information ('lspci -vvv' as root)
Irrelevant.

[8.6.] SCSI information (from /proc/scsi/scsi)
Irrelevant.

[8.7.] Other information that might be relevant to the problem
        (please look in /proc and include all information that you
        think to be relevant):
None.

[X.] Other notes, patches, fixes, workarounds:
I'll place the patch in the attachment as well.

             return false;

==========

Yours faithfully,
Henry Lee
Michal Kubeček - Aug. 1, 2013, 11:24 a.m.
On Wed, Jul 31, 2013 at 04:42:15PM +0800, Henry Lee wrote:
> diff --git a/net/netfilter/xt_time.c b/net/netfilter/xt_time.c
> index 0ae55a3..753573c 100644
> --- a/net/netfilter/xt_time.c
> +++ b/net/netfilter/xt_time.c
> @@ -192,7 +192,7 @@ time_mt(const struct sk_buff *skb, struct
> xt_action_param *par)
> 
>      packet_time = localtime_1(&current_time, stamp);
> 
> -   if (info->daytime_start < info->daytime_stop) {
> +   if (info->daytime_start <= info->daytime_stop) {
>          if (packet_time < info->daytime_start ||
>              packet_time > info->daytime_stop)
>              return false;
> 

As far as I can see, this would cause only packets arriving at midnight
to match by default (i.e. without both --timestart and --timestop).

                                                         Michal Kubecek

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maciej Żenczykowski - Aug. 2, 2013, 1:35 a.m.
Does having timestart == timestop make any sense?
Why would you want to do that?

Perhaps make iptables reject such input?

On Thu, Aug 1, 2013 at 4:24 AM, Michal Kubecek <mkubecek@suse.cz> wrote:
> On Wed, Jul 31, 2013 at 04:42:15PM +0800, Henry Lee wrote:
>> diff --git a/net/netfilter/xt_time.c b/net/netfilter/xt_time.c
>> index 0ae55a3..753573c 100644
>> --- a/net/netfilter/xt_time.c
>> +++ b/net/netfilter/xt_time.c
>> @@ -192,7 +192,7 @@ time_mt(const struct sk_buff *skb, struct
>> xt_action_param *par)
>>
>>      packet_time = localtime_1(&current_time, stamp);
>>
>> -   if (info->daytime_start < info->daytime_stop) {
>> +   if (info->daytime_start <= info->daytime_stop) {
>>          if (packet_time < info->daytime_start ||
>>              packet_time > info->daytime_stop)
>>              return false;
>>
>
> As far as I can see, this would cause only packets arriving at midnight
> to match by default (i.e. without both --timestart and --timestop).
>
>                                                          Michal Kubecek
>
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Henry Lee - Aug. 2, 2013, 3:18 a.m.
On 08/02/2013 09:35 AM, Maciej Żenczykowski wrote:
> Does having timestart == timestop make any sense?
> Why would you want to do that?
>
> Perhaps make iptables reject such input?
>
> On Thu, Aug 1, 2013 at 4:24 AM, Michal Kubecek <mkubecek@suse.cz> wrote:
>> On Wed, Jul 31, 2013 at 04:42:15PM +0800, Henry Lee wrote:
>>> diff --git a/net/netfilter/xt_time.c b/net/netfilter/xt_time.c
>>> index 0ae55a3..753573c 100644
>>> --- a/net/netfilter/xt_time.c
>>> +++ b/net/netfilter/xt_time.c
>>> @@ -192,7 +192,7 @@ time_mt(const struct sk_buff *skb, struct
>>> xt_action_param *par)
>>>
>>>       packet_time = localtime_1(&current_time, stamp);
>>>
>>> -   if (info->daytime_start < info->daytime_stop) {
>>> +   if (info->daytime_start <= info->daytime_stop) {
>>>           if (packet_time < info->daytime_start ||
>>>               packet_time > info->daytime_stop)
>>>               return false;
>>>
>> As far as I can see, this would cause only packets arriving at midnight
>> to match by default (i.e. without both --timestart and --timestop).
>>
>>                                                           Michal Kubecek
>>
Dear Mr Kubecek,
I can see that iptables uses 00:00:00 and 23:59:59 as the default value 
of timestart and timestop. In this case, even if both timestart and 
timestop are not defined by user, it still works correctly.
Of cause, if some other tools use 00:00:00 and 00:00:00 as the default, 
your concern will become a problem.

Dear Mr Żenczykowski,
I wouldn't use a timestart == timestop rule manually. But if I create 
iptables rules in a program or a script, this case may happen.
Rejecting this rule seems a little bit harsh, in my opinion, since it 
doesn't look so unacceptable.

Thank you both for your time.


Yours faithfully,
Henry Lee

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pablo Neira - Aug. 8, 2013, 4:02 p.m.
Hi,

On Fri, Aug 02, 2013 at 11:18:54AM +0800, Henry Lee wrote:
[...]
> I wouldn't use a timestart == timestop rule manually. But if I
> create iptables rules in a program or a script, this case may
> happen.
> Rejecting this rule seems a little bit harsh, in my opinion, since
> it doesn't look so unacceptable.

I cannot take this patch since others may be relaying in the current
behaviour. You'll have to fix your script/program to catch that case
and avoid it.

Regards.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/netfilter/xt_time.c b/net/netfilter/xt_time.c

index 0ae55a3..753573c 100644

--- a/net/netfilter/xt_time.c

+++ b/net/netfilter/xt_time.c

@@ -192,7 +192,7 @@  time_mt(const struct sk_buff *skb, struct xt_action_param *par)

 
 	packet_time = localtime_1(&current_time, stamp);
 
-	if (info->daytime_start < info->daytime_stop) {

+	if (info->daytime_start <= info->daytime_stop) {

 		if (packet_time < info->daytime_start ||
 		    packet_time > info->daytime_stop)
 			return false;