firewall: rules [new]: port selection doesn't have "well-known" ports list in dropdown

[fichtner]

Important notices

Our forum is located at https://forum.opnsense.org , please consider joining discussions there in stead of using GitHub for these matters.

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

• I have read the contributing guide lines at https://github.com/opnsense/core/blob/master/CONTRIBUTING.md
• I am convinced that my issue is new after having checked both open and closed issues at https://github.com/opnsense/core/issues?q=is%3Aissue

Selecting TCP etc and src/dst port doesn't have well known listed.

[Monviech]

I wanted to add something but during that time it was decided against it. Here's the comment I left:

core/src/opnsense/mvc/app/controllers/OPNsense/Firewall/Api/FilterBaseController.php

Line 219 in 7062992

// XXX: Well known ports could be gathered from /etc/services but there is a lot of noise

[fichtner]

Ok, but it’s available in DNAT, isn’t it?

[Monviech]

Not right now, all new NAT and the rules view use the same port selector above.

You can still type the well known port in manually and it validates.

Try "http" as port manually.

[fichtner]

Right, sorry, I confused this with the protocol selector. :)

[Monviech]

@fichtner Where would you add a method that returns these? I would like to keep a central list somewhere, so that validation and what the port selector returns are the same.

E.g. right now it exists here, but I'm sure if I add a public method to a field type that returns this, and then use it in the controller, it's "wrong" in some way.

core/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/PortField.php

Line 42 in cf0341f

private static $wellknownservices = [

[fichtner]

@Monviech something along the lines of e724f76 but I'm not sure about the whole construct that is listPortSelectOptionsAction() e.g. we don't really care if well known is enabled or not, if any is used, etc which would be in the actual PortField, not the static dummy. Maybe @AdSchellevis has a better idea.

[AdSchellevis]

@fichtner the limited set of "well known" shouldn't be much of an issue indeed to add, the full services list is problematic. e724f76 looks simple and pragmatic, although I'm doubting a bit if we shouldn't just push the list to a static in the Util class for reuse instead of using the porttype class, but that's also a matter of taste to be honest (not sure myself either yet).

[fichtner]

Keeping it in the port field could help with steering the use of well-known allowed or any allowed but at the downside of more computation. A static util is also fine. I think that’s what a lot of old code is using too.

[AdSchellevis]

Currently I don't think I have very strong feelings about either option, starting with e724f76 and reassess when needed later also works for me.