opnsense / core

OPNsense GUI, API and systems backend
https://opnsense.org/
BSD 2-Clause "Simplified" License
3.36k stars 754 forks source link

Firewall - Rules -WAN - unable to expand the Automatically generated rules with auto-added VPN rules and "legacy" IPsec tunnel(s) #6991

Closed doktornotor closed 1 year ago

doktornotor commented 1 year ago

Important notices

Before you add a new report, we ask you kindly to acknowledge the following:

Describe the bug

The button just does not do anything. Only happens with WAN interface (PPPoE/DHCPv6). It works fine with other interfaces. The floating rules button works just fine even on the WAN interface. Tested with latest stable Edge and Chrome, in incognito mode as well. Nothing obvious in the browser console either, beyond "normal" complaints that jquery-3.5.1.min.js takes long time - see below.

To Reproduce

Steps to reproduce the behavior:

  1. Go to 'Firewall: Rules: WAN'
  2. Click the button next to 'Automatically generated rules'
  3. Nothing happens.

Expected behavior

Expand the autogenerated rules list.

Relevant log files

Nothing obvious in the browser console either, beyond complaints that the jquery-3.5.1.min.js takes long time.

[Violation]'click' handler took <N>ms
jquery-3.5.1.min.js:2 [Violation]'click' handler took 235ms
jquery-3.5.1.min.js:2 [Violation]'click' handler took 187ms
jquery-3.5.1.min.js:2 [Violation]'click' handler took 189ms
jquery-3.5.1.min.js:2 [Violation]'click' handler took 179ms
[Violation]Forced reflow while executing JavaScript took <N>ms
[Violation]Forced reflow while executing JavaScript took 204ms
[Violation]Forced reflow while executing JavaScript took 78ms
[Violation]Forced reflow while executing JavaScript took 161ms
[Violation]Forced reflow while executing JavaScript took 164ms

Additional context

Forums thread for discussion

Environment

Software version used and hardware type if relevant, e.g.:

OPNsense 23.7.7_3 (amd64)

doktornotor commented 1 year ago

Just noting here as well, the GUI restart did not fix this one. Also testing with an incognito browser instance, no add-ons there.

fichtner commented 1 year ago

Yeah, this is not part of MVC so the other thing was just a false flag.

fichtner commented 1 year ago

I'd like to fix this but don't know how. is the DOM structure damaged during the rendering of an automatic rule that makes this impossible to expand?

fichtner commented 1 year ago

Unfortunately, a (non-functional) test setup for PPPoE/DHCPv6 doesn't show this by default. Could also be in combination with another option or language setting worst case?

doktornotor commented 1 year ago

Language, you mean browser or GUI? Using English for the GUI. Really no idea. Not sure whather this config snippet helps (or which other parts would you like to have?)

  <interfaces>
    <wan>
      <if>pppoe0</if>
      <descr>WAN</descr>
      <enable>1</enable>
      <lock>1</lock>
      <spoofmac/>
      <ipaddr>pppoe</ipaddr>
      <ipaddrv6>dhcp6</ipaddrv6>
      <dhcp6-ia-pd-len>8</dhcp6-ia-pd-len>
      <dhcp6-ia-pd-send-hint>1</dhcp6-ia-pd-send-hint>
      <dhcp6usev4iface>1</dhcp6usev4iface>
      <adv_dhcp6_interface_statement_send_options/>
      <adv_dhcp6_interface_statement_request_options/>
      <adv_dhcp6_interface_statement_information_only_enable/>
      <adv_dhcp6_interface_statement_script/>
      <adv_dhcp6_id_assoc_statement_address_enable/>
      <adv_dhcp6_id_assoc_statement_address/>
      <adv_dhcp6_id_assoc_statement_address_id/>
      <adv_dhcp6_id_assoc_statement_address_pltime/>
      <adv_dhcp6_id_assoc_statement_address_vltime/>
      <adv_dhcp6_id_assoc_statement_prefix_enable/>
      <adv_dhcp6_id_assoc_statement_prefix/>
      <adv_dhcp6_id_assoc_statement_prefix_id/>
      <adv_dhcp6_id_assoc_statement_prefix_pltime/>
      <adv_dhcp6_id_assoc_statement_prefix_vltime/>
      <adv_dhcp6_prefix_interface_statement_sla_len/>
      <adv_dhcp6_authentication_statement_authname/>
      <adv_dhcp6_authentication_statement_protocol/>
      <adv_dhcp6_authentication_statement_algorithm/>
      <adv_dhcp6_authentication_statement_rdm/>
      <adv_dhcp6_key_info_statement_keyname/>
      <adv_dhcp6_key_info_statement_realm/>
      <adv_dhcp6_key_info_statement_keyid/>
      <adv_dhcp6_key_info_statement_secret/>
      <adv_dhcp6_key_info_statement_expire/>
      <adv_dhcp6_config_advanced/>
      <adv_dhcp6_config_file_override/>
      <adv_dhcp6_config_file_override_path/>
    </wan>
    <lan>
      <if>igb1</if>
      <descr>LAN</descr>
      <enable>1</enable>
      <lock>1</lock>
      <spoofmac/>
      <ipaddr>192.168.100.254</ipaddr>
      <subnet>24</subnet>
      <ipaddrv6>track6</ipaddrv6>
      <track6-interface>wan</track6-interface>
      <track6-prefix-id>0</track6-prefix-id>
      <dhcpd6track6allowoverride>1</dhcpd6track6allowoverride>
    </lan>
  <ppps>
    <ppp>
      <ptpid>0</ptpid>
      <type>pppoe</type>
      <if>pppoe0</if>
      <ports>igb0</ports>
      <username>vdsl</username>
      <password>dmRzbA==</password>
      <descr>T-Mobile CZ VDSL</descr>
      <provider/>
      <bandwidth/>
      <mtu/>
      <mru/>
      <mrru/>
    </ppp>
  </ppps>
fichtner commented 1 year ago

Hmm, no luck. You are seeing the badge with the count (non-zero) but clicking doesn't help?

fichtner commented 1 year ago

Can you try https://github.com/opnsense/core/commit/94b269a8 just to be sure we're not looking at a broken rendering? Somehow the code hides it multiple times. That could be an issue, but hard to debug :/

doktornotor commented 1 year ago

Can you try 94b269a8 just to be sure we're not looking at a broken rendering? Somehow the code hides it multiple times. That could be an issue, but hard to debug :/

Yes, this works on all interfaces, the autogenerated rules are expanded automatically on page load. (Wrt the previous comment, yes, I can see the badge but it does not do anything when you click it.)

fichtner commented 1 year ago

Are you using categories or groups?

doktornotor commented 1 year ago

Yes, using categories. Groups - you mean IPsec/OpenVPN/WG? That'd be a yes as well.

fichtner commented 1 year ago

Firewall: Groups entries specifically ,but I don't see an overlap there. Categories get merged into the rules table, however. Selecting a non-matching category makes the rule sets not expand but then the behaviour is consistent between automatic and floating.

doktornotor commented 1 year ago

Firewall: Groups entries specifically

Well, there's just openvpn and enc0 listed, nothing manually added there.

fichtner commented 1 year ago

Last try for today.. this seemed odd 556d394

doktornotor commented 1 year ago

Ok, that does not work (reverted the previous debug commit, applied this one.) Messing with the developer console, all the rules are there, but hidden.

<tr class="rule internal-rule" style="background-color: inherit; display: none;" data-category="">

Will play with it later. Thanks.

doktornotor commented 1 year ago

Ok, maybe I found something. In the browser, for the WAN interface specifically, when I choose "Inspect" - that badge button is listed twice in Event Listeners -> Click. This is not the case with any other interface.

WAN: image

LAN or anything else: image

With those two listeners, clicking on the badge produces no action. When I remove one of them, clicking indeed toggles the is_collapsed button state, but the rules are still not expanded.

fichtner commented 1 year ago

Is there more than one occurrence of "expand-internal" in the source code generated by the page? The more I look at the JS code the more sense it makes and more mysterious this bug becomes.

doktornotor commented 1 year ago

Yes, it is twice in the source code as well. Hundreds of lines apart. Adding some context before as well.

Those <!--StartFragment--> tags and | chars are from Chrome copy-paste, ignore.

Around line 1555

<!--StartFragment-->
<td class="text-nowrap">
--
  | <strong>Description</strong>
  | <i class="fa fa-question-circle" data-toggle="collapse" data-target=".rule_md5_hash" ></i>
  | </td>
  | <td class="text-nowrap button-th">
  | <a href="firewall_rules_edit.php?if=wan" class="btn btn-primary btn-xs" data-toggle="tooltip" title="Add">
  | <i class="fa fa-plus fa-fw"></i>
  | </a>
  | <button id="move_26" name="move_26_x" data-toggle="tooltip" title="Move selected rules to end" class="act_move btn btn-default btn-xs">
  | <i class="fa fa-arrow-left fa-fw"></i>
  | </button>
  | <button id="del_x" title="Delete selected" data-toggle="tooltip" class="act_delete btn btn-default btn-xs">
  | <i class="fa fa-trash fa-fw"></i>
  | </button>
  | <button title="Enable selected" data-toggle="tooltip" class="act_toggle_enable btn btn-default btn-xs">
  | <i class="fa fa-check-square-o fa-fw"></i>
  | </button>
  | <button title="Disable selected" data-toggle="tooltip" class="act_toggle_disable btn btn-default btn-xs">
  | <i class="fa fa-square-o fa-fw"></i>
  | </button>
  | </td>
  | </tr>
  | <tr id="expand-internal-rules" class="expand_type is_collapsed" data-type="internal" style="display: none;">
  | <td><i class="fa fa-folder-o text-muted"></i></td>
  | <td></td>
  | <td class="view-info" colspan="2"> </td>
  | <td class="view-info hidden-xs hidden-sm" colspan="5"> </td>
  | <td colspan="2" class="view-stats hidden-xs hidden-sm"></td>
  | <td colspan="2" class="view-stats"></td>
  | <td class="view-info"></td>
  | <td>Automatically generated rules</td>
  | <td>
  | <button class="btn btn-default btn-xs" id="expand-internal">
  | <i class="fa fa-chevron-circle-down" aria-hidden="true"></i>
  | <span class="badge">
  | <span id="internal-rule-count"><span>
  | </span>
  | </button>
  | </td>
  | </tr>

<!--EndFragment-->

And then around line 2700

<!--StartFragment-->
<tr id="expand-internal-rules" class="expand_type is_collapsed" data-type="internal" style="display: none;">
--
  | <td><i class="fa fa-folder-o text-muted"></i></td>
  | <td></td>
  | <td class="view-info" colspan="2"> </td>
  | <td class="view-info hidden-xs hidden-sm" colspan="5"> </td>
  | <td colspan="2" class="view-stats hidden-xs hidden-sm"></td>
  | <td colspan="2" class="view-stats"></td>
  | <td class="view-info"></td>
  | <td>Automatically generated rules</td>
  | <td>
  | <button class="btn btn-default btn-xs" id="expand-internal">
  | <i class="fa fa-chevron-circle-down" aria-hidden="true"></i>
  | <span class="badge">
  | <span id="internal-rule-count"><span>
  | </span>
  | </button>
  | </td>
  | </tr>

<!--EndFragment-->

😖😕

fichtner commented 1 year ago

Which rules are in the second segment? for some reason it looks like the rules are not sorted properly and this is then failing to avoid duplication of the expand header:

https://github.com/opnsense/core/blob/db4b90d218296826d37b7ce3fa368c274408f401/src/www/firewall_rules.php#L785

(yay, progress!)

doktornotor commented 1 year ago

The segment after the ~2700 line occurence? Well, those are the manually created rules on WAN.

doktornotor commented 1 year ago

Uhm... Scratch that. Those are a little bit below. The ones directly after the second badge come from the "legacy" IPsec tunnels. Autogenerated.

doktornotor commented 1 year ago

And with that, the mystery is solved. " Disable all auto-added VPN rules" -> button immediately working. Re-enable -> button broken again.

fichtner commented 1 year ago

Handing this over to @AdSchellevis … fix should be easy now that we know. Thanks!

doktornotor commented 1 year ago

Thanks for the help with debugging. (As a bonus, I am now able to break the button on any/all interfaces, just adjust the legacy tunnel interface accordingly. LOL.)