[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-05-08 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #31 from Javier Martinez Canillas  ---
So I finally found some time to work on this, as agreed I went with (b).

Following is the pull request for Fedora selinux-policy-contrib repo. Please
let me know if I got something wrong:


https://github.com/fedora-selinux/selinux-policy-contrib/pull/57

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-10 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #30 from dac.overr...@gmail.com ---
Yes, It would have been less painful if your process did not pass fd's to dbus.
That is really something I dislike about dbus. I think I like varlink a lot in
that regard.

Nevertheless, I agree that currently b is probably the best way to go.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-10 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #29 from Javier Martinez Canillas  ---
Got it. Thanks a lot for your explanations.

I think I'll probably go with (b) then. I like the idea of having independent
modules for SELinux policies but now I understand that policies for the
different components are more coupled than I thought.

I would like to go with (a), but my SELinux knowledge is close to non-existent
so I'm by no means qualified to set a precedence on this.

I'm really just interested in updating tpm2-abrmd to the latest release to be
honest.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-10 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #28 from dac.overr...@gmail.com ---
The CIL policy language would be a solution to this particular challenge. With
the CIL language the interfaces are part of the modules. That means that there
are no header packages. The interfaces are alway's available in the module
store

CIL provides other benefits for this modularity scenario, The thing is that CIL
is meant to be a intermediate layer, and there currently is no higher level
language that leverages CIL.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-10 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #27 from dac.overr...@gmail.com ---
Exactly.

a. Is in theory the most sane solution I Believe.
b. Is probably the most practical solution but that basically ignores
modularization
c. Would be a short-term solution but is eventually probably a dead-end and
sets a bad precendence. You see processes interact and operate. The purpose of
the interfaces is to keep policy maintainable. If you start ignoring the
interfaces that introduces inconsistencies and eventually part of the policy
becomes hard to maintain

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-10 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #26 from Javier Martinez Canillas  ---
(In reply to dac.override from comment #25)
> Basically the way I see it is that this modularization effort requires that
> the headers are alway's installed if policy is installed. That then means
> that the various policy-devel packages need to alway's be installed.

Right, and then selinux-policy would need a BuildRequires dependency with
tpm2-abrmd-selinux-devel (and all the -devel packages exporting interfaces).
But then it won't be an independent SELinux policy module anymore as explained
in the IndependentPolicy guideline...

So I think that we have these options:

a) Due as you propose and make selinux-policy-contrib to BuildRequires
tpm2-abrmd-selinux-devel

b) Not having a tpm2-abrmd-selinux package and instead add the tpm2-abrmd AV
rules to selinux-policy-contrib.

c) Just have "allow system_dbusd_t tabrmd_t:unix_stream_socket { read write }"
in optional_policy as you first suggested.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-10 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #25 from dac.overr...@gmail.com ---
Basically the way I see it is that this modularization effort requires that the
headers are alway's installed if policy is installed. That then means that the
various policy-devel packages need to alway's be installed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-10 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #24 from dac.overr...@gmail.com ---
In other words, you might get into a chicken and egg situation here.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-10 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #23 from dac.overr...@gmail.com ---
Indeed when the dbus module gets compiled it will be looking for the
tabrmd_rw_inherited_unix_stream_sockets() interface that you export in
tabrmd.if

If it is not there at build-time then it will just not include the rule.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-10 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #22 from dac.overr...@gmail.com ---
Yes.This is not going to work.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-10 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #21 from Javier Martinez Canillas  ---
(In reply to dac.override from comment #20)
> So basically you export "tabrmd_rw_inherited_unix_stream_sockets()" in
> tabrmd.if and then you call "optional_policy(`
> tabrmd_rw_inherited_unix_stream_sockets(dbusd_system_t) ')" in dbus.te

I see, so then tpm2-abrmd-selinux will have to depend on a version of
selinux-policy-contrib that contains the dbus.te changes, right?

I would also like Lukas opinion about this as well before doing the proposed
change.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #20 from dac.overr...@gmail.com ---
So basically you export "tabrmd_rw_inherited_unix_stream_sockets()" in
tabrmd.if and then you call "optional_policy(`
tabrmd_rw_inherited_unix_stream_sockets(dbusd_system_t) ')" in dbus.te

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #19 from dac.overr...@gmail.com ---
typo's


## 
##Use and inherit tabrmd file descriptors.
## 
## 
##
##Domain allowed access.
##
## 
#
interface(`tabrmd_use_fds',`
gen_require(`
type tabrmd_t;
')

allow $1 tabrmd_t:fd use;
')


## 
##Read and write inherited tabrmd unix stream sockets.
## 
## 
##
##Domain allowed access.
##
## 
#
interface(`tabrmd_rw_inherited_unix_stream_sockets',`
gen_require(`
type tabrmd_t;
')

tabrmd_use_fds($1)
allow $1 tabrmd_t:unix_stream_socket { read write };
')

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #18 from dac.overr...@gmail.com ---
I other words this also demonstrates how the "selinux-policy modularization"
effort lacks. Even now you have to ideally add changes to selinux-policy
(dbus.te and file_contexts.subs_dist) to get it nice and tidy

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #17 from dac.overr...@gmail.com ---
Oops i am wrong

You should add a tabrmd_rw_inherited_unix_stream_sockets() interface to
tabrmd.if
and them call that in dbus.if instead


## 
##Use and inherit system tabrmd file descriptors.
## 
## 
##
##Domain allowed access.
##
## 
#
interface(`tabrmd_use_fds',`
gen_require(`
type tabrmd_t;
')

allow $1 tabrmd_t:fd use;
')


## 
##Read and write inherited tabrmd DBUS unix stream sockets.
## 
## 
##
##Domain allowed access.
##
## 
#
interface(`tabrmd_rw_inherited_unix_stream_sockets',`
gen_require(`
type tabrmd_t;
')

tabrmd_use_fds($1)
allow $1 tabrmd_t:unix_stream_socket { read write };
')

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #16 from Javier Martinez Canillas  ---
(In reply to dac.override from comment #15)
> it should be clarified because it is questionable.
> 
> If a "system_dbusd_domain" would need this permission then the permission
> would have been enclosed with "system_dbusd_domain()"
> 
> Looking at 
> https://github.com/tpm2-software/tpm2-abrmd/commit/51a3c55d772b
> it seems that this file descriptor gets passed to dbusd
> 
> So at least now that part is explained.
> 
> ideally the dbusd.if header would have exported an
> "dbus_rw_inherited_system_unix_stream_sockets()" interface for you to call,
> but there is not so just change line:
> 
> https://github.com/tpm2-software/tpm2-abrmd/blob/1.x/selinux/tabrmd.te#L20
> 
> to look like:
> 
> allow system_dbusd_t tabrmd_t:unix_stream_socket { read write};
> 
> Optionally add a comment: # TODO: add to dbus.if:
> dbus_rw_inherited_system_unix_stream_sockets() and call that instead

I will, thanks again!

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #15 from dac.overr...@gmail.com ---
it should be clarified because it is questionable.

If a "system_dbusd_domain" would need this permission then the permission would
have been enclosed with "system_dbusd_domain()"

Looking at 
https://github.com/tpm2-software/tpm2-abrmd/commit/51a3c55d772b
it seems that this file descriptor gets passed to dbusd

So at least now that part is explained.

ideally the dbusd.if header would have exported an
"dbus_rw_inherited_system_unix_stream_sockets()" interface for you to call, but
there is not so just change line:

https://github.com/tpm2-software/tpm2-abrmd/blob/1.x/selinux/tabrmd.te#L20

to look like:

allow system_dbusd_t tabrmd_t:unix_stream_socket { read write};

Optionally add a comment: # TODO: add to dbus.if:
dbus_rw_inherited_system_unix_stream_sockets() and call that instead

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #14 from Javier Martinez Canillas  ---
(In reply to dac.override from comment #13)
> also this should be investigated reproduced:
> 
> https://github.com/tpm2-software/tpm2-abrmd/blob/1.x/selinux/tabrmd.te#L20
> 
> Its definitely not "rw_stream_socket_perms", if anything it is
> "unix_stream_socket { read write }" but even that should be clarified

Ah, I see. The rw_stream_socket_perms it's actually much more than just read
and write by looking at its definition in selinux-policy. I think you are
correct and unix_stream_socket { read write } should be enough.

What do you mean by clarified? That's the reason why we need this policy in the
first place, it's needed after the following tpm2-abrmd commit:

https://github.com/tpm2-software/tpm2-abrmd/commit/51a3c55d772b

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #13 from dac.overr...@gmail.com ---
also this should be investigated reproduced:

https://github.com/tpm2-software/tpm2-abrmd/blob/1.x/selinux/tabrmd.te#L20

Its definitely not "rw_stream_socket_perms", if anything it is
"unix_stream_socket { read write }" but even that should be clarified

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #12 from Javier Martinez Canillas  ---
(In reply to dac.override from comment #10)
> redundant:
> https://github.com/tpm2-software/tpm2-abrmd/blob/1.x/selinux/tabrmd.te#L12
> 
> No i mean that you should probably populate that file with at least a
> minimal set of interfaces to interface with your domain.
> 
> Also thart .if file should ideally be installed with a seperate header
> devel-rpm that relies on the selinux-policy-devel rpm
> 
> This is a "header file" or a "devel" file

Got it, I saw your other comment mentioned that too. I'll take care of all
these when doing a re-spin of the package.

Thanks a lot for your review!

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #11 from dac.overr...@gmail.com ---
redudant:
https://github.com/tpm2-software/tpm2-abrmd/blob/1.x/selinux/tabrmd.te#L18

the system_dbusd_t type is already enclosed with "dbus_system_domain()", no
need to "import" it again with "dbus_stub()"

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #10 from dac.overr...@gmail.com ---
redundant:
https://github.com/tpm2-software/tpm2-abrmd/blob/1.x/selinux/tabrmd.te#L12

No i mean that you should probably populate that file with at least a minimal
set of interfaces to interface with your domain.

Also thart .if file should ideally be installed with a seperate header
devel-rpm that relies on the selinux-policy-devel rpm

This is a "header file" or a "devel" file

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #9 from Javier Martinez Canillas  ---
(In reply to dac.override from comment #4)
> tpm2-abrmd-1.2.0/selinux/tabrmd.te:
> 
> allow tabrmd_t self:unix_dgram_socket { create_socket_perms };
> 
> redundant: provided by logging_send_syslog_msg(tabrmd_t)
> 
> https://github.com/fedora-selinux/selinux-policy/blob/rawhide/policy/modules/
> system/logging.te#L691
> 
> Questionable (can you reproduce this?): 
> 
> # This next bit doesn't belong here. It should be exposed through an
> # interface likely from the dbus policy module.
> gen_require(`
> type system_dbusd_t;
> ')
> allow system_dbusd_t tabrmd_t:unix_stream_socket { read write };
> 
> If you can reproduce this then it should be inside the below optional block
> (no need to require type system_dbusd_t:
> 
> optional_policy(`
> dbus_system_domain(tabrmd_t, tabrmd_exec_t)
> ')
>

Can you please take a look to the latest version of the policy module? Lukas
already fixed tpm2-abrmd upstream:

https://github.com/tpm2-software/tpm2-abrmd/blob/1.x/selinux/tabrmd.te

 > Your tabrmd.if file is useless (its like a library providing interfaces
> required to interact with your domain).

Do you mean that it can just be removed? Sorry for the silly question but I'm
not that familiar with SELinux.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #8 from Javier Martinez Canillas  ---
(In reply to Robert-André Mauchin from comment #3)
>  - Add the LICENSE file with %license in %install
> 
>  - Own these directories:
> 
> [!]: Package must own all directories that it creates.
>  Note: Directories without known owners:
>  /usr/share/selinux/devel/include/contrib,
>  /usr/share/selinux/devel/include, /usr/share/selinux/devel
> 
>  - Use %make_build instead of make for parallel build (unless it fails the
> build)
> 
>  
>  Package Review
> ==

I'll fix all these and upload a new version. Thanks!

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #7 from Javier Martinez Canillas  ---
(In reply to Robert-André Mauchin from comment #2)
> Thanks Lukas, I'm not a SELinux specialist so I didn't take this package,
> I''ll finish the review now.
>

Thanks a lot for your review!

> 
>  - These are not needed as this is the default:
> 
> %defattr(-,root,root,0755)
> %attr(0644,root,root)
> %attr(0644,root,root)
> 

Ok.

> 
>  - The latest version of tpm2-abrmd is 1.3.1, please bump your package.
>

That version wasn't released yet when I proposed the package for review more
than a month ago (my original plan was to get reviewed so I could have this and
update the tpm2-abrmd package to 1.3.0).

>  - The version in the header and the %changeloq are mismatched:
> 
> * Thu Mar 01 2018 Javier Martinez Canillas  - 0.0.1-1
>
>   It should be 1.2.0-1 (or 1.3.1-1 when you update)

Right, I thought that other packages were using their selinux_policyver in the
%changelog but probably just got confused. I'll use the Version-Release
instead.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #6 from dac.overr...@gmail.com ---
https://raw.githubusercontent.com/martinezjavier/tpm2-abrmd-selinux/master/tpm2-abrmd-selinux.spec

Excuse me but I believe that this spec is wrong:

The tabrmd.if file should be installed optionally seperately as part of a
tpm2-abrmd-selinux-devel rpm, that requires selinux-policy-devel package (that
owns /etc/selinux/targeted/devel

Look at these .if files as development headers

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #5 from dac.overr...@gmail.com ---
tabrmd.fc: arguably a bug in selinux-policy:

/usr/local/sbin/tpm2-abrmd  -- 
gen_context(system_u:object_r:tabrmd_exec_t,s0)

ideally an entry should be added to:

https://github.com/fedora-selinux/selinux-policy/blob/rawhide/config/file_contexts.subs_dist

/usr/local/sbin /usr/sbin

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595

dac.overr...@gmail.com changed:

   What|Removed |Added

 CC||dac.overr...@gmail.com



--- Comment #4 from dac.overr...@gmail.com ---
tpm2-abrmd-1.2.0/selinux/tabrmd.te:

allow tabrmd_t self:unix_dgram_socket { create_socket_perms };

redundant: provided by logging_send_syslog_msg(tabrmd_t)

https://github.com/fedora-selinux/selinux-policy/blob/rawhide/policy/modules/system/logging.te#L691

Questionable (can you reproduce this?): 

# This next bit doesn't belong here. It should be exposed through an
# interface likely from the dbus policy module.
gen_require(`
type system_dbusd_t;
')
allow system_dbusd_t tabrmd_t:unix_stream_socket { read write };

If you can reproduce this then it should be inside the below optional block (no
need to require type system_dbusd_t:

optional_policy(`
dbus_system_domain(tabrmd_t, tabrmd_exec_t)
')

Your tabrmd.if file is useless (its like a library providing interfaces
required to interact with your domain).

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #3 from Robert-André Mauchin  ---
 - Add the LICENSE file with %license in %install

 - Own these directories:

[!]: Package must own all directories that it creates.
 Note: Directories without known owners:
 /usr/share/selinux/devel/include/contrib,
 /usr/share/selinux/devel/include, /usr/share/selinux/devel

 - Use %make_build instead of make for parallel build (unless it fails the
build)


 Package Review
==

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



= MUST items =

Generic:
[x]: Package is licensed with an open-source compatible license and meets
 other legal requirements as defined in the legal section of Packaging
 Guidelines.
[!]: If (and only if) the source package includes the text of the
 license(s) in its own file, then that file, containing the text of the
 license(s) for the package is included in %license.
[x]: License field in the package spec file matches the actual license.
 Note: Checking patched sources after %prep for licenses. Licenses
 found: "*No copyright* BSD (2 clause)", "BSD (2 clause)", "Unknown or
 generated". 30 files have unknown license. Detailed output of
 licensecheck in /home/bob/packaging/review/tpm2-abrmd-selinux/review-
 tpm2-abrmd-selinux/licensecheck.txt
[!]: Package must own all directories that it creates.
 Note: Directories without known owners:
 /usr/share/selinux/devel/include/contrib,
 /usr/share/selinux/devel/include, /usr/share/selinux/devel
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[!]: Each %files section contains %defattr if rpm < 4.4
 Note: %defattr present but not needed
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
 names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
 Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
 one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
 Note: There are rpmlint messages (see attachment).
[x]: Package requires other packages for directories it uses.
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
 beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
 work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
 provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
 %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
 (~1MB) or number of files.
 Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

= SHOULD items =

Generic:
[!]: Uses parallel make %{?_smp_mflags} macro.
[-]: If the source package does not include license text(s) as a separate
 file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[?]: Package functions as described.
[!]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
 translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
 architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
 files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no 

[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595

Robert-André Mauchin  changed:

   What|Removed |Added

 CC||zebo...@gmail.com



--- Comment #2 from Robert-André Mauchin  ---
Thanks Lukas, I'm not a SELinux specialist so I didn't take this package, I''ll
finish the review now.


 - These are not needed as this is the default:

%defattr(-,root,root,0755)
%attr(0644,root,root)
%attr(0644,root,root)


 - The latest version of tpm2-abrmd is 1.3.1, please bump your package.

 - The version in the header and the %changeloq are mismatched:

* Thu Mar 01 2018 Javier Martinez Canillas  - 0.0.1-1

  It should be 1.2.0-1 (or 1.3.1-1 when you update)

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org


[Bug 1550595] Review Request: tpm2-abrmd-selinux - SELinux policies for tpm2-abrmd

2018-04-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1550595



--- Comment #1 from Lukas Vrabec  ---
Hi All,

I reviewed SELinux security policy for tpm2-abrmd and both spec file and
policy looks good to me, it reflects IndependentPolicy guidelines.

Thanks,
Lukas.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org