Possible WG Widget bug

[jfenech]

Hi,

Just upgraded to OPNsense 24.7.2-amd64. Love the new controls on the widgets.

It seems though that the WireGuard widget is not showing WG0 in the status, see screenshot below.

It states correctly there are 3 tunnels (which are WG0, WG1, WG2) but is only showing status for WG1 and WG2

[franco]

wg0 is not permitted for framework reasons since quite a while. Maybe we can fix the widget, but only in exchange for a GitHub ticket. Deal?


Cheers,
Franco

[jfenech]

Hi Franco,

Was just gonna post that the header / row seem to be Generated correctly (console.log (header)) just before super.updateTable('wgTunnelTable', [[header, row]], tunnel.publicKey);

returns this :

                <div style="display: flex; justify-content: space-between; align-items: center;">
                    <div style="display: flex; align-items: center;">
                        <i class="fa fa-exchange text-success wireguard-interface" style="cursor: pointer;"
                            data-toggle="tooltip" title="Online">
                        </i>
                        &nbsp;
                        <span><b>wg0 (NordVPN_Instance_XX_1) </b></span>
                    </div>
                </div>
                <div style="display: flex; justify-content: space-between; align-items: center;">
                    <div style="display: flex; align-items: center;">
                        <i class="fa fa-exchange text-success wireguard-interface" style="cursor: pointer;"
                            data-toggle="tooltip" title="Online">
                        </i>
                        &nbsp;
                        <span><b>wg1 (NordVPN_Instance_XX_2) </b></span>
                    </div>
                </div>

                <div style="display: flex; justify-content: space-between; align-items: center;">
                    <div style="display: flex; align-items: center;">
                        <i class="fa fa-exchange text-success wireguard-interface" style="cursor: pointer;"
                            data-toggle="tooltip" title="Online">
                        </i>
                        &nbsp;
                        <span><b>wg2 (NordVPN_Instance_XX_3) </b></span>
                    </div>
                </div>
    }

So it seems to be related to the framework.

If WG0 is not permitted, should I just  change my config, or do you still want to open a ticket to fix ?

[doktornotor]

Nice investigation. That said, the lead developer already told you beforehand that's been explicitly forbidden by the framework for quite some time - which you have just confirmed. 🥇🏆

[franco]

Let's agree on a ticket to investigate, ok?

We never migrated existing wg0 instances and they do continue to work. If the outcome is to migrate wg0 automatically to something wgX still free that's also fine. Whatever is easier.


Cheers,
Franco

[meyergru]

FWIW, I had an old entry with wg0 as well. You can correct this by saving your config.xml, then look for something like this:

Code: [Select]

      <server uuid="1a43f632-c3c0-41aa-a72e-e029cd53624b">
            <enabled>0</enabled>
            <name>dummy_instance</name>
            <instance>0</instance>
            <pubkey>TWhRzosD0WTmMzmI3ObQnKVvcQ8JhwDUgtr8VvbMakk=</pubkey>
            <privkey>uG6aKC5LEIIbTgW0yD66Opotj5b76T2agJP2oasdd22=</privkey>
            <port>606</port>
            <mtu/>
            <dns>103.86.96.100,103.86.99.100</dns>
            <tunneladdress>10.4.0.2/16</tunneladdress>
            <disableroutes>1</disableroutes>
            <gateway>10.4.0.1</gateway>
            <carp_depend_on/>
      </server>

Replace the <instance>0</instance> by an unused number X, save, then restore only the wireguard part of the configuration. Et voila, wg0 is replace with wgX!

[doktornotor]

  I'd say the risks of migrating are much higher than a "broken"widget. Any modifications attempted on the tunnel will tell the user to change the naming, or... ? But yes, that's third time just today where users don't understand the concept of opening a ticket at GitHub. 🤔🤷‍♂️

[franco]

That works too but doesn't scale exceptionally well. I suppose there will still be a number of people with wg0 out in the wild.

Also if you do this it would be safest to reboot so wg0 is gone, because if you just fudge the config.xml it will not be maybe leading to clashes with the new wgX.


Cheers,
Franco

[franco]

Doing an automatic migration in a major upgrade is probably the safest thing, but I did say whatever is easier. Sometimes things like that don't pan out nicely indeed (although the migration code is a lot better to handle than a config.xml revision change -- hint, hint).


Cheers,
Franco

[doktornotor]

❤️🍻

[jfenech]

For the sake of completeness, I have changed WG0 to WG1, WG1 to WG2, and WG2 to WG3, by manipulating the config.xml as suggested above and rebooting. I also had to update the interface assignments from the UI after.

The name change was successful, all connections are up and routing traffic however the widget still shows 2 entries only.

I have deleted the widget and re-created it to no avail

Github ticket @doktonotor is nearly complete, with all the info. I do understand the concept btw. It was just that I did not want to open a ticket just in case, I had some remote case with a wrongly named interface for whatever reason which would not merit an investigation at all.

I will be linking the screenshots in this post in the ticket too.

Thank you all for your replies.

(https://github.com/opnsense/core/issues/7812)

[jfenech]

To put this to bed once and for all, in my specific case the problem was that I have two tunnels with the exact same public key so when this gets executed

Code: [Select]

super.updateTable('wgTunnelTable', [[header, row]], tunnel.publicKey);
The second instance, overwrites the first instance coz they have the same index (tunnel.publicKey).

My workaround I added an index to the forEach loop and  the following :

Code: [Select]

let modifiedPublicKey = `${tunnel.publicKey}-${index}`;

    // Update the HTML table with the sorted rows
    super.updateTable('wgTunnelTable', [[header, row]], modifiedPublicKey);[/font]

So the full function would look like :

Code: [Select]

tunnels.forEach((tunnel, index) => { // 'index' gives you the current position in the loop
    let header = `
        <div style="display: flex; justify-content: space-between; align-items: center;">
            <div style="display: flex; align-items: center;">
                <i class="fa ${tunnel.statusIcon} wireguard-interface" style="cursor: pointer;"
                    data-toggle="tooltip" title="${tunnel.connected ? this.translations.online : this.translations.offline}">
                </i>
                &nbsp;
                <span><b>${tunnel.ifname}</b></span>
            </div>
        </div>`;
    let row = `
        <div>
            <span><a href="/ui/wireguard/general#peers">${tunnel.name}</a> | ${tunnel.allowed_ips}</span>
        </div>
        <div>
            ${tunnel.latest_handshake_fmt
                ? `<span>${tunnel.latest_handshake_fmt}</span>
                   <div style="padding-bottom: 10px;">
                       <i class="fa fa-arrow-down" style="font-size: 13px;"></i>
                       ${tunnel.rx}
                       |
                       <i class="fa fa-arrow-up" style="font-size: 13px;"></i>
                       ${tunnel.tx}
                   </div>`
                : `<span>${this.translations.disconnected}</span>`}
        </div>`;

    // Add '-0', '-1', etc. to the public key using the index
    let modifiedPublicKey = `${tunnel.publicKey}-${index}`;

    // Update the HTML table with the sorted rows
    super.updateTable('wgTunnelTable', [[header, row]], modifiedPublicKey);
});

To further confirm, I have also restored my original configuration with the first interface called WG0, and the widget still works.


Will add this info to the ticket too

Thank you all for your help

J

[jfenech]

Screenshot of the working widget

[franco]

We were discussing today. It's probably possible to just use ifname as the key and be done with it.


Cheers,
Franco

[jfenech]

That should be unique yeah. Or else just use the ${index} which just increments on every iteration.

Thanks again for all the replies and time on this.