Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
2 tasks done
doktornotor opened this issue Nov 11, 2023 · 21 comments
Closed
2 tasks done
Labels
upstream Third party issue

Comments

@doktornotor
Copy link

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
Copy link
Member

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
Copy link
Author

doktornotor commented Nov 11, 2023

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

@fichtner
Copy link
Member

Pftop was modified for libpfctl use. Can you confirm?

# opnsense-revert -r 23.7.7 pftop

@doktornotor
Copy link
Author

Yes, working now.

@fichtner
Copy link
Member

fichtner commented Nov 11, 2023

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

@doktornotor
Copy link
Author

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 fichtner transferred this issue from opnsense/src Nov 12, 2023
@fichtner fichtner added the upstream Third party issue label Nov 12, 2023
@fichtner
Copy link
Member

fichtner commented Nov 12, 2023

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
Copy link
Member

Relevant commit d46bf725e7

@doktornotor
Copy link
Author

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
Copy link
Member

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
Copy link
Member

fichtner commented Nov 12, 2023

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 added a commit that referenced this issue Nov 13, 2023
So we have a "libpfctl" for abstraction but the base system
a particular "libpfctl" originates from does not provide the
libary so we need to jump through a couple of hoops to embed
the libary into the pftop build in order to retain compatibility
on stable/13?
@fichtner
Copy link
Member

@doktornotor how's this one on 5808f51?

# opnsense-revert -z pftop

@fichtner
Copy link
Member

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

@doktornotor
Copy link
Author

@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
Copy link
Member

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

fichtner added a commit that referenced this issue Nov 13, 2023
This reverts commit 5808f51.
libpfctl does not appear to be the issue here.
@fichtner
Copy link
Member

How about 5a84f7dc9 then?

# opnsense-revert -z pftop

@doktornotor
Copy link
Author

Hmmm, that's still null.

@fichtner
Copy link
Member

grembo/pftop@bcef03b59

@doktornotor
Copy link
Author

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

@fichtner
Copy link
Member

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 ac50fa94

@fichtner
Copy link
Member

and thanks for spotting this quickly ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream Third party issue
Development

No branches or pull requests

3 participants