opnsense / ports

OPNsense ports on top of FreeBSD
https://opnsense.org/
Other
157 stars 112 forks source link

[23.7.8] Firewall - Diagnostics - Sessions: Rule column shown as null #182

Closed doktornotor closed 9 months ago

doktornotor commented 9 months ago

Important notices

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

Describe the bug

Firewall - Diagnostics - Sessions: "Rule" is shown as null. See https://forum.opnsense.org/index.php?topic=36896.0

To Reproduce

Steps to reproduce the behavior:

  1. Upgrade to 23.7.8
  2. Go to Firewall - Diagnostics - Session.

Expected behavior

Get a rule reference via the proper label.

Additional context

Definitely related to the broken upstream label patch in kernel.

Add any other context about the problem here.

OPNsense 23.7.8 (amd64}

AdSchellevis commented 9 months ago

you did try to reinstall the base system? system: firmware: packages, reinstall base. the unlucky ones upgraded shortly after release may have the old version containing the upstream label issue indeed.

doktornotor commented 9 months ago

I only installed it today. The kernel/labels are fine otherwise, it's just the pftop output.

fichtner commented 9 months ago

Pftop was modified for libpfctl use. Can you confirm?

# opnsense-revert -r 23.7.7 pftop
doktornotor commented 9 months ago

Yes, working now.

fichtner commented 9 months ago

Meh, not sure how to proceed? What do you suggest?

doktornotor commented 9 months ago

LOL, well I have the labels back with firewall rules and pftop 0.8_4 seems to be working with that. Did the "sponsored" commit actually fix some real issue that someone reported before?

Someone also mentioned some weird issue with the dashboard widget but I cannot reproduce that plus seems more logical that it would not work at all if that was related, regardless of which interfaces are selected - this is the relevant forum post

fichtner commented 9 months ago

I think for the empty widget this still stands: https://forum.opnsense.org/index.php?topic=36896.msg180592#msg180592

The widget's JS broke rendering the dashboard useless and as a side effect it was empty. But if you have a filter or no log output the widget is empty too but the dashboard keeps working since the base fix (cross check against label display on live log).

About libpfctl in ports... what's strange is that a ports version is used but that needs to do cross-compatibility between FreeBSD "15", 14 and 13 and I'm not sure it does. Why not link against the base library in that case... because they all do the right thing unless you wanted to wedge new features in libpfctl and use it in pftop right away and break compat with 14 and or 13?

fichtner commented 9 months ago

Relevant commit https://github.com/opnsense/ports/commit/d46bf725e7

doktornotor commented 9 months ago

Yeah I found that commit and could not make sense of it, looked like someone accidentally deleted files.

Does the code actually work on FreeBSD 14 (Release)? No idea about ETA for v14 in OPNsense, but reverting all this is just stopgap measure otherwise (not necessarily a bad one - this is supposed to be stable code, not such a mess due to upstream backporting incompatible changes.)

fichtner commented 9 months ago

Does it work? Definitely maybe. I think the obvious fix is using libpfctl from base. I’ll add an option to test this on Monday. If that works we push this upstream to see what the plan is.

fichtner commented 9 months ago

Just as a side note a lot of code has NOT been backported to stable/13 (guess who is not using FreeBSD 13) causing this drift in code and potential for regression. But I’m more scared of 14.0 in that regard and our policy is 14.1 anyway for sanity. I don’t think 14.0 is all that ready and the bug tracker agrees, but eventually something has to be pushed out and to be used in order to find the leftover bugs (if any exist on 14.1 still).

13.1 was really smooth BTW for one reason or another.

fichtner commented 9 months ago

@doktornotor how's this one on 5808f51?

# opnsense-revert -z pftop
fichtner commented 9 months ago

If that's not the issue we are looking at https://github.com/grembo/pftop/commit/2489210f9a most likely. That would be my favourite issue because then we could just report to the "upstream" repo.

doktornotor commented 9 months ago

@doktornotor how's this one on 5808f51?

# opnsense-revert -z pftop

Well, that one produces the nice nulls again.

 # opnsense-revert -z pftop
Fetching pftop.pkg: ... done
Verifying signature with trusted certificate pkg.opnsense.org.20230717... done
pftop-0.8_4: already unlocked
Updating OPNsense repository catalogue...
OPNsense repository is up to date.
Updating mimugmail repository catalogue...
mimugmail repository is up to date.
All repositories are up to date.
Checking integrity... done (0 conflicting)
The following 1 package(s) will be affected (of 0 checked):

New packages to be INSTALLED:
        pftop: 0.9 [unknown-repository]

Number of packages to be installed: 1
[1/1] Installing pftop-0.9...
Extracting pftop-0.9: 100%

image

fichtner commented 9 months ago

thanks, give me an hour... need to fix an OpenSSL 3 thing first

fichtner commented 9 months ago

How about https://github.com/opnsense/ports/commit/5a84f7dc9 then?

# opnsense-revert -z pftop
doktornotor commented 9 months ago

Hmmm, that's still null.

fichtner commented 9 months ago

https://github.com/grembo/pftop/commit/bcef03b59

doktornotor commented 9 months ago

Looks promising. will be able to test this evening if you recompile it again, need to get some other work done meanwhile. Thanks.

fichtner commented 9 months ago

I could reproduce via /usr/local/sbin/pftop -w 1000 -b -v long 9999999999999 | head so I'll just throw 0.10 into today's hotfix for unrelated reasons. See https://github.com/opnsense/ports/commit/ac50fa94

fichtner commented 9 months ago

and thanks for spotting this quickly ❤️