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

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

[Guido Trentalancia]

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 <guido@trentalancia.net>
---
 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)
 ')
 

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

[Guido Trentalancia]

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 <guido@trentalancia.net>
---
 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;
 ')
 
 ########################################

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

[Chris PeBenito]

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 <guido@trentalancia.net>
> ---
>  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

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

[Guido Trentalancia]

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

On the 11th of February 2017 20:54:00 CET, Chris PeBenito <pebenito@ieee.org> 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 <guido@trentalancia.net>
>> ---
>>  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] [PATCH 2/2 v2] cups/lpd: read permission for cupsd_var_run_t socket files

[Guido Trentalancia]

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 <guido@trentalancia.net>
---
 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(`

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

[Dominick Grift]

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 <pebenito@ieee.org> 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 <guido@trentalancia.net>
>>> ---
>>>  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 

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

[Guido Trentalancia]

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 <refpolicy@oss.tresys.com> 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
><pebenito@ieee.org> 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 <guido@trentalancia.net>
>>>> ---
>>>>  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
>> 

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

[Russell Coker]

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/

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

[Chris PeBenito]

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 <guido@trentalancia.net>
> ---
>  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

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

[Chris PeBenito]

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 <guido@trentalancia.net>
> ---
>  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

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

[Chris PeBenito]

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

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

[Russell Coker]

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/

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

[Russell Coker]

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/

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

[Guido Trentalancia]

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 <pebenito@ieee.org> 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 <guido@trentalancia.net>
>> ---
>>  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.