OPNsense Forum

English Forums => 24.7, 24.10 Series => Topic started by: jfenech on August 24, 2024, 09:44:20 PM

Title: Possible WG Widget bug
Post by: jfenech on August 24, 2024, 09:44:20 PM
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



Title: Re: Possible WG Widget bug
Post by: franco on August 24, 2024, 09:45:59 PM
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
Title: Re: Possible WG Widget bug
Post by: jfenech on August 24, 2024, 10:21:15 PM
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 ?


Title: Re: Possible WG Widget bug
Post by: doktornotor on August 24, 2024, 10:34:59 PM
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. 🥇🏆
Title: Re: Possible WG Widget bug
Post by: franco on August 24, 2024, 10:37:21 PM
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
Title: Re: Possible WG Widget bug
Post by: meyergru on August 24, 2024, 10:41:13 PM
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:


      <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!
Title: Re: Possible WG Widget bug
Post by: doktornotor on August 24, 2024, 10:41:58 PM
 8) 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. 🤔🤷‍♂️
Title: Re: Possible WG Widget bug
Post by: franco on August 24, 2024, 10:43:48 PM
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
Title: Re: Possible WG Widget bug
Post by: franco on August 24, 2024, 10:45:22 PM
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
Title: Re: Possible WG Widget bug
Post by: doktornotor on August 24, 2024, 10:48:48 PM
❤️🍻
Title: Re: Possible WG Widget bug
Post by: jfenech on August 24, 2024, 11:42:13 PM
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)
Title: Re: Possible WG Widget bug
Post by: jfenech on August 25, 2024, 08:55:25 AM
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

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 :



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 :

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
Title: Re: Possible WG Widget bug
Post by: jfenech on August 25, 2024, 09:08:00 AM
Screenshot of the working widget
Title: Re: Possible WG Widget bug
Post by: franco on August 26, 2024, 07:31:48 PM
We were discussing today. It's probably possible to just use ifname as the key and be done with it.


Cheers,
Franco
Title: Re: Possible WG Widget bug
Post by: jfenech on August 26, 2024, 08:14:25 PM
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.
Title: Re: Possible WG Widget bug
Post by: franco on August 26, 2024, 08:16:33 PM
My colleague promised to look at it, but I'm not sure it's done until 24.7.3 needs to be built and shipped this week.


Cheers,
Franco
Title: Re: Possible WG Widget bug
Post by: Spokesman5636 on August 27, 2024, 06:31:52 PM
I found this thread whilst searching for the same thing, turns out my WireGuard tunnels to NordVPN have the same public keys too. Happy to see it's already been reported and in the process of being fixed. Thanks!
Title: Re: Possible WG Widget bug
Post by: chemlud on August 27, 2024, 06:44:46 PM
fwiw my wg0 are showing up in the Dashboard widget...
Title: Re: Possible WG Widget bug
Post by: franco on August 27, 2024, 08:30:41 PM
Yep, this wasn't about wg0 as initially thought. It's just about duplicated public keys.


Cheers,
Franco