Bug reports on 1.3.3 and feature requests

[braveheartleo]

A quick note about the bugs:

These bugs, at least nos. 2 and 3 have existed since 1.2.x versions, and since it seems nobody has noticed them this far, then it's about time to bring these to light.

1. enabling or disabling upnp in the webif doesn't change the enabled option in /etc/config/upnpd, thus upnp remains disabled, but changing the up and down bitrates reflect the values in it.

2. setting the system->time in webif is erratic, for example selecting utc+8 timezone and asia timeserver then saving it doesn't persist the changes, and defaults to utc-8 timezone. moreover, what's even more bugging is that trying to make the change persist by saving a couple of times would _destroy_ the button and led sections in /etc/config/system such that they become integrated into the system section and that several button options are lost, and the timezone option still remains at utc-8.

3. setting firewall->conn limits produces two invalid sysctl.conf lines:

* net.ipv4.netfilter.ip_conntrack_udp_timout_stream, missing an e
* net.ipv4.ip_conntrack_max is reported as an unknown key, although the correct key net.ipv4.netfilter.ip_conntrack_max sets the total conntrack limit fine

thus the original udp timeout remains the same and that the erroneous lines are reported in syslog as an unknown sysctl keys.

some quick feature requests:

1. would it be possible for the webif to offer mixed psk encryption mode? although it can be set via command line, when it is viewed in the webif the encryption is identified as disabled, although this is not the case.

2. would it be possible also to offer up a page for iptables? it may sound not-so-useful for the average joe, but i guess it can be made so that it details the statistics in iptables, such as dropped connections and defined rules, call it a firewall status page if you will. if it can be made advanced then something like how it would look in luci or x-wrt.

those are all for now. will be reporting back for more when found. as always, a great firmware release.

[braveheartleo]

a cosmetic bug is that setting connection->dhcp->dhcp range end value produces an off-by-one count in the dhcp range. if the intention is to set IP start 100 and end 150, then dnsmasq will create a range between 100 and 151, because the "limit" option will be written as 51 by webif, and dnsmasq will interpret it as start+limit.

i guess this error stemmed from the past where setting the option "limit" produces a range minus 1. this is no longer the case, as it is indicated in the updated docs page for dhcp and dns at openwrt. also, setting "end" to a value greater than 254 is not a problem because even if "limit" exceeds the range, the initscript corrects it no problem. it's only that if you set dhcp range via webif believing the router would dish out ips in that range, then the router might give the last IP that is out of that range by chance.

also, i was testing web monitoring and i noticed that in the local host column the ips are indicated in reverse, such that 192.168.1.2 is displayed as 2.1.168.192, thus "display hostnames" do not work.

i specified an ip to be excluded from monitoring and this works ok, although looking at the iptables "filter" table, "web_monitor" chain in the commandline indicates this ip match in reverse too. odd, but it works.

a quick request: i noticed that hovering upon a line in the website column links to that website, but this is not as helpful as simply visiting such website because often times visiting sub-domains do not really host any meaningful webservice and would sometimes redirect to the main website. i was thinking of instead linking to the address itself, maybe the webif can instead link to an online service that can provide useful information, something that would look like this:

www.gargoyle-router.com

this would make it extremely useful.

[Eric]

Regarding your bug reports:

1. This should now be fixed (in the git repository)

2. The time was not being set improperly, but rather the display of the time was incorrect. Whenever the time being set was just an offset of UTC (instead of a specially labeled timezone, e.g. EST for Eastern Standard time), the date would be reported as a time with the appropriate offset, but with a timezone of UTC instead of UTC+/-offset. This should now be fixed, along with the bug you report regarding how saving the time can clobber the LED sections.

3. The typo in net.ipv4.netfilter.ip_conntrack_udp_timout_stream is now fixed. However, the reason we have net.ipv4.ip_conntrack_max and net.ipv4.netfilter.ip_conntrack_max is because one is appropriate for the 2.4 kernels used in brcm devices, and the other is appropriate for 2.6 kernels. Having the extra, invalid sysctl key on each system really doesn't do any harm, and this ensures both kernels work.

4. (DHCP issue) If you want to allow ip addresses 192.168.1.100-192.168.1.150 inclusive that is 51 addresses. You have 192.168.1.101-150 (that's 50), and then 192.168.1.100 which makes 51. I don't think this is a bug.

5. I've fixed the web monitor so that IPs will now be displayed properly on both big and little endian systems.

Feature Requests:

1. Yes, I really should implement the mixed mode psk encryption sometime soon. Shouldn't be too hard.

2. No. I have no intention to give users direct access to iptables -- I'd rather keep things simple. If you need direct access to iptables, you should login via ssh.

3. I think it makes a lot more sense to link to a domain in question than a report on the visitors/geolocation of the domain. The former will likely give you a clue as to what it might be about, while the latter will not. You'll note if you follow your link that I will learn where Gargoyle is being hosted, but it won't tell me that it is a site dedicated to router firmware. I really think actually visiting the site is a LOT more informative.

[braveheartleo]

Thank you for your support and explanations.

However, regarding the DHCP issue I guess I got the whole thing reversed. The OpenWRT documentation description of this option is inaccurate. Here's why:

Here's an excerpt of my dhcp config:

Code: Select all

config 'dhcp' 'lan'
        option 'interface' 'lan'
        option 'start' '100'
        option 'leasetime' '12h'
        option 'limit' '50'

Now upon restarting the service, dnsmasq script generators will be setting up the dhcp range, and is reported back in syslog as:

Code: Select all

OpenWrt daemon.info dnsmasq-dhcp[1198]: DHCP, IP range 192.168.1.100 -- 192.168.1.150, lease time 12h

Unless the dnsmasq man page is inaccurate too, let me quote:

-F, --dhcp-range=[interface:<interface>,][tag:<tag>[,tag:<tag>],][set:<tag],]<start-addr>,<end-addr>[,<netmask>[,<broadcast>]][,<lease time>]

Enable the DHCP server. Addresses will be given out from the range <start-addr> to <end-addr> and from statically defined addresses given in dhcp-host options.

If I'm not mistaken, dnsmasq will be providing addresses from 100-150 inclusive as per the man page, which is a total of 51 addresses, which is _not_ equal to the limit "50" as specified in the config line. I've reopened the bug in OpenWRT related to this https://dev.openwrt.org/ticket/2240.

With respect to gargoyle, if this bug is fixed, then the webif will be correct. So setting start=100 and end=149 in webif will write

Code: Select all

option 'start' '100'
option 'limit' '50'

which is the correct logic for having an ip range from 100-149, a total of 50 inclusive addresses.

[braveheartleo]

Feature Requests:

3. I think it makes a lot more sense to link to a domain in question than a report on the visitors/geolocation of the domain. The former will likely give you a clue as to what it might be about, while the latter will not. You'll note if you follow your link that I will learn where Gargoyle is being hosted, but it won't tell me that it is a site dedicated to router firmware. I really think actually visiting the site is a LOT more informative.

What I had in mind was to give the user an informed choice before proceeding with visiting the actual address, apart from my previous comment that visiting sub-domains, especially second-level sub-domains and higher of websites as this would be common, would not much offer anything informative, if not redirecting to the main website itself.

This goes the same way with something like "DO NOT click weblinks on emails with carefreeness, espcecially from senders you don't know." Taking the implied security of that advice into this context, it will be prudent to not link directly to the domain in question but first tell something about it, something that users might find them to their surprise, informative enough to provide more "identity" to a web url in the face of growing phishing attacks, and make them aware of the essentials of becoming familiar with a webserver's geolocation.

After all, in case the user needs to visit the domain to see for herself, it can be done by simply editing the address so the site can be visited directly, unless of course such task can be considered a "loss" in accessibility instead replaced with a better information service providing entity that a user might not even be aware of. That will be hitting two birds with one stone: providing information relevant to the domain and visiting the domain where appropriate to find out what it is about, or better yet googling said domain.

I respect your take on this matter, and I'm just adding my two cents worth.

Cheers!

[braveheartleo]

UPDATE DHCP Issue:

Changed 11 hours ago by jow
Correction of the statement above: "Start" is the offset from the network address and specifies the minimum allowed IP "Limit" is the offset from the minimum IP specifying the highest allowable IP

Therefore there _is_ an issue, rather a logic issue that must be corrected in the webif.

More info here: https://dev.openwrt.org/ticket/2240

[braveheartleo]

Another discovered bug is with the qos up and down pages, where having rules with min and max pkt matches are inversed when saving changes, and reverts back after another save.

This has been present like forever as confirmed by and fixed with the help of pbix, so I just thought posting here might be relevant. The offending file is /www/js/qos.js.

[braveheartleo]

Another feature request. Can we provide gargoyle the ability to specify additional files to be backed up?

This doesn't have to have a ui in webif, because such a function is for those that have advanced setups like mine. For example I have made some significant config changes involving files not normally included in gargoyle's config backup, such as:

1. /etc/dnsmasq.conf where I have defined several servers and other functions not provided in the standard dhcp config file.

2. /etc/miniupnpd.conf for enabling nat-pmp and passing other parameters not provided in the upnpd config file.

3. /etc/init.d/custom-user-scripts is a custom startup script I have setup where I can specify commands to be executed every boot.

And others. Rewriting those line changes in each of the config files every configuration restore can be bothersome. Having this ability can be very flexible. I'm thinking of adding another parameter in the gargoyle config file where one can specify files to be included.

As a workaround I have to edit /usr/lib/gargoyle/create_backup.sh and edit the relevant line to include additional files upon backup in webif.

[Eric]

I'm not inclined to implement a feature like this in the interface just for people who are editing these files on the command line.

However, there's a really easy way for you to make sure these keep getting backed up for you even after you restore: You say you've already edited /usr/lib/gargoyle/create_backup.sh so that the files you edited get backed up. So... all you have to do is add an entry for /usr/lib/gargoyle/create_backup.sh to itself. Then whenever you backup/restore, you save your custom backup script too. That's even easier than setting it in the web interface every time.

[braveheartleo]

First of all congratulations on your efforts at finally implementing quotas with shaping. I'm seeing that Gargoyle has a great road ahead, and I want to be any help I can on this journey to greatness.

Second, it's nice to see that I have landed most of the bugs that are fixed in 1.3.4. I will continue to hunt bugs however I can.

I'm not inclined to implement a feature like this in the interface just for people who are editing these files on the command line.

About my recent feature request, I was thinking more about adding additional config options instead of having a UI in webif. Something like this:

Code: Select all

/etc/config/gargoyle:

config 'backup' 'files_to_keep'
  option 'name1' '/path/to/name1'
  option 'name2' '/path/to/name2'

This should be more elegant, code-wise. If only I could start understanding how parsing configs are done in OpenWRT, then adding code that will look for the section above in /etc/config/gargoyle can be added to /usr/lib/gargoyle/create_backup.sh.

I have also considrered adding an entry so that create_backup.sh will also backup itself and be included when restoring backups. However, I'm hesistant in doing so because in the long run some code changes might be implemented in create_backup.sh and I might forget that it is included in the last backup I have made, so that restoring will override the changes with the old copy of the file I have kept with the backups.

But for now I suppose it should be ok to do so.