Suggestion: Bind NQPTP_CONTROL_PORT to localhost instead of NULL

[th0u]

Hello,
from what I understand from the code of nqptp and shairport-sync, nqptp listens as Server to Port 9000 and shairport-sync talks to port 9000.

Lets assume that nqptp and shaiport-sync run always on the same host/system/container - but I am not sure if this assumption is always valid - then it would be ideal, if nqptp would listen only on localhost instead on any interface.

I tested this successfully with very few quick hack changes to nqptp.c and nqptp-utilities(.c | .h).

[mikebrady]

Thanks for the suggestion -- it's a good idea alright!

[mikebrady]

Actually, if you had some code suggestions, that would be great 🙂.

[th0u]

Here is the Code I use ...
in nqptp-utilities.c

• add parameter "node" to function
• use parameter "node" in getaddrinfo(..)

 void open_sockets_at_port( const char *node, uint16_t port, sockets_open_bundle *sockets_open_stuff) {
  // open up sockets for UDP ports 319 and 320

  struct addrinfo hints, *info, *p;
  int ret;

  memset(&hints, 0, sizeof(hints));
  hints.ai_family = AF_UNSPEC;
  hints.ai_socktype = SOCK_DGRAM;
  hints.ai_flags = AI_PASSIVE;

  char portstr[20];
  snprintf(portstr, 20, "%d", port);

  ret = getaddrinfo(node, portstr, &hints, &info);
  if (ret) {
    die("getifaddrs: %s", gai_strerror(ret));
  }

  for (p = info; p; p = p->ai_next) {

... in nqptp.c

• add parameter to function call

 // open sockets 319 and 320

  open_sockets_at_port(NULL, 319, &sockets_open_stuff);
  open_sockets_at_port(NULL, 320, &sockets_open_stuff);
  open_sockets_at_port("localhost", NQPTP_CONTROL_PORT,
                       &sockets_open_stuff); // this for messages from the client

  // start the timed tasks

[mikebrady]

Great, thanks!

[herrernst]

Any chance of getting this merged?

[mikebrady]

Thanks for the post. Yeah, I can set about it.

[mikebrady]

Just pushed that update into the development branch of NQPTP. You'll need to use the latest development branch for Shairport Sync too. If you got a chance to check it and report back, that would be appreciated.

As a matter of interest, is this change important to your setup?

[herrernst]

I just don't wanna run a service as root with open ports to everyone inside the network (but on Linux I am now runnning it as a user via setcap as described in #12). Change seems to work for me, thank you!

[modafroman]

Just pushed that update into the development branch of NQPTP. You'll need to use the latest development branch for Shairport Sync too. If you got a chance to check it and report back, that would be appreciated.

As a matter of interest, is this change important to your setup?

Hi @mikebrady, just doing a bit of digging in trying to solve an issue that I've come up with trying to get a multi shairport-sync instance setup working in docker using @noelhibbard's scripts:

noelhibbard/shairport-sync-docker#2

I have a feeling that this change might be some how related to the issue i'm trying to fix, any thoughts? Thanks :)

[mikebrady]

Hi @modafroman. Thanks for the note. The effect of this change to NQPTP was to require UDP messages received by NQPTP to come from Shairport Sync on the same system rather than from any computer on the network. So, on the face of it, it doesn't seem likely to be a cause of problems. Of course I could be wrong -- would there be any more evidence that might suggest a cause?

[modafroman]

Hi @modafroman. Thanks for the note. The effect of this change to NQPTP was to require UDP messages received by NQPTP to come from Shairport Sync on the same system rather than from any computer on the network. So, on the face of it, it doesn't seem likely to be a cause of problems. Of course I could be wrong -- would there be any more evidence that might suggest a cause?

Thanks Mike, no worries I'll keep doing some digging. It might just be something as simple as the docker network config being wrong or just my weird edge case install.

[mikebrady]

Hi @modafroman. There was an issue discovered, that in some situations the ports had to be open both in IPv4 and IPv6. The fix is in the development branch BUT it has a different Shared Memory Interface version, and at the moment would require you to be using the development branch of Shairport Sync too. This should change in the next short while -- I'm hoping to get a new matching release of both NQPTP and Shairport Sync out in the next few days.