2017-02-09 16:25:15

by guido

[permalink] [raw]
Subject: [refpolicy] [PATCH 1/2] cups: read permission for cupsd_var_run_t socket files in cups_stream_connect()

Modify the cups_stream_connect() interface so that it can also
read cupsd_var_run_t socket files in addition to writing them.

Signed-off-by: Guido Trentalancia <[email protected]>
---
policy/modules/contrib/cups.if | 1 +
1 file changed, 1 insertion(+)

diff -pru a/policy/modules/contrib/cups.if b/policy/modules/contrib/cups.if
--- a/policy/modules/contrib/cups.if 2017-01-24 18:56:19.569106107 +0100
+++ b/policy/modules/contrib/cups.if 2017-02-09 16:57:59.936511815 +0100
@@ -69,6 +69,7 @@ interface(`cups_stream_connect',`
')

files_search_pids($1)
+ allow $1 cupsd_var_run_t:sock_file read_sock_file_perms;
stream_connect_pattern($1, cupsd_var_run_t, cupsd_var_run_t, cupsd_t)
')



2017-02-09 16:26:23

by guido

[permalink] [raw]
Subject: [refpolicy] [PATCH 2/2] cups: read permission for cupsd_var_run_t socket files in cups_read_pid_files()

Modify the cups_read_pid_files() interface so that it allows
to read socket files of the cupsd_var_run_t type and not only
standard files.

Signed-off-by: Guido Trentalancia <[email protected]>
---
policy/modules/contrib/cups.if | 1 +
1 file changed, 1 insertion(+)

diff -pru a/policy/modules/contrib/cups.if b/policy/modules/contrib/cups.if
--- a/policy/modules/contrib/cups.if 2017-01-24 18:56:19.569106107 +0100
+++ b/policy/modules/contrib/cups.if 2017-02-09 16:46:23.649827258 +0100
@@ -124,6 +124,7 @@ interface(`cups_read_pid_files',`

files_search_pids($1)
allow $1 cupsd_var_run_t:file read_file_perms;
+ allow $1 cupsd_var_run_t:sock_file read_sock_file_perms;
')

########################################

2017-02-11 19:54:00

by Chris PeBenito

[permalink] [raw]
Subject: [refpolicy] [PATCH 2/2] cups: read permission for cupsd_var_run_t socket files in cups_read_pid_files()

On 02/09/17 11:26, Guido Trentalancia via refpolicy wrote:
> Modify the cups_read_pid_files() interface so that it allows
> to read socket files of the cupsd_var_run_t type and not only
> standard files.
>
> Signed-off-by: Guido Trentalancia <[email protected]>
> ---
> policy/modules/contrib/cups.if | 1 +
> 1 file changed, 1 insertion(+)
>
> diff -pru a/policy/modules/contrib/cups.if b/policy/modules/contrib/cups.if
> --- a/policy/modules/contrib/cups.if 2017-01-24 18:56:19.569106107 +0100
> +++ b/policy/modules/contrib/cups.if 2017-02-09 16:46:23.649827258 +0100
> @@ -124,6 +124,7 @@ interface(`cups_read_pid_files',`
>
> files_search_pids($1)
> allow $1 cupsd_var_run_t:file read_file_perms;
> + allow $1 cupsd_var_run_t:sock_file read_sock_file_perms;
> ')
>
> ########################################

You really saw sock_file read? I don't think I've ever seen that.
Regardless, this should be a separate interface.

--
Chris PeBenito

2017-02-11 20:00:44

by guido

[permalink] [raw]
Subject: [refpolicy] [PATCH 2/2] cups: read permission for cupsd_var_run_t socket files in cups_read_pid_files()

Yes, I confirm, sock_file read permissions are needed to print.

On the 11th of February 2017 20:54:00 CET, Chris PeBenito <[email protected]> wrote:
>On 02/09/17 11:26, Guido Trentalancia via refpolicy wrote:
>> Modify the cups_read_pid_files() interface so that it allows
>> to read socket files of the cupsd_var_run_t type and not only
>> standard files.
>>
>> Signed-off-by: Guido Trentalancia <[email protected]>
>> ---
>> policy/modules/contrib/cups.if | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff -pru a/policy/modules/contrib/cups.if
>b/policy/modules/contrib/cups.if
>> --- a/policy/modules/contrib/cups.if 2017-01-24 18:56:19.569106107
>+0100
>> +++ b/policy/modules/contrib/cups.if 2017-02-09 16:46:23.649827258
>+0100
>> @@ -124,6 +124,7 @@ interface(`cups_read_pid_files',`
>>
>> files_search_pids($1)
>> allow $1 cupsd_var_run_t:file read_file_perms;
>> + allow $1 cupsd_var_run_t:sock_file read_sock_file_perms;
>> ')
>>
>> ########################################
>
>You really saw sock_file read? I don't think I've ever seen that.
>Regardless, this should be a separate interface.

2017-02-11 20:13:40

by guido

[permalink] [raw]
Subject: [refpolicy] [PATCH 2/2 v2] cups/lpd: read permission for cupsd_var_run_t socket files

Introduce a new interface in the cups module to read cups socket
files and call such interface from the lpd module.

Thanks to Christpher PeBenito for revising this patch.

Signed-off-by: Guido Trentalancia <[email protected]>
---
policy/modules/contrib/cups.if | 19 +++++++++++++++++++
policy/modules/contrib/lpd.te | 1 +
2 files changed, 20 insertions(+)

diff -pru a/policy/modules/contrib/cups.if b/policy/modules/contrib/cups.if
--- a/policy/modules/contrib/cups.if 2017-01-24 18:56:19.569106107 +0100
+++ b/policy/modules/contrib/cups.if 2017-02-11 21:04:00.346144792 +0100
@@ -128,6 +128,25 @@ interface(`cups_read_pid_files',`

########################################
## <summary>
+## Read cups socket files.
+## </summary>
+## <param name="domain">
+## <summary>
+## Domain allowed access.
+## </summary>
+## </param>
+#
+interface(`cups_read_sock_files',`
+ gen_require(`
+ type cupsd_var_run_t;
+ ')
+
+ files_search_pids($1)
+ allow $1 cupsd_var_run_t:sock_file read_sock_file_perms;
+')
+
+########################################
+## <summary>
## Execute cups_config in the
## cups config domain.
## </summary>
diff -pru a/policy/modules/contrib/lpd.te b/policy/modules/contrib/lpd.te
--- a/policy/modules/contrib/lpd.te 2016-12-22 23:12:59.385081782 +0100
+++ b/policy/modules/contrib/lpd.te 2017-02-11 21:04:28.457255575 +0100
@@ -295,6 +295,7 @@ optional_policy(`
cups_read_config(lpr_t)
cups_stream_connect(lpr_t)
cups_read_pid_files(lpr_t)
+ cups_read_sock_files(lpr_t)
')

optional_policy(`

2017-02-11 20:22:35

by Dac Override

[permalink] [raw]
Subject: [refpolicy] [PATCH 2/2] cups: read permission for cupsd_var_run_t socket files in cups_read_pid_files()

On 02/11/2017 09:00 PM, Guido Trentalancia via refpolicy wrote:
> Yes, I confirm, sock_file read permissions are needed to print.

Just to be clear: So it does not work if you do not allow the read?
Sounds to me like this might be a leaked file descriptor issue instead

>
> On the 11th of February 2017 20:54:00 CET, Chris PeBenito <[email protected]> wrote:
>> On 02/09/17 11:26, Guido Trentalancia via refpolicy wrote:
>>> Modify the cups_read_pid_files() interface so that it allows
>>> to read socket files of the cupsd_var_run_t type and not only
>>> standard files.
>>>
>>> Signed-off-by: Guido Trentalancia <[email protected]>
>>> ---
>>> policy/modules/contrib/cups.if | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff -pru a/policy/modules/contrib/cups.if
>> b/policy/modules/contrib/cups.if
>>> --- a/policy/modules/contrib/cups.if 2017-01-24 18:56:19.569106107
>> +0100
>>> +++ b/policy/modules/contrib/cups.if 2017-02-09 16:46:23.649827258
>> +0100
>>> @@ -124,6 +124,7 @@ interface(`cups_read_pid_files',`
>>>
>>> files_search_pids($1)
>>> allow $1 cupsd_var_run_t:file read_file_perms;
>>> + allow $1 cupsd_var_run_t:sock_file read_sock_file_perms;
>>> ')
>>>
>>> ########################################
>>
>> You really saw sock_file read? I don't think I've ever seen that.
>> Regardless, this should be a separate interface.
>
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy
>


--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 648 bytes
Desc: OpenPGP digital signature
Url : http://oss.tresys.com/pipermail/refpolicy/attachments/20170211/90a4cd9d/attachment.bin

2017-02-11 21:03:53

by guido

[permalink] [raw]
Subject: [refpolicy] [PATCH 2/2] cups: read permission for cupsd_var_run_t socket files in cups_read_pid_files()

No, it is not able to detect printers without the permission to read cupsd_var_run_t socket files...

On the 11th of February 2017 21:22:35 CET, Dominick Grift via refpolicy <[email protected]> wrote:
>On 02/11/2017 09:00 PM, Guido Trentalancia via refpolicy wrote:
>> Yes, I confirm, sock_file read permissions are needed to print.
>
>Just to be clear: So it does not work if you do not allow the read?
>Sounds to me like this might be a leaked file descriptor issue instead
>
>>
>> On the 11th of February 2017 20:54:00 CET, Chris PeBenito
><[email protected]> wrote:
>>> On 02/09/17 11:26, Guido Trentalancia via refpolicy wrote:
>>>> Modify the cups_read_pid_files() interface so that it allows
>>>> to read socket files of the cupsd_var_run_t type and not only
>>>> standard files.
>>>>
>>>> Signed-off-by: Guido Trentalancia <[email protected]>
>>>> ---
>>>> policy/modules/contrib/cups.if | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff -pru a/policy/modules/contrib/cups.if
>>> b/policy/modules/contrib/cups.if
>>>> --- a/policy/modules/contrib/cups.if 2017-01-24 18:56:19.569106107
>>> +0100
>>>> +++ b/policy/modules/contrib/cups.if 2017-02-09 16:46:23.649827258
>>> +0100
>>>> @@ -124,6 +124,7 @@ interface(`cups_read_pid_files',`
>>>>
>>>> files_search_pids($1)
>>>> allow $1 cupsd_var_run_t:file read_file_perms;
>>>> + allow $1 cupsd_var_run_t:sock_file read_sock_file_perms;
>>>> ')
>>>>
>>>> ########################################
>>>
>>> You really saw sock_file read? I don't think I've ever seen that.
>>> Regardless, this should be a separate interface.
>>
>> _______________________________________________
>> refpolicy mailing list
>> refpolicy at oss.tresys.com
>> http://oss.tresys.com/mailman/listinfo/refpolicy
>>

2017-02-12 06:59:21

by Russell Coker

[permalink] [raw]
Subject: [refpolicy] [PATCH 2/2] cups: read permission for cupsd_var_run_t socket files in cups_read_pid_files()

On Saturday, 11 February 2017 9:00:44 PM AEDT Guido Trentalancia via refpolicy
wrote:
> Yes, I confirm, sock_file read permissions are needed to print.

I've seen that too. I have something similar in the Debian policy.

It's not needed to print, it's needed in some configurations which are the
default for some situations. It should be possible to configure cups to not
need that if you don't need lpr/lpq type functionality - but that may not be
possible for all clients.

--
My Main Blog http://etbe.coker.com.au/
My Documents Blog http://doc.coker.com.au/

2017-02-12 18:35:02

by Chris PeBenito

[permalink] [raw]
Subject: [refpolicy] [PATCH 1/2] cups: read permission for cupsd_var_run_t socket files in cups_stream_connect()

On 02/09/17 11:25, Guido Trentalancia via refpolicy wrote:
> Modify the cups_stream_connect() interface so that it can also
> read cupsd_var_run_t socket files in addition to writing them.
>
> Signed-off-by: Guido Trentalancia <[email protected]>
> ---
> policy/modules/contrib/cups.if | 1 +
> 1 file changed, 1 insertion(+)
>
> diff -pru a/policy/modules/contrib/cups.if b/policy/modules/contrib/cups.if
> --- a/policy/modules/contrib/cups.if 2017-01-24 18:56:19.569106107 +0100
> +++ b/policy/modules/contrib/cups.if 2017-02-09 16:57:59.936511815 +0100
> @@ -69,6 +69,7 @@ interface(`cups_stream_connect',`
> ')
>
> files_search_pids($1)
> + allow $1 cupsd_var_run_t:sock_file read_sock_file_perms;
> stream_connect_pattern($1, cupsd_var_run_t, cupsd_var_run_t, cupsd_t)
> ')

Merged.

--
Chris PeBenito

2017-02-12 18:35:12

by Chris PeBenito

[permalink] [raw]
Subject: [refpolicy] [PATCH 2/2 v2] cups/lpd: read permission for cupsd_var_run_t socket files

On 02/11/17 15:13, Guido Trentalancia via refpolicy wrote:
> Introduce a new interface in the cups module to read cups socket
> files and call such interface from the lpd module.
>
> Thanks to Christpher PeBenito for revising this patch.
>
> Signed-off-by: Guido Trentalancia <[email protected]>
> ---
> policy/modules/contrib/cups.if | 19 +++++++++++++++++++
> policy/modules/contrib/lpd.te | 1 +
> 2 files changed, 20 insertions(+)
>
> diff -pru a/policy/modules/contrib/cups.if b/policy/modules/contrib/cups.if
> --- a/policy/modules/contrib/cups.if 2017-01-24 18:56:19.569106107 +0100
> +++ b/policy/modules/contrib/cups.if 2017-02-11 21:04:00.346144792 +0100
> @@ -128,6 +128,25 @@ interface(`cups_read_pid_files',`
>
> ########################################
> ## <summary>
> +## Read cups socket files.
> +## </summary>
> +## <param name="domain">
> +## <summary>
> +## Domain allowed access.
> +## </summary>
> +## </param>
> +#
> +interface(`cups_read_sock_files',`
> + gen_require(`
> + type cupsd_var_run_t;
> + ')
> +
> + files_search_pids($1)
> + allow $1 cupsd_var_run_t:sock_file read_sock_file_perms;
> +')
> +
> +########################################
> +## <summary>
> ## Execute cups_config in the
> ## cups config domain.
> ## </summary>
> diff -pru a/policy/modules/contrib/lpd.te b/policy/modules/contrib/lpd.te
> --- a/policy/modules/contrib/lpd.te 2016-12-22 23:12:59.385081782 +0100
> +++ b/policy/modules/contrib/lpd.te 2017-02-11 21:04:28.457255575 +0100
> @@ -295,6 +295,7 @@ optional_policy(`
> cups_read_config(lpr_t)
> cups_stream_connect(lpr_t)
> cups_read_pid_files(lpr_t)
> + cups_read_sock_files(lpr_t)
> ')
>
> optional_policy(`

Merged.

--
Chris PeBenito

2017-02-12 18:35:45

by Chris PeBenito

[permalink] [raw]
Subject: [refpolicy] [PATCH 2/2] cups: read permission for cupsd_var_run_t socket files in cups_read_pid_files()

On 02/12/17 01:59, Russell Coker via refpolicy wrote:
> On Saturday, 11 February 2017 9:00:44 PM AEDT Guido Trentalancia via refpolicy
> wrote:
>> Yes, I confirm, sock_file read permissions are needed to print.
>
> I've seen that too. I have something similar in the Debian policy.
>
> It's not needed to print, it's needed in some configurations which are the
> default for some situations. It should be possible to configure cups to not
> need that if you don't need lpr/lpq type functionality - but that may not be
> possible for all clients.

It's interesting. Years ago when I put together the socket pattern
macros I couldn't trigger a sock_file:read, though I didn't try every
single UNIX stream socket function. I'll have to look at the kernel
code again in case it warrants an update to one of the patterns.

--
Chris PeBenito

2017-02-14 04:08:28

by Russell Coker

[permalink] [raw]
Subject: [refpolicy] [PATCH 2/2] cups: read permission for cupsd_var_run_t socket files in cups_read_pid_files()

On Sunday, 12 February 2017 5:59:21 PM AEDT Russell Coker via refpolicy wrote:
> On Saturday, 11 February 2017 9:00:44 PM AEDT Guido Trentalancia via
> refpolicy
> wrote:
> > Yes, I confirm, sock_file read permissions are needed to print.
>
> I've seen that too. I have something similar in the Debian policy.
>
> It's not needed to print, it's needed in some configurations which are the
> default for some situations. It should be possible to configure cups to not
> need that if you don't need lpr/lpq type functionality - but that may not
> be possible for all clients.

Does it make sense to have a cups_read_sock_files when the cups clients need
read/write access? Why not just have a single interface granting read-write?

In Debian I used the below patch to make cups_stream_connect do what is
necessary. Otherwise you will just have to add cups_read_sock_files after
every call to cups_stream_connect.

Index: refpolicy-2.20170212/policy/modules/contrib/cups.if
===================================================================
--- refpolicy-2.20170212.orig/policy/modules/contrib/cups.if
+++ refpolicy-2.20170212/policy/modules/contrib/cups.if
@@ -69,7 +69,9 @@ interface(`cups_stream_connect',`
')

files_search_pids($1)
- stream_connect_pattern($1, cupsd_var_run_t, cupsd_var_run_t, cupsd_t)
+ allow $1 cupsd_var_run_t:dir search_dir_perms;
+ allow $1 cupsd_var_run_t:sock_file { read write_sock_file_perms };
+ allow $1 cupsd_t:unix_stream_socket connectto;
')

########################################

--
My Main Blog http://etbe.coker.com.au/
My Documents Blog http://doc.coker.com.au/

2017-02-14 04:14:58

by Russell Coker

[permalink] [raw]
Subject: [refpolicy] [PATCH 2/2] cups: read permission for cupsd_var_run_t socket files in cups_read_pid_files()

On Tuesday, 14 February 2017 3:08:28 PM AEDT Russell Coker wrote:
> Does it make sense to have a cups_read_sock_files when the cups clients
> need read/write access? Why not just have a single interface granting
> read-write?

I just noticed you added read access to cups_stream_connect so
cups_read_sock_files is redundant.

--
My Main Blog http://etbe.coker.com.au/
My Documents Blog http://doc.coker.com.au/

2017-02-14 13:36:56

by guido

[permalink] [raw]
Subject: [refpolicy] [PATCH 2/2 v2] cups/lpd: read permission for cupsd_var_run_t socket files

Hello.

As correctly noted by Russell Coker, this patch is unneeded after part 1/2 is applied.

So, part 2/2 can be safely reverted.

Regards,

Guido

On the 12th of February 2017 19:35:12 CET, Chris PeBenito <[email protected]> wrote:
>On 02/11/17 15:13, Guido Trentalancia via refpolicy wrote:
>> Introduce a new interface in the cups module to read cups socket
>> files and call such interface from the lpd module.
>>
>> Thanks to Christpher PeBenito for revising this patch.
>>
>> Signed-off-by: Guido Trentalancia <[email protected]>
>> ---
>> policy/modules/contrib/cups.if | 19 +++++++++++++++++++
>> policy/modules/contrib/lpd.te | 1 +
>> 2 files changed, 20 insertions(+)
>>
>> diff -pru a/policy/modules/contrib/cups.if
>b/policy/modules/contrib/cups.if
>> --- a/policy/modules/contrib/cups.if 2017-01-24 18:56:19.569106107
>+0100
>> +++ b/policy/modules/contrib/cups.if 2017-02-11 21:04:00.346144792
>+0100
>> @@ -128,6 +128,25 @@ interface(`cups_read_pid_files',`
>>
>> ########################################
>> ## <summary>
>> +## Read cups socket files.
>> +## </summary>
>> +## <param name="domain">
>> +## <summary>
>> +## Domain allowed access.
>> +## </summary>
>> +## </param>
>> +#
>> +interface(`cups_read_sock_files',`
>> + gen_require(`
>> + type cupsd_var_run_t;
>> + ')
>> +
>> + files_search_pids($1)
>> + allow $1 cupsd_var_run_t:sock_file read_sock_file_perms;
>> +')
>> +
>> +########################################
>> +## <summary>
>> ## Execute cups_config in the
>> ## cups config domain.
>> ## </summary>
>> diff -pru a/policy/modules/contrib/lpd.te
>b/policy/modules/contrib/lpd.te
>> --- a/policy/modules/contrib/lpd.te 2016-12-22 23:12:59.385081782
>+0100
>> +++ b/policy/modules/contrib/lpd.te 2017-02-11 21:04:28.457255575
>+0100
>> @@ -295,6 +295,7 @@ optional_policy(`
>> cups_read_config(lpr_t)
>> cups_stream_connect(lpr_t)
>> cups_read_pid_files(lpr_t)
>> + cups_read_sock_files(lpr_t)
>> ')
>>
>> optional_policy(`
>
>Merged.