Message ID | 20200501192135.15043-1-philipp@redfish-solutions.com |
---|---|
State | Needs Review / ACK |
Headers | show |
Series | [OpenWrt-Devel,1/1] firewall3: add --contiguous to time-based rules where needed | expand |
Should I open a PR or is this the correct way to get a review for firewall3? Thanks > On May 1, 2020, at 1:21 PM, Philip Prindeville <philipp@redfish-solutions.com> wrote: > > From: Philip Prindeville <philipp@redfish-solutions.com> > > If the start_time > stop_time on a rule, then the --contiguous arg > should be included in the rule. > > Signed-off-by: Philip Prindeville <philipp@redfish-solutions.com> > --- > iptables.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/iptables.c b/iptables.c > index 559fe7defef3be85c4eb2934884caf549f932bc5..5c02e6e26c93468f4ef6a7f917069bb49985aad8 100644 > --- a/iptables.c > +++ b/iptables.c > @@ -1099,6 +1099,9 @@ fw3_ipt_rule_time(struct fw3_ipt_rule *r, struct fw3_time *time) > fw3_ipt_rule_addarg(r, false, "--timestop", buf); > } > > + if (time->timestart && time->timestop && time->timestart > time->timestop) > + fw3_ipt_rule_addarg(r, false, "--contiguous", NULL); > + > if (time->monthdays & 0xFFFFFFFE) > { > for (i = 1, p = buf; i < 32; i++)
On Sat, 2 May 2020 at 03:21, Philip Prindeville <philipp@redfish-solutions.com> wrote: > > From: Philip Prindeville <philipp@redfish-solutions.com> > > If the start_time > stop_time on a rule, then the --contiguous arg > should be included in the rule. It seems that start_time >= stop_time has its defined meaning in xt_time module. Better add another uci option for this --contiguous flag. Regards, yousong > > Signed-off-by: Philip Prindeville <philipp@redfish-solutions.com> > --- > iptables.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/iptables.c b/iptables.c > index 559fe7defef3be85c4eb2934884caf549f932bc5..5c02e6e26c93468f4ef6a7f917069bb49985aad8 100644 > --- a/iptables.c > +++ b/iptables.c > @@ -1099,6 +1099,9 @@ fw3_ipt_rule_time(struct fw3_ipt_rule *r, struct fw3_time *time) > fw3_ipt_rule_addarg(r, false, "--timestop", buf); > } > > + if (time->timestart && time->timestop && time->timestart > time->timestop) > + fw3_ipt_rule_addarg(r, false, "--contiguous", NULL); > + > if (time->monthdays & 0xFFFFFFFE) > { > for (i = 1, p = buf; i < 32; i++) > -- > 2.17.2 > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
> On May 12, 2020, at 7:08 AM, Yousong Zhou <yszhou4tech@gmail.com> wrote: > > On Sat, 2 May 2020 at 03:21, Philip Prindeville > <philipp@redfish-solutions.com> wrote: >> >> From: Philip Prindeville <philipp@redfish-solutions.com> >> >> If the start_time > stop_time on a rule, then the --contiguous arg >> should be included in the rule. > > It seems that start_time >= stop_time has its defined meaning in > xt_time module. Better add another uci option for this --contiguous > flag. > > Regards, > yousong Sorry, not following. What would that UCI option look like? From iptables-extensions: time This matches if the packet arrival time/date is within a given range. All options are optional, but are ANDed when specified. All times are interpreted as UTC by default. --datestart YYYY[-MM[-DD[Thh[:mm[:ss]]]]] --datestop YYYY[-MM[-DD[Thh[:mm[:ss]]]]] Only match during the given time, which must be in ISO 8601 "T" notation. The possible time range is 1970-01-01T00:00:00 to 2038-01-19T04:17:07. If --datestart or --datestop are not specified, it will default to 1970-01-01 and 2038-01-19, respectively. --timestart hh:mm[:ss] --timestop hh:mm[:ss] Only match during the given daytime. The possible time range is 00:00:00 to 23:59:59. Leading zeroes are allowed (e.g. "06:03") and correctly interpreted as base-10. [!] --monthdays day[,day...] Only match on the given days of the month. Possible values are 1 to 31. Note that specifying 31 will of course not match on months which do not have a 31st day; the same goes for 28- or 29-day February. [!] --weekdays day[,day...] Only match on the given weekdays. Possible values are Mon, Tue, Wed, Thu, Fri, Sat, Sun, or values from 1 to 7, respectively. You may also use two-character variants (Mo, Tu, etc.). --contiguous When --timestop is smaller than --timestart value, match this as a single time period instead distinct intervals. See EXAMPLES. --kerneltz Use the kernel timezone instead of UTC to determine whether a packet meets the time regulations. About kernel timezones: Linux keeps the system time in UTC, and always does so. On boot, system time is initialized from a referential time source. Where this time source has no timezone information, such as the x86 CMOS RTC, UTC will be assumed. If the time source is however not in UTC, userspace should provide the correct system time and timezone to the kernel once it has the information. Local time is a feature on top of the (timezone independent) system time. Each process has its own idea of local time, specified via the TZ environment variable. The kernel also has its own timezone offset vari‐ able. The TZ userspace environment variable specifies how the UTC-based system time is displayed, e.g. when you run date(1), or what you see on your desktop clock. The TZ string may resolve to different offsets at different dates, which is what enables the automatic time-jumping in userspace. when DST changes. The kernel's timezone offset variable is used when it has to convert between non-UTC sources, such as FAT filesystems, to UTC (since the latter is what the rest of the system uses). The caveat with the kernel timezone is that Linux distributions may ignore to set the kernel timezone, and instead only set the system time. Even if a particular distribution does set the timezone at boot, it is usually does not keep the kernel timezone offset - which is what changes on DST - up to date. ntpd will not touch the kernel timezone, so running it will not resolve the issue. As such, one may encounter a timezone that is always +0000, or one that is wrong half of the time of the year. As such, using --kerneltz is highly discouraged. EXAMPLES. To match on weekends, use: -m time --weekdays Sa,Su Or, to match (once) on a national holiday block: -m time --datestart 2007-12-24 --datestop 2007-12-27 Since the stop time is actually inclusive, you would need the following stop time to not match the first second of the new day: -m time --datestart 2007-01-01T17:00 --datestop 2007-01-01T23:59:59 During lunch hour: -m time --timestart 12:30 --timestop 13:30 The fourth Friday in the month: -m time --weekdays Fr --monthdays 22,23,24,25,26,27,28 (Note that this exploits a certain mathematical property. It is not possible to say "fourth Thursday OR fourth Friday" in one rule. It is possible with multiple rules, though.) Matching across days might not do what is expected. For instance, -m time --weekdays Mo --timestart 23:00 --timestop 01:00 Will match Monday, for one hour from midnight to 1 a.m., and then again for another hour from 23:00 onwards. If this is unwanted, e.g. if you would like 'match for two hours from Montay 23:00 onwards' you need to also specify the --contiguous option in the example above. This last section is the bit that I’m trying to address. I’m in GMT-0700. So if I want a rule granting access from 8am to 9pm… local, that’s 14:00 GMT … 03:00 GMT, but it requires the --contiguous flag for the above reason. Are you suggesting adding an option like: option contiguous true | false that would cause the argument to get appended when timestart > timestop? Would it always be appended or just when timestart > timestop (it’s not clear what it does in the case where timestart < timestop && contiguous)? > >> >> Signed-off-by: Philip Prindeville <philipp@redfish-solutions.com> >> --- >> iptables.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/iptables.c b/iptables.c >> index 559fe7defef3be85c4eb2934884caf549f932bc5..5c02e6e26c93468f4ef6a7f917069bb49985aad8 100644 >> --- a/iptables.c >> +++ b/iptables.c >> @@ -1099,6 +1099,9 @@ fw3_ipt_rule_time(struct fw3_ipt_rule *r, struct fw3_time *time) >> fw3_ipt_rule_addarg(r, false, "--timestop", buf); >> } >> >> + if (time->timestart && time->timestop && time->timestart > time->timestop) >> + fw3_ipt_rule_addarg(r, false, "--contiguous", NULL); >> + >> if (time->monthdays & 0xFFFFFFFE) >> { >> for (i = 1, p = buf; i < 32; i++) >> -- >> 2.17.2
On Wed, 13 May 2020 at 00:39, Philip Prindeville <philipp_subx@redfish-solutions.com> wrote: > > > > > On May 12, 2020, at 7:08 AM, Yousong Zhou <yszhou4tech@gmail.com> wrote: > > > > On Sat, 2 May 2020 at 03:21, Philip Prindeville > > <philipp@redfish-solutions.com> wrote: > >> > >> From: Philip Prindeville <philipp@redfish-solutions.com> > >> > >> If the start_time > stop_time on a rule, then the --contiguous arg > >> should be included in the rule. > > > > It seems that start_time >= stop_time has its defined meaning in > > xt_time module. Better add another uci option for this --contiguous > > flag. > > > > Regards, > > yousong > > > Sorry, not following. What would that UCI option look like? > > From iptables-extensions: > > time > This matches if the packet arrival time/date is within a given range. > All options are optional, but are ANDed when specified. All times are > interpreted as UTC by default. > > --datestart YYYY[-MM[-DD[Thh[:mm[:ss]]]]] > > --datestop YYYY[-MM[-DD[Thh[:mm[:ss]]]]] > Only match during the given time, which must be in ISO 8601 "T" > notation. The possible time range is 1970-01-01T00:00:00 to > 2038-01-19T04:17:07. > > If --datestart or --datestop are not specified, it will default > to 1970-01-01 and 2038-01-19, respectively. > > --timestart hh:mm[:ss] > > --timestop hh:mm[:ss] > Only match during the given daytime. The possible time range is > 00:00:00 to 23:59:59. Leading zeroes are allowed (e.g. "06:03") > and correctly interpreted as base-10. > > [!] --monthdays day[,day...] > Only match on the given days of the month. Possible values are 1 > to 31. Note that specifying 31 will of course not match on > months which do not have a 31st day; the same goes for 28- or > 29-day February. > > [!] --weekdays day[,day...] > Only match on the given weekdays. Possible values are Mon, Tue, > Wed, Thu, Fri, Sat, Sun, or values from 1 to 7, respectively. > You may also use two-character variants (Mo, Tu, etc.). > > --contiguous > When --timestop is smaller than --timestart value, match this as > a single time period instead distinct intervals. See EXAMPLES. > > --kerneltz > Use the kernel timezone instead of UTC to determine whether a > packet meets the time regulations. > > About kernel timezones: Linux keeps the system time in UTC, and always > does so. On boot, system time is initialized from a referential time > source. Where this time source has no timezone information, such as the > x86 CMOS RTC, UTC will be assumed. If the time source is however not in > UTC, userspace should provide the correct system time and timezone to > the kernel once it has the information. > > Local time is a feature on top of the (timezone independent) system > time. Each process has its own idea of local time, specified via the TZ > environment variable. The kernel also has its own timezone offset vari‐ > able. The TZ userspace environment variable specifies how the UTC-based > system time is displayed, e.g. when you run date(1), or what you see on > your desktop clock. The TZ string may resolve to different offsets at > different dates, which is what enables the automatic time-jumping in > userspace. when DST changes. The kernel's timezone offset variable is > used when it has to convert between non-UTC sources, such as FAT > filesystems, to UTC (since the latter is what the rest of the system > uses). > > The caveat with the kernel timezone is that Linux distributions may > ignore to set the kernel timezone, and instead only set the system > time. Even if a particular distribution does set the timezone at boot, > it is usually does not keep the kernel timezone offset - which is what > changes on DST - up to date. ntpd will not touch the kernel timezone, > so running it will not resolve the issue. As such, one may encounter a > timezone that is always +0000, or one that is wrong half of the time of > the year. As such, using --kerneltz is highly discouraged. > > EXAMPLES. To match on weekends, use: > > -m time --weekdays Sa,Su > > Or, to match (once) on a national holiday block: > > -m time --datestart 2007-12-24 --datestop 2007-12-27 > > Since the stop time is actually inclusive, you would need the following > stop time to not match the first second of the new day: > > -m time --datestart 2007-01-01T17:00 --datestop > 2007-01-01T23:59:59 > > During lunch hour: > > -m time --timestart 12:30 --timestop 13:30 > > The fourth Friday in the month: > > -m time --weekdays Fr --monthdays 22,23,24,25,26,27,28 > > (Note that this exploits a certain mathematical property. It is not > possible to say "fourth Thursday OR fourth Friday" in one rule. It is > possible with multiple rules, though.) > > Matching across days might not do what is expected. For instance, > > -m time --weekdays Mo --timestart 23:00 --timestop 01:00 Will > match Monday, for one hour from midnight to 1 a.m., and then > again for another hour from 23:00 onwards. If this is unwanted, > e.g. if you would like 'match for two hours from Montay 23:00 > onwards' you need to also specify the --contiguous option in the > example above. > > This last section is the bit that I’m trying to address. > > I’m in GMT-0700. > > So if I want a rule granting access from 8am to 9pm… local, that’s 14:00 GMT … 03:00 GMT, but it requires the --contiguous flag for the above reason. > > Are you suggesting adding an option like: > > option contiguous true | false > > that would cause the argument to get appended when timestart > timestop? > > Would it always be appended or just when timestart > timestop (it’s not clear what it does in the case where timestart < timestop && contiguous)? My understanding is that "--contiguous" for timestart > timestop makes sense *only when* either weekday or monthday match are also specified (for timestart). See [1] It's invalid combination "timestart < timestop && contiguous". See [2] [1] time_mt, https://github.com/torvalds/linux/blob/24085f70a6e1b0cb647ec92623284641d8270637/net/netfilter/xt_time.c#L215-L225 [2] time_mt_check, https://github.com/torvalds/linux/blob/24085f70a6e1b0cb647ec92623284641d8270637/net/netfilter/xt_time.c#L259-L261 Regards, yousong
diff --git a/iptables.c b/iptables.c index 559fe7defef3be85c4eb2934884caf549f932bc5..5c02e6e26c93468f4ef6a7f917069bb49985aad8 100644 --- a/iptables.c +++ b/iptables.c @@ -1099,6 +1099,9 @@ fw3_ipt_rule_time(struct fw3_ipt_rule *r, struct fw3_time *time) fw3_ipt_rule_addarg(r, false, "--timestop", buf); } + if (time->timestart && time->timestop && time->timestart > time->timestop) + fw3_ipt_rule_addarg(r, false, "--contiguous", NULL); + if (time->monthdays & 0xFFFFFFFE) { for (i = 1, p = buf; i < 32; i++)