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

Provide firewalld service definitions #13845

Merged
merged 1 commit into from May 17, 2018

Conversation

@mavit
Copy link
Contributor

commented May 2, 2018

Description

On some Linux distributions, including firewalld service definition files will cause the associated network ports to appear in the distribution’s firewall-config GUI for opening or closing.

Motivation and Context

I'm new to using Kodi, and realised that I would need to open firewall ports on my Fedora workstation in order to control it remotely. The HOW-TO:Install Kodi on Fedora 26 using RPMFusion packages wiki page suggests that I might like to disable the firewall entirely, which is probably good advice if you have a machine dedicated to running Kodi, but is less ideal if the computer also performs other tasks. Instead, we can provide hints to firewalld about the ports that Kodi uses, to make it easier for the user to open the necessary ports.

I'm sure other ports will be required to enable other Kodi features, but hopefully defining these few ports is a step in the right direction.

How Has This Been Tested?

I applied this change as a patch to RPMFusion's Kodi package (master, 7877ce2, packaging Kodi 18.0a1-Leia) and used rfpkg mockbuild to successfully build Fedora 29 packages that included the new files.

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the Code guidelines of this project
  • My change requires benefits from a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the CONTRIBUTING document
  • I have added tests to cover my change
  • All new and existing tests passed
install(FILES ${CMAKE_SOURCE_DIR}/tools/Linux/firewalld-services/kodi-jsonrpc.xml
DESTINATION ${prefix}/lib/firewalld/services
COMPONENT kodi)

This comment has been minimized.

Copy link
@hudokkow

hudokkow May 3, 2018

Member

Thank you for your first contribution to Kodi. Very much appreciated.

You can make it shorter:

# Install firewalld service definitions
install(FILES ${CMAKE_SOURCE_DIR}/tools/Linux/firewalld-services/kodi-eventserver.xml
              ${CMAKE_SOURCE_DIR}/tools/Linux/firewalld-services/kodi-http.xml
              ${CMAKE_SOURCE_DIR}/tools/Linux/firewalld-services/kodi-jsonrpc.xml
        DESTINATION ${prefix}/lib/firewalld/services
        COMPONENT kodi)

Apart from that, looks good to me. Build failure is unrelated to this change.

On some Linux distributions, including these files will cause the associated network ports to appear in the distribution’s `firewall-config` GUI for opening or closing.
@mavit mavit force-pushed the mavit:firewalld-services branch from 94f350d to 0142d77 May 3, 2018
<service>
<short>Kodi web interface</short>
<description>Kodi is a free cross-platform media-player jukebox and entertainment hub. Enable this option to remotely control Kodi via its web interface.</description>
<port protocol="tcp" port="8080"/>

This comment has been minimized.

Copy link
@Razzeee

Razzeee May 16, 2018

Member

Only problem I can think of is that you can edit this port inside of kodi and it won't match this anymore. Not sure if you can do for the others too, but this one is quiet prominent in the settings.

Frustratingly the only thing I can think of we can do, is point that out in the description.

This comment has been minimized.

Copy link
@mavit

mavit May 16, 2018

Author Contributor

I suppose the same thing is true of pretty much every service that firewalld knows about, since most services are configurable in this way, but I don't think it's a problem. If someone's changed the port, they should know what port they've set it to, and should be able to tell that to the firewall. Teaching firewalld about the default ports mainly helps users who don't already know what the default port numbers are.

@hudokkow

This comment has been minimized.

Copy link
Member

commented May 17, 2018

jenkins build this please

@hudokkow hudokkow merged commit 4ea3683 into xbmc:master May 17, 2018
1 check passed
1 check passed
default You're awesome. Have a cookie
Details
@hudokkow hudokkow added this to the Leia 18.0-alpha2 milestone May 17, 2018
@hudokkow hudokkow added the v18 Leia label May 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.