Bug 1204 - cups-browsed: unsupported BrowseAllow value lets cups-browsed accept from all hosts
Summary: cups-browsed: unsupported BrowseAllow value lets cups-browsed accept from all...
Status: RESOLVED FIXED
Alias: None
Product: OpenPrinting
Classification: Unclassified
Component: cups-filters (show other bugs)
Version: unspecified
Hardware: All All
: P2 major
Assignee: Till Kamppeter
URL:
Depends on:
Blocks:
 
Reported: 2014-04-24 12:31 UTC by Johannes Meixner
Modified: 2014-04-25 12:59 UTC (History)
2 users (show)

See Also:


Attachments
browseallow-fix.patch (1.25 KB, patch)
2014-04-24 21:08 UTC, Till Kamppeter
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Johannes Meixner 2014-04-24 12:31:42 UTC
The actually supported syntax for BrowseAllow values
in /etc/cups/cups-browsed.conf is rather limited.

I run cups-browsed with debugging via:
# /usr/sbin/cups-browsed -d &>/tmp/cups-browsed.log &

I tried
"BrowseAllow nelson" (nelson is a host name)
and got in /tmp/cups-browsed.log
----------------------------------------------------------------------
cups-browsed: BrowseAllow value "nelson" not understood
----------------------------------------------------------------------

Same issue for an IPv6 address via
"BrowseAllow 2620:113:80c0:8080:10:120:0:103"
or
"BrowseAllow [2620:113:80c0:8080:10:120:0:103]"
because - as far as I see - in utils/cups-browsed.c
there is no IPv6 support in the read_browseallow_value function
regardless that the allowed function seems to have IPv6 support.

When there is only one BrowseAllow line but its value
is not understood by cups-browsed, then it is effectively the same
as no BrowseAllow line and that means to accept browse packets
from all hosts.

This is a security issue.

The fallback should not be to accept browse packets from all hosts.
The fallback should be secure.

There is a difference between the documented default behaviour
(i.e. accept from all hosts when there is no BrowseAllow line)
according to "man cups-browsed.conf" and the fallback behaviour
in case of unsupported BrowseAllow values.

I think it might become complicated to implement a reasonable
working but still secure fallback because the default behaviour
cannot be used here as fallback behaviour.

I think it would be too harsh if cups-browsed would fall back
to not accept any browse packets from any hosts if only one
of several BrowseAllow values is unsupported.

I think cups-browsed should still accept browse packets from hosts
specified by the supported BrowseAllow values even if there
are also unsupported BrowseAllow values.

Therefore I like to suggest at least as a band-aid fix for now
that each not understood BrowseAllow value could be replaced
by an IP address which does not exist in a real network
as a (hopefully) secure fallback, for example 192.0.2.1
(a host address from 192.0.2.0/24 "TEST-NET-1", see RFC5737).

This way something like
-----------------------------------------------------------------------
BrowseAllow supported_value_1
BrowseAllow unsupported_value_2
BrowseAllow supported_value_3
BrowseAllow unsupported_value_4
-----------------------------------------------------------------------
would become
-----------------------------------------------------------------------
BrowseAllow supported_value_1
BrowseAllow 192.0.2.1
BrowseAllow supported_value_3
BrowseAllow 192.0.2.1
-----------------------------------------------------------------------
which results that cups-browsed still accepts browse packets
from hosts specified by the supported BrowseAllow values
but not more than those.
Comment 1 Till Kamppeter 2014-04-24 21:08:04 UTC
Created attachment 449 [details]
browseallow-fix.patch

As a proposed fix I have now pushed BZR rev. 7195, which is the attached patch. Please test this whether it fixes the problem and tell me if it does so, or if not, I am grateful if you could propose a working fix.

This does not fix the IPv6 problem, Tim, I would be grateful for a fix from you.

What it does:

An invalid argument for BrowseAllow (including no argument at all) is not treated any more as the BrowseAllow line not being present, but instead, as a BrowseAllow line which no host can fulfill.

This means, that if it is the only BrowseAllow line, no host is allowed to access. If there are other, valid BrowseAllow lines, these lines determine who can access, and if there are no BrowseAllow lines at all, all hosts can access.

Please test. If you report that it works as it should, I will release 1.0.53.
Comment 2 Johannes Meixner 2014-04-25 10:27:18 UTC
Hello Till,

many thanks for the very fast fix!

cups-filters-1.0-20140425 works technically well for me
but only when I use IPv4 addresses (but no hostnames).

Therefore there is a documentation issue left
that only IPv4 addresses but no hostnames are supported.

Here my results:

I have in /etc/cups/cups-browsed.conf
----------------------------------------------------------------------------
# Only browse remote printers from selected servers
# BrowseAllow cups.example.com
# BrowseAllow 192.168.1.12
# BrowseAllow 192.168.1.0/24
# BrowseAllow 192.168.1.0/255.255.255.0
BrowseAllow foo
BrowseAllow
BrowseAllow
BrowseAllow 10.120.4.251
BrowseAllow nelson.suse.de
----------------------------------------------------------------------------
The first empty BrowseAllow line is "BrowseAllow\n",
the second empty BrowseAllow line is "BrowseAllow         \n".

I start cups-browsed:
# /usr/sbin/cups-browsed -d &>/tmp/cups-browsed.log &

I get in /tmp/cups-browsed.log
----------------------------------------------------------------------------
cups-browsed: Reading config: BrowseAllow foo
cups-browsed: BrowseAllow value "foo" not understood
cups-browsed: Reading config: BrowseAllow (null)
cups-browsed: BrowseAllow value "(null)" not understood
cups-browsed: Reading config: BrowseAllow (null)
cups-browsed: BrowseAllow value "(null)" not understood
cups-browsed: Reading config: BrowseAllow 10.120.4.251
cups-browsed: Reading config: BrowseAllow nelson.suse.de
cups-browsed: BrowseAllow value "nelson.suse.de" not understood
----------------------------------------------------------------------------

With this settings the cups-browsed only creates local queues
for the remote queues from 10.120.4.251
----------------------------------------------------------------------------
# host 10.120.4.251
251.4.120.10.in-addr.arpa domain name pointer nelson.suse.de.

# lpstat -v
device for lj1220ljet4: ipp://nelson.suse.de:631/printers/lj1220ljet4
device for lj1220ps: ipp://nelson.suse.de:631/printers/lj1220ps
----------------------------------------------------------------------------

I also tested "BrowseAllow 10.120.4.0/24" to exclude some other
remote CUPS servers in our 10.120.0.0/16 network (10.120.4.251
is the only CUPS server in the 10.120.4.0/24 sub-network
and that also works well for me (I still get only the queues
from 10.120.4.251 but none from any other remote CUPS server).

The documentation issue is that the comment in /etc/cups/cups-browsed.conf
reads that "BrowseAllow nelson.suse.de" should be supported but
actually it is not.
Comment 3 Till Kamppeter 2014-04-25 11:37:59 UTC
Thanks for testing, Johannes, so this bug is fixed.

Can you report a bug that BrowseAllow does not support IPv6 and addresses like nelson.suse.de? I will assign this to Tim Waugh then. Mention in the bug report that there is IPv6 support in allowed() but not in read_browseallow_value().

So I will release 1.0.53 today.
Comment 4 Johannes Meixner 2014-04-25 12:36:55 UTC
I submitted Bug 1205
"cups-browsed: BrowseAllow values should support hostnames and IPv6 addresses"
and assigned it to Tim Waugh.
Comment 5 Till Kamppeter 2014-04-25 12:59:05 UTC
Johannes, thank you very much.