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.

Sign up for GitHub

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

Jump to bottom

feat(usr/lib): Add firewalld configuration files #186

Merged
merged 1 commit into from
Dec 15, 2023
Merged

feat(usr/lib): Add firewalld configuration files #186

merged 1 commit into from
Dec 15, 2023

Conversation

ondrejholy
Copy link
Contributor

The project already contains UFW configuration files, but firewalld configuration files are missing. Let's add a copy of firewalld configuration files used in Fedora.

@christgau
Copy link
Owner

I have two comments/suggestions

  1. Would it make sense to merge the two files? THB, I don't know what conventions for firewalld apply. Or is that not possible since the definition for the multicast traffic (wsdd.xml) would also restrict the HTTP traffic, which would also not make sense from the technical point of view...
  2. can we move the two files to etc/firewalld/services?

@ondrejholy
Copy link
Contributor Author

Thanks for your reply:

  1. This is exactly how it is packaged under Fedora. I will try to find an answer, but probably not before January. But it seems likely that you have answered your question already...
  2. The preferred location for distro-managed services is /usr/lib, but if you wish, I should not be a problem putting this under /etc inside git repository and extracting this to /usr/lib downstream.

@ondrejholy @christgau
feat(usr/lib): Add firewalld configuration files
5d0776b 
The project already contains UFW configuration files, but firewalld
configuration files are missing. Let's add a copy of firewalld configuration
files used in Fedora.
@christgau christgau force-pushed the wip/oholy/firewalld-configuration-files branch from c809caf to 5d0776b Compare December 15, 2023 20:52
@christgau christgau merged commit 942bd7e into christgau:master Dec 15, 2023
3 checks passed
@hgraeber
Copy link

firewalld already has a service file "ws-discovery-udp.xml" that is identical to your "wsdd.xml", except for the include of "wsdd-http.xml". For my opensuse package I added a service file "wsdd.xml" which is equivalent to the latter and includes "ws-discovery-udp.xml". For older distributions I added a copy of firewallds "ws-discovery-udp.xml", too.

@christgau
Copy link
Owner

firewalld already has a service file "ws-discovery-udp.xml" that is identical to your "wsdd.xml", except for the include of "wsdd-http.xml".

By firewalld, you mean upstream here?

For my opensuse package I added a service file "wsdd.xml" which is equivalent to the latter and includes "ws-discovery-udp.xml".

Sounds reasonable.

For older distributions I added a copy of firewallds "ws-discovery-udp.xml", too.

So you proposal would be to rely on the existence of ws-discovery-udp.xml and rewrite the service file for wsdd accordingly?!
@ondrejholy would that also work from your perspective?

@ondrejholy
Copy link
Contributor Author

I haven't known that it is part of firewalld. It will surely work if firewalld already contains some of those. Hmm, shouldn't the missing part be proposed to firewalld upstream after all?

@hgraeber
Copy link

firewalld already has a service file "ws-discovery-udp.xml" that is identical to your "wsdd.xml", except for the include of "wsdd-http.xml".

By firewalld, you mean upstream here?

Yes, the "ws-discovery-udp.xml" service file is part of firewalld since version 1.1.0.

For my opensuse package I added a service file "wsdd.xml" which is equivalent to the latter and includes "ws-discovery-udp.xml".

Sounds reasonable.

For older distributions I added a copy of firewallds "ws-discovery-udp.xml", too.

So you proposal would be to rely on the existence of ws-discovery-udp.xml and rewrite the service file for wsdd accordingly?! @ondrejholy would that also work from your perspective?

I think the firewalld service file(s) for wsdd shall use the files from firewalld so far as already available there. For the remaining ports one can profiled them as part of wsdd and in parallel shall give them upstream.

Here is the "wsdd.xml" form may opensuse package:

<?xml version="1.0" encoding="utf-8"?>
<service>
  <short>wsdd</short>
  <description>Web Service Discovery</description>
  <port protocol="tcp" port="5357"/>
  <include service="ws-discovery-udp"/>
</service>

Maybe for upstream one shall not use "wsdd" as part of the files to be friendly to other wsd implementations.

christgau added a commit to christgau/firewalld that referenced this pull request Dec 28, 2023
@christgau @hgraeber
feat(service): add WS-Discovery host
b6ee7ea 
Add a generic service definition for a WS-Discovery host implementation,
which, e.g., allows to discover Samba file sharing services via Windows.
As per https://learn.microsoft.com/en-us/windows/win32/wsdapi/additional-ws-discovery-functionality,
TCP port 5357 is used by the service and respective implementations (see
below).

The file re-uses the already existing UDP multicast service definition.

The service file is taken from openSUSE (see
https://build.opensuse.org/package/view_file/openSUSE:Leap:15.5/wsdd/wsdd.xml),
which did not validate against the firewalld serivce schema. Thus, the
include statement was moved. The original author of the openSUSE
package, suggested to upstream a generic service definition that is
agnostic to a specific implementation, like wsdd
(https://github.com/christgau/wsdd) or wsdd2
(https://github.com/Netgear/wsdd2).

See also: christgau/wsdd#186

Co-authored-by: Herbert Graeber <[email protected]>
@christgau christgau mentioned this pull request Dec 28, 2023
Merged
erig0 pushed a commit to christgau/firewalld that referenced this pull request Mar 7, 2024
@christgau @hgraeber @erig0
feat(service): add WS-Discovery host
0e7b9f6 
Add a generic service definition for a WS-Discovery host implementation,
which, e.g., allows to discover Samba file sharing services via Windows.
As per https://learn.microsoft.com/en-us/windows/win32/wsdapi/additional-ws-discovery-functionality,
TCP port 5357 is used by the service and respective implementations (see
below).

The file re-uses the already existing UDP multicast service definition.

The service file is taken from openSUSE (see
https://build.opensuse.org/package/view_file/openSUSE:Leap:15.5/wsdd/wsdd.xml),
which did not validate against the firewalld serivce schema. Thus, the
include statement was moved. The original author of the openSUSE
package, suggested to upstream a generic service definition that is
agnostic to a specific implementation, like wsdd
(https://github.com/christgau/wsdd) or wsdd2
(https://github.com/Netgear/wsdd2).

See also: christgau/wsdd#186

Co-authored-by: Herbert Graeber <[email protected]>
erig0 pushed a commit to firewalld/firewalld that referenced this pull request Mar 7, 2024
@christgau @hgraeber @erig0
feat(service): add WS-Discovery host
984c321 
Add a generic service definition for a WS-Discovery host implementation,
which, e.g., allows to discover Samba file sharing services via Windows.
As per https://learn.microsoft.com/en-us/windows/win32/wsdapi/additional-ws-discovery-functionality,
TCP port 5357 is used by the service and respective implementations (see
below).

The file re-uses the already existing UDP multicast service definition.

The service file is taken from openSUSE (see
https://build.opensuse.org/package/view_file/openSUSE:Leap:15.5/wsdd/wsdd.xml),
which did not validate against the firewalld serivce schema. Thus, the
include statement was moved. The original author of the openSUSE
package, suggested to upstream a generic service definition that is
agnostic to a specific implementation, like wsdd
(https://github.com/christgau/wsdd) or wsdd2
(https://github.com/Netgear/wsdd2).

See also: christgau/wsdd#186

Co-authored-by: Herbert Graeber <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ondrejholy @christgau @hgraeber