cups debugging, continued

cups debugging, continued

[Isaac Dunham]

On Sun, Jun 29, 2014 at 05:12:01PM -0700, Isaac Dunham wrote:
> Now I hit the next issue: the 'lpd' backend reports 
> 'Unable to reserve port: Invalid argument'
> And I need to figure out what's causing this--or rather, what the solution is.
> Right now, I'm stuck (lpd is the main way to access it, and it isn't working).
OK, now we're in cups-1.7.3/backend/lpd.c:

...the problem is a local implementation of rresvport_af(), trying to reserve
a port but failing.

And strace -p `pidof lpd` says this:
Process 16113 attached
restart_syscall(<... resuming interrupted call ...>) = 0
geteuid32()                             = 0
socket(PF_INET, SOCK_STREAM, IPPROTO_IP) = 7
bind(7, {sa_family=AF_INET, sin_port=htons(905), sin_addr=inet_addr("0.0.0.0")}, 256) = -1 EINVAL (Invalid argument)
close(7)                                = 0
writev(2, [{"", 0}, {"DEBUG: Unable to reserve port", 29}], 2) = 29
writev(2, [{"", 0}, {":", 1}], 2)       = 1
writev(2, [{"", 0}, {" ", 1}], 2)       = 1
writev(2, [{"", 0}, {"Invalid argument", 16}], 2) = 16
writev(2, [{"", 0}, {"\n", 1}], 2)      = 1
nanosleep({1, 0}, 0x5d2bcae8)           = 0
geteuid32()                             = 0
socket(PF_INET, SOCK_STREAM, IPPROTO_IP) = 7
bind(7, {sa_family=AF_INET, sin_port=htons(904), sin_addr=inet_addr("0.0.0.0")}, 256) = -1 EINVAL (Invalid argument)
close(7)                                = 0
writev(2, [{"", 0}, {"DEBUG: Unable to reserve port", 29}], 2) = 29
writev(2, [{"", 0}, {":", 1}], 2)       = 1
writev(2, [{"", 0}, {" ", 1}], 2)       = 1
writev(2, [{"", 0}, {"Invalid argument", 16}], 2) = 16
writev(2, [{"", 0}, {"\n", 1}], 2)      = 1
nanosleep({1, 0}, 0x5d2bcae8)           = 0
geteuid32()                             = 0
socket(PF_INET, SOCK_STREAM, IPPROTO_IP) = 7
bind(7, {sa_family=AF_INET, sin_port=htons(903), sin_addr=inet_addr("0.0.0.0")}, 256) = -1 EINVAL (Invalid argument)
close(7)                                = 0
writev(2, [{"", 0}, {"DEBUG: Unable to reserve port", 29}], 2) = 29
writev(2, [{"", 0}, {":", 1}], 2)       = 1
writev(2, [{"", 0}, {" ", 1}], 2)       = 1
writev(2, [{"", 0}, {"Invalid argument", 16}], 2) = 16
writev(2, [{"", 0}, {"\n", 1}], 2)      = 1
nanosleep({1, 0}, Process 16113 detached
 <detached ...>

Clearly something needs to be adjusted for this to work with musl; I'm not sure what it is.

Thanks,
Isaac Dunham


---
Unsubscribe:  alpine-devel+unsubscribe@lists.alpinelinux.org
Help:         alpine-devel+help@lists.alpinelinux.org
---

Re: cups debugging, continued

[Rich Felker]

On Sun, Jun 29, 2014 at 06:28:31PM -0700, Isaac Dunham wrote:
> On Sun, Jun 29, 2014 at 05:12:01PM -0700, Isaac Dunham wrote:
> > Now I hit the next issue: the 'lpd' backend reports 
> > 'Unable to reserve port: Invalid argument'
> > And I need to figure out what's causing this--or rather, what the solution is.
> > Right now, I'm stuck (lpd is the main way to access it, and it isn't working).
> OK, now we're in cups-1.7.3/backend/lpd.c:
> 
> ....the problem is a local implementation of rresvport_af(), trying to reserve
> a port but failing.
> 
> And strace -p `pidof lpd` says this:
> Process 16113 attached
> restart_syscall(<... resuming interrupted call ...>) = 0
> geteuid32()                             = 0
> socket(PF_INET, SOCK_STREAM, IPPROTO_IP) = 7
> bind(7, {sa_family=AF_INET, sin_port=htons(905), sin_addr=inet_addr("0.0.0.0")}, 256) = -1 EINVAL (Invalid argument)

Where did the value of 256 for socklen_t come from? That's almost
surely the cause of the EINVAL. sockaddr_in should have a length of
16, not 256.

Rich

Re: cups debugging, continued

[Isaac Dunham]

On Sun, Jun 29, 2014 at 10:03:11PM -0400, Rich Felker wrote:
> On Sun, Jun 29, 2014 at 06:28:31PM -0700, Isaac Dunham wrote:
> > On Sun, Jun 29, 2014 at 05:12:01PM -0700, Isaac Dunham wrote:
> > > Now I hit the next issue: the 'lpd' backend reports 
> > > 'Unable to reserve port: Invalid argument'
> > > And I need to figure out what's causing this--or rather, what the solution is.
> > > Right now, I'm stuck (lpd is the main way to access it, and it isn't working).
> > OK, now we're in cups-1.7.3/backend/lpd.c:
> > 
> > ....the problem is a local implementation of rresvport_af(), trying to reserve
> > a port but failing.
> > 
> > And strace -p `pidof lpd` says this:
> > Process 16113 attached
> > restart_syscall(<... resuming interrupted call ...>) = 0
> > geteuid32()                             = 0
> > socket(PF_INET, SOCK_STREAM, IPPROTO_IP) = 7
> > bind(7, {sa_family=AF_INET, sin_port=htons(905), sin_addr=inet_addr("0.0.0.0")}, 256) = -1 EINVAL (Invalid argument)
> 
> Where did the value of 256 for socklen_t come from? That's almost
> surely the cause of the EINVAL. sockaddr_in should have a length of
> 16, not 256.
Apparently this:
    if (!bind(fd, (struct sockaddr *)&addr, sizeof(addr)))
          return (fd);

addr is of type http_addr_t, defined in cups/http.h thus:
typedef union _http_addr_u              /**** Socket address union, which
                                         **** makes using IPv6 and other
					 **** address types easier and
					 **** more portable. @since CUPS 1.2/OS X 10.5@                                  ****/
										{
  struct sockaddr       addr;           /* Base structure for family value */
  struct sockaddr_in    ipv4;           /* IPv4 address */
#ifdef AF_INET6
  struct sockaddr_in6   ipv6;           /* IPv6 address */
#endif /* AF_INET6 */
#ifdef AF_LOCAL
  struct sockaddr_un    un;             /* Domain socket file */
#endif /* AF_LOCAL */
  char                  pad[256];       /* Padding to ensure binary compatibility */
} http_addr_t;

I'm not sure how to handle this.

Thanks,
Isaac Dunham

Re: cups debugging, continued...ugly patch

[Isaac Dunham]

[-- Attachment #1: Type: text/plain, Size: 351 bytes --]

Thanks to Rich's comment, I've found a solution that works here.
The patch doesn't exactly look nice, though.

And yes, once I add this patch I can print using Peter De Wachter's brlaser
driver (github.com/pdewacht/brlaser).

I'll be sending a patch for main/cups to alpine-devel shortly,
and probably packaging brlaser for alpine.

HTH,
Isaac Dunham

[-- Attachment #2: socksz.diff --]
[-- Type: text/plain, Size: 1210 bytes --]

diff --git a/backend/lpd.c b/backend/lpd.c
index 6e4ab36..bbffb82 100644
--- a/backend/lpd.c
+++ b/backend/lpd.c
@@ -1244,6 +1244,7 @@ rresvport_af(int *port,			/* IO - Port number to bind to */
 {
   http_addr_t	addr;			/* Socket address */
   int		fd;			/* Socket file descriptor */
+  socklen_t	socksz=sizeof(struct sockaddr);	/* size of struct sockaddr* */
 
 
  /*
@@ -1260,6 +1261,23 @@ rresvport_af(int *port,			/* IO - Port number to bind to */
   memset(&addr, 0, sizeof(addr));
   addr.addr.sa_family = family;
 
+  switch(family) {
+    case AF_INET:
+      socksz = sizeof(struct sockaddr_in);
+      break;
+#ifdef AF_INET6
+    case AF_INET6:
+      socksz = sizeof(struct sockaddr_in6);
+      break;
+#endif
+#ifdef AF_LOCAL
+    case AF_LOCAL:
+      socksz = sizeof(struct sockaddr_un);
+#endif
+    default:
+      break;
+  }
+
  /*
   * Try to bind the socket to a reserved port...
   */
@@ -1276,7 +1294,7 @@ rresvport_af(int *port,			/* IO - Port number to bind to */
     * Try binding the port to the socket; return if all is OK...
     */
 
-    if (!bind(fd, (struct sockaddr *)&addr, sizeof(addr)))
+    if (!bind(fd, (struct sockaddr *)&addr, socksz))
       return (fd);
 
    /*

Re: Re: cups debugging, continued...ugly patch

[Luca Barbato]

On 30/06/14 07:34, Isaac Dunham wrote:
> Thanks to Rich's comment, I've found a solution that works here.
> The patch doesn't exactly look nice, though.

Shouldn't bind check that the size is at least the one needed and by
happy even if the size is larger than expected?

lu

Re: Re: cups debugging, continued...ugly patch

[Rich Felker]

On Mon, Jun 30, 2014 at 08:21:10AM +0200, Luca Barbato wrote:
> On 30/06/14 07:34, Isaac Dunham wrote:
> > Thanks to Rich's comment, I've found a solution that works here.
> > The patch doesn't exactly look nice, though.
> 
> Shouldn't bind check that the size is at least the one needed and by
> happy even if the size is larger than expected?

No, that's not how it works. POSIX permits but does not require the
error in this case:

    The bind() function may fail if:

    [EINVAL]

        The address_len argument is not a valid length for the address
        family.

I think the idea is that, if you pass the wrong length, there's
likely something seriously wrong going on. Which can easily happen
with all the bogus pseudo-polymorphism of struct sockaddr...

Rich

Re: Re: cups debugging, continued...ugly patch

[Rich Felker]

On Sun, Jun 29, 2014 at 10:34:27PM -0700, Isaac Dunham wrote:
> Thanks to Rich's comment, I've found a solution that works here.
> The patch doesn't exactly look nice, though.

I agree it's ugly. The clean way to do it would be to use getaddrinfo
to request an AI_PASSIVE address for the desired family. Then you get
the right sockaddr type and size for free, and you can subsequently
fill in the port to try. That wouldn't work for AF_LOCAL (at least not
on musl) but this whole function makes no sense for AF_LOCAL.

Of course binding a reserved port like this is a serious security
smell -- it sounds like they're trying to facilitate port-based
authentication, which is unsafe if used for anything except localhost.

I would see if it's possible to remove the binding completely and have
it work. Remote lpd's would not be able to trust the ldp that omits
the bind based on its port, but this kind of trust is fundamentally
wrong and insecure anyway. If such a trust relationship is needed, a
proper authentication channel must be used.

Rich

Re: Re: cups debugging, continued...ugly patch

[James Cloos]

>>>>> "RF" == Rich Felker <dalias@libc.org> writes:

RF> Of course binding a reserved port like this is a serious security
RF> smell -- it sounds like they're trying to facilitate port-based
RF> authentication, which is unsafe if used for anything except localhost.

If it is for the lpd protocol, there are (or at least have been) systems
and printers which only accepted print jobs via lpd if they originated
on the lpd port.  Or maybe it was if they originated on a <1024 port.

I have a vague recollection of bug reports for cups in the early days
about that.  

-JimC
-- 
James Cloos <cloos@jhcloos.com>         OpenPGP: 0x997A9F17ED7DAEA6

Re: Re: cups debugging, continued...ugly patch

[Rich Felker]

On Mon, Jun 30, 2014 at 03:43:25PM -0400, James Cloos wrote:
> >>>>> "RF" == Rich Felker <dalias@libc.org> writes:
> 
> RF> Of course binding a reserved port like this is a serious security
> RF> smell -- it sounds like they're trying to facilitate port-based
> RF> authentication, which is unsafe if used for anything except localhost.
> 
> If it is for the lpd protocol, there are (or at least have been) systems
> and printers which only accepted print jobs via lpd if they originated
> on the lpd port.  Or maybe it was if they originated on a <1024 port.
> 
> I have a vague recollection of bug reports for cups in the early days
> about that.  

Yes, this sounds plausible, but those print servers/printers are
highly insecure already and should really be fixed to use proper
authentication.

It's also something of an issue that cups lpd even has permissions to
bind reserved ports for outgoing connections, since this means it is
retaining root permissions past startup...

Rich

Re: [alpine-devel] Re: cups debugging, continued...ugly patch

[Natanael Copa]

On Sun, 29 Jun 2014 22:34:27 -0700
Isaac Dunham <ibid.ag@gmail.com> wrote:

> Thanks to Rich's comment, I've found a solution that works here.
> The patch doesn't exactly look nice, though.
> 
> And yes, once I add this patch I can print using Peter De Wachter's brlaser
> driver (github.com/pdewacht/brlaser).
> 
> I'll be sending a patch for main/cups to alpine-devel shortly,
> and probably packaging brlaser for alpine.
> 
> HTH,
> Isaac Dunham

good catch!

would be nice to know what upstream says about it.



-nc