SELinux Archive on lore.kernel.org
 help / Atom feed
From: jwcart2 <jwcart2@tycho.nsa.gov>
To: Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com>,
	selinux@vger.kernel.org
Subject: Re: [Non-DoD Source] [PATCH] libsepol: error in CIL if a permission cannot be resolved
Date: Fri, 14 Jun 2019 11:35:15 -0400
Message-ID: <ac39d408-134f-8e8a-26a7-c61a5e75d42c@tycho.nsa.gov> (raw)
In-Reply-To: <20190613201827.8616-1-yuli@crunchydata.com>

On 6/13/19 4:18 PM, Yuli Khodorkovskiy wrote:
> In the following example, "relabeltoo" is not a valid permission
> in the loaded policy nor in the new module. Before, CIL would not
> complain about the invalid permission and proceed to install the module:
> 
> 	$ cat test.cil
> 
> 	(mlsconstrain (db_procedure (create relabeltoo)) (eq l2 h2))
> 
> With this patch, an error is now prompted to a user:
> 
> 	$ sudo semodule -i foo.cil
> 
> 	Failed to resolve permission relabeltoo
> 	Failed to resolve mlsconstrain statement at /etc/selinux/mls/tmp/modules/400/test/cil:1
> 	semodule:  Failed!
> 
> Signed-off-by: Yuli Khodorkovskiy <yuli@crunchydata.com>
> ---
>   libsepol/cil/src/cil_resolve_ast.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
> index ea08087d..22d37f05 100644
> --- a/libsepol/cil/src/cil_resolve_ast.c
> +++ b/libsepol/cil/src/cil_resolve_ast.c
> @@ -135,8 +135,11 @@ static int __cil_resolve_perms(symtab_t *class_symtab, symtab_t *common_symtab,
>   				if (class_flavor == CIL_MAP_CLASS) {
>   					cil_log(CIL_ERR, "Failed to resolve permission %s for map class\n", (char*)curr->data);
>   					goto exit;
> +				} else if (class_flavor == CIL_CLASS) {
> +					cil_log(CIL_ERR, "Failed to resolve permission %s\n", (char*)curr->data);
> +					goto exit;
>   				}
> -				cil_log(CIL_WARN, "Failed to resolve permission %s\n", (char*)curr->data);
> +
>   				/* Use an empty list to represent unknown perm */
>   				cil_list_init(&empty_list, perm_strs->flavor);
>   				cil_list_append(*perm_datums, CIL_LIST, empty_list);
> 

You essentially want to revert commits 46e157b47, da51020d6, and 2eefb20d8 from 
back in July to November 2016.

I did not do a good job of specifying the motivation in the initial patch in 
that series, but here is Steve's reply to Steve Lawrence's understandable objection.

 From Steve Smalley's email "Re: [PATCH] libsepol/cil: Warn instead of fail if 
permission is not resolve" on 7/28/16
> The specific motivation for the change is that Fedora is trying to
> expunge permissions from its policy that were never upstream (e.g.
> ptrace_child in class process, compromise_kernel in class capability2,
> etc).  This presently breaks policy updates when there is a third party
> or local policy module already installed that was built against the
> older selinux-policy-devel that had those permissions.  What is worse is
> that rpm thinks that the policy was updated, since only semodule fails
> during %post, so you could easily brick a system this way on an upgrade
> because you'll end up running the old policy but rpm will think it has
> succeeded and proceed to install any other updated packages, which may
> depend on the new policy.


I would be fine with reverting the error handling back to what it was initially. 
It seems unlikely that we are still worried about local policy modules built on 
three year old policy.

Is this still a concern to anyone?
Jim

-- 
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency

      reply index

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-13 20:18 Yuli Khodorkovskiy
2019-06-14 15:35 ` jwcart2 [this message]

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ac39d408-134f-8e8a-26a7-c61a5e75d42c@tycho.nsa.gov \
    --to=jwcart2@tycho.nsa.gov \
    --cc=selinux@vger.kernel.org \
    --cc=yuli.khodorkovskiy@crunchydata.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

SELinux Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/selinux/0 selinux/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 selinux selinux/ https://lore.kernel.org/selinux \
		selinux@vger.kernel.org selinux@archiver.kernel.org
	public-inbox-index selinux


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.selinux


AGPL code for this site: git clone https://public-inbox.org/ public-inbox