* This patch adds some output to load_policy to say which policy file it tries to load.
@ 2010-12-13 18:39 Daniel J Walsh
2010-12-15 18:55 ` Chad Sellers
0 siblings, 1 reply; 4+ messages in thread
From: Daniel J Walsh @ 2010-12-13 18:39 UTC (permalink / raw)
To: 'Chad Sellers'; +Cc: SELinux
[-- Attachment #1: Type: text/plain, Size: 486 bytes --]
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Currently load_policy will just fail without a decent error message.
Note:
The patch has to check if load_policy failed on a disabled machine, in
order to not report an error.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk0GaEYACgkQrlYvE4MpobMxrwCg6JMdPm28IEuL2Eco++OCHThw
sYAAn2BTXe1BYCjYdzDAjnA08t0dKquQ
=N1Uu
-----END PGP SIGNATURE-----
[-- Attachment #2: load_policy.patch --]
[-- Type: text/plain, Size: 2085 bytes --]
diff --git a/policycoreutils/load_policy/load_policy.c b/policycoreutils/load_policy/load_policy.c
index 47d9b0f..566565f 100644
--- a/policycoreutils/load_policy/load_policy.c
+++ b/policycoreutils/load_policy/load_policy.c
@@ -1,3 +1,4 @@
+#define _GNU_SOURCE
#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>
@@ -23,6 +24,14 @@ void usage(char *progname)
exit(1);
}
+char *policy_path(void) {
+ char *path=NULL;
+ if (asprintf(&path, "%s.%d", selinux_binary_policy_path(), security_policyvers()) < 0) {
+ return NULL;
+ }
+ return path;
+}
+
int main(int argc, char **argv)
{
int ret, opt, quiet = 0, nargs, init=0, enforce=0;
@@ -64,6 +73,7 @@ int main(int argc, char **argv)
"%s: Warning! Boolean file argument (%s) is no longer supported, installed booleans file is always used. Continuing...\n",
argv[0], argv[optind++]);
}
+ errno = 0;
if (init) {
if (is_selinux_enabled() == 1) {
/* SELinux is already enabled, we should not do an initial load again */
@@ -76,9 +86,11 @@ int main(int argc, char **argv)
if (ret != 0 ) {
if (enforce > 0) {
/* SELinux in enforcing mode but load_policy failed */
+ char *path=policy_path();
fprintf(stderr,
- _("%s: Can't load policy and enforcing mode requested: %s\n"),
- argv[0], strerror(errno));
+ _("%s: Can't load policy file %s and enforcing mode requested: %s\n"),
+ argv[0], path, strerror(errno));
+ free(path);
exit(3);
}
}
@@ -86,9 +98,16 @@ int main(int argc, char **argv)
else {
ret = selinux_mkload_policy(1);
}
- if (ret < 0) {
- fprintf(stderr, _("%s: Can't load policy: %s\n"),
- argv[0], strerror(errno));
+
+ /* selinux_init_load_policy returns -1 if it did not load_policy
+ * On SELinux disabled system it will always return -1
+ * So check errno to see if anything went wrong
+ */
+ if (ret < 0 && errno != 0) {
+ char *path=policy_path();
+ fprintf(stderr, _("%s: Can't load policy file %s: %s\n"),
+ argv[0], path, strerror(errno));
+ free(path);
exit(2);
}
exit(0);
[-- Attachment #3: load_policy.patch.sig --]
[-- Type: application/pgp-signature, Size: 72 bytes --]
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: This patch adds some output to load_policy to say which policy file it tries to load.
2010-12-13 18:39 This patch adds some output to load_policy to say which policy file it tries to load Daniel J Walsh
@ 2010-12-15 18:55 ` Chad Sellers
2010-12-16 13:59 ` Daniel J Walsh
0 siblings, 1 reply; 4+ messages in thread
From: Chad Sellers @ 2010-12-15 18:55 UTC (permalink / raw)
To: Daniel J Walsh; +Cc: SELinux
On 12/13/10 1:39 PM, "Daniel J Walsh" <dwalsh@redhat.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Currently load_policy will just fail without a decent error message.
>
> Note:
>
> The patch has to check if load_policy failed on a disabled machine, in
> order to not report an error.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAk0GaEYACgkQrlYvE4MpobMxrwCg6JMdPm28IEuL2Eco++OCHThw
> sYAAn2BTXe1BYCjYdzDAjnA08t0dKquQ
> =N1Uu
> -----END PGP SIGNATURE-----
>
> diff --git a/policycoreutils/load_policy/load_policy.c
> b/policycoreutils/load_policy/load_policy.c
> index 47d9b0f..566565f 100644
> --- a/policycoreutils/load_policy/load_policy.c
> +++ b/policycoreutils/load_policy/load_policy.c
> @@ -1,3 +1,4 @@
> +#define _GNU_SOURCE
> #include <unistd.h>
> #include <stdlib.h>
> #include <stdio.h>
> @@ -23,6 +24,14 @@ void usage(char *progname)
> exit(1);
> }
>
> +char *policy_path(void) {
> + char *path=NULL;
> + if (asprintf(&path, "%s.%d", selinux_binary_policy_path(),
> security_policyvers()) < 0) {
> + return NULL;
> + }
> + return path;
> +}
> +
This function will return a bogus result if any error occurs in
security_policyvers(). The only likely candidate for that is if SELinux is
disabled, which this theoretically should not be called in. However, that
isn't true (more on that later). So, I get messages like this:
[root@f14 ~]# load_policy -i
load_policy: Can't load policy file /etc/selinux/targeted/policy/policy.-1:
No such file or directory
The -1 comes from the error return code from security_policyvers().
More importantly, this just assumes that the path computed here and in
libselinux are the same. Since libselinux searches back for policy versions,
this isn't necessarily true.
> int main(int argc, char **argv)
> {
> int ret, opt, quiet = 0, nargs, init=0, enforce=0;
> @@ -64,6 +73,7 @@ int main(int argc, char **argv)
> "%s: Warning! Boolean file argument (%s) is no longer
> supported, installed booleans file is always used. Continuing...\n",
> argv[0], argv[optind++]);
> }
> + errno = 0;
> if (init) {
> if (is_selinux_enabled() == 1) {
> /* SELinux is already enabled, we should not do an initial load
> again */
> @@ -76,9 +86,11 @@ int main(int argc, char **argv)
> if (ret != 0 ) {
> if (enforce > 0) {
> /* SELinux in enforcing mode but load_policy failed */
> + char *path=policy_path();
> fprintf(stderr,
> - _("%s: Can't load policy and enforcing mode
> requested: %s\n"),
> - argv[0], strerror(errno));
> + _("%s: Can't load policy file %s and enforcing mode
> requested: %s\n"),
> + argv[0], path, strerror(errno));
> + free(path);
This assumes errno is set by selinux_init_load_policy() (more on this
below).
> exit(3);
> }
> }
> @@ -86,9 +98,16 @@ int main(int argc, char **argv)
> else {
> ret = selinux_mkload_policy(1);
> }
> - if (ret < 0) {
> - fprintf(stderr, _("%s: Can't load policy: %s\n"),
> - argv[0], strerror(errno));
> +
> + /* selinux_init_load_policy returns -1 if it did not load_policy
> + * On SELinux disabled system it will always return -1
> + * So check errno to see if anything went wrong
> + */
> + if (ret < 0 && errno != 0) {
> + char *path=policy_path();
> + fprintf(stderr, _("%s: Can't load policy file %s: %s\n"),
> + argv[0], path, strerror(errno));
> + free(path);
This assumes that errno is set properly by selinux_init_load_policy() or
selinux_mkload_policy(). It's not. For instance, if /selinux can't be
mounted (because SELinux is disabled), errno will be set to ENODEV. So, this
new errno check doesn't seem to help here. For instance, I booted my F14
system with selinux=0 on the kernel command-line. Then:
[root@f14 ~]# load_policy -i
load_policy: Can't load policy file /etc/selinux/targeted/policy/policy.-1:
No such file or directory
I'd say we either need a proper communication channel (e.g. return code or
start setting errno properly) between libselinux and load_policy, or we need
libselinux to handle everything (including logging) itself.
Thanks,
Chad
> exit(2);
> }
> exit(0);
>
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: This patch adds some output to load_policy to say which policy file it tries to load.
2010-12-15 18:55 ` Chad Sellers
@ 2010-12-16 13:59 ` Daniel J Walsh
2010-12-16 14:07 ` Daniel J Walsh
0 siblings, 1 reply; 4+ messages in thread
From: Daniel J Walsh @ 2010-12-16 13:59 UTC (permalink / raw)
To: Chad Sellers; +Cc: SELinux
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 12/15/2010 01:55 PM, Chad Sellers wrote:
> On 12/13/10 1:39 PM, "Daniel J Walsh" <dwalsh@redhat.com> wrote:
>
> Currently load_policy will just fail without a decent error message.
>
> Note:
>
> The patch has to check if load_policy failed on a disabled machine, in
> order to not report an error.
>>
diff --git a/policycoreutils/load_policy/load_policy.c
b/policycoreutils/load_policy/load_policy.c
index 47d9b0f..566565f 100644
- --- a/policycoreutils/load_policy/load_policy.c
+++ b/policycoreutils/load_policy/load_policy.c
@@ -1,3 +1,4 @@
+#define _GNU_SOURCE
#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>
@@ -23,6 +24,14 @@ void usage(char *progname)
exit(1);
}
+char *policy_path(void) {
+ char *path=NULL;
+ if (asprintf(&path, "%s.%d", selinux_binary_policy_path(),
security_policyvers()) < 0) {
+ return NULL;
+ }
+ return path;
+}
+
> This function will return a bogus result if any error occurs in
> security_policyvers(). The only likely candidate for that is if SELinux is
> disabled, which this theoretically should not be called in. However, that
> isn't true (more on that later). So, I get messages like this:
> [root@f14 ~]# load_policy -i
> load_policy: Can't load policy file /etc/selinux/targeted/policy/policy.-1:
> No such file or directory
> The -1 comes from the error return code from security_policyvers().
> More importantly, this just assumes that the path computed here and in
> libselinux are the same. Since libselinux searches back for policy versions,
> this isn't necessarily true.
int main(int argc, char **argv)
{
int ret, opt, quiet = 0, nargs, init=0, enforce=0;
@@ -64,6 +73,7 @@ int main(int argc, char **argv)
"%s: Warning! Boolean file argument (%s) is no longer
supported, installed booleans file is always used. Continuing...\n",
argv[0], argv[optind++]);
}
+ errno = 0;
if (init) {
if (is_selinux_enabled() == 1) {
/* SELinux is already enabled, we should not do an initial load
again */
@@ -76,9 +86,11 @@ int main(int argc, char **argv)
if (ret != 0 ) {
if (enforce > 0) {
/* SELinux in enforcing mode but load_policy failed */
+ char *path=policy_path();
fprintf(stderr,
- - _("%s: Can't load policy and enforcing mode
requested: %s\n"),
- - argv[0], strerror(errno));
+ _("%s: Can't load policy file %s and enforcing
mode
requested: %s\n"),
+ argv[0], path, strerror(errno));
+ free(path);
> This assumes errno is set by selinux_init_load_policy() (more on this
> below).
exit(3);
}
}
@@ -86,9 +98,16 @@ int main(int argc, char **argv)
else {
ret = selinux_mkload_policy(1);
}
- - if (ret < 0) {
- - fprintf(stderr, _("%s: Can't load policy: %s\n"),
- - argv[0], strerror(errno));
+
+ /* selinux_init_load_policy returns -1 if it did not load_policy
+ * On SELinux disabled system it will always return -1
+ * So check errno to see if anything went wrong
+ */
+ if (ret < 0 && errno != 0) {
+ char *path=policy_path();
+ fprintf(stderr, _("%s: Can't load policy file %s: %s\n"),
+ argv[0], path, strerror(errno));
+ free(path);
> This assumes that errno is set properly by selinux_init_load_policy() or
> selinux_mkload_policy(). It's not. For instance, if /selinux can't be
> mounted (because SELinux is disabled), errno will be set to ENODEV. So, this
> new errno check doesn't seem to help here. For instance, I booted my F14
> system with selinux=0 on the kernel command-line. Then:
> [root@f14 ~]# load_policy -i
> load_policy: Can't load policy file /etc/selinux/targeted/policy/policy.-1:
> No such file or directory
> I'd say we either need a proper communication channel (e.g. return code or
> start setting errno properly) between libselinux and load_policy, or we need
> libselinux to handle everything (including logging) itself.
Libraries should not log. We should just set errno within libselinux to
0 if SELinux is disabled. If the goal was to disable SELinux. Which
might be the real problem is we are disabling SELInux with the
load_policy call.
> Thanks,
> Chad
exit(2);
}
exit(0);
>>
The original problem this is trying to solve was Eric was dealing with a
new policy version then the system supported. Lets say the system
supported 25, and the system supported 24, we would report an error
message that said "No such File" With no indication of which file it
tried to install.
So this patch would help his situation. How about something like.
char *policy_path(void) {
char *path=NULL;
int vers = security_policyvers();
if (vers > 0) {
if (asprintf(&path, "%s.%d", selinux_binary_policy_path(),
security_policyvers()) < 0) {
return NULL;
}
} else {
if (asprintf(&path, "%s.*",
selinux_binary_policy_path()) < 0) {
return NULL;
}
}
return path;
}
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk0KGzgACgkQrlYvE4MpobN+yACcDyPZf54gs5ST4x+B6OALWNaN
3aMAn3ASY+rwuu+/e2tRq5Jis9OpjvDp
=/Xu0
-----END PGP SIGNATURE-----
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: This patch adds some output to load_policy to say which policy file it tries to load.
2010-12-16 13:59 ` Daniel J Walsh
@ 2010-12-16 14:07 ` Daniel J Walsh
0 siblings, 0 replies; 4+ messages in thread
From: Daniel J Walsh @ 2010-12-16 14:07 UTC (permalink / raw)
To: Chad Sellers; +Cc: SELinux
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 12/16/2010 08:59 AM, Daniel J Walsh wrote:
> On 12/15/2010 01:55 PM, Chad Sellers wrote:
>> On 12/13/10 1:39 PM, "Daniel J Walsh" <dwalsh@redhat.com> wrote:
>
>> Currently load_policy will just fail without a decent error message.
>
>> Note:
>
>> The patch has to check if load_policy failed on a disabled machine, in
>> order to not report an error.
>>>
> diff --git a/policycoreutils/load_policy/load_policy.c
> b/policycoreutils/load_policy/load_policy.c
> index 47d9b0f..566565f 100644
> --- a/policycoreutils/load_policy/load_policy.c
> +++ b/policycoreutils/load_policy/load_policy.c
> @@ -1,3 +1,4 @@
> +#define _GNU_SOURCE
> #include <unistd.h>
> #include <stdlib.h>
> #include <stdio.h>
> @@ -23,6 +24,14 @@ void usage(char *progname)
> exit(1);
> }
>
> +char *policy_path(void) {
> + char *path=NULL;
> + if (asprintf(&path, "%s.%d", selinux_binary_policy_path(),
> security_policyvers()) < 0) {
> + return NULL;
> + }
> + return path;
> +}
> +
>
>> This function will return a bogus result if any error occurs in
>> security_policyvers(). The only likely candidate for that is if SELinux is
>> disabled, which this theoretically should not be called in. However, that
>> isn't true (more on that later). So, I get messages like this:
>
>> [root@f14 ~]# load_policy -i
>> load_policy: Can't load policy file /etc/selinux/targeted/policy/policy.-1:
>> No such file or directory
>
>> The -1 comes from the error return code from security_policyvers().
>
>> More importantly, this just assumes that the path computed here and in
>> libselinux are the same. Since libselinux searches back for policy versions,
>> this isn't necessarily true.
>
> int main(int argc, char **argv)
> {
> int ret, opt, quiet = 0, nargs, init=0, enforce=0;
> @@ -64,6 +73,7 @@ int main(int argc, char **argv)
> "%s: Warning! Boolean file argument (%s) is no longer
> supported, installed booleans file is always used. Continuing...\n",
> argv[0], argv[optind++]);
> }
> + errno = 0;
> if (init) {
> if (is_selinux_enabled() == 1) {
> /* SELinux is already enabled, we should not do an initial load
> again */
> @@ -76,9 +86,11 @@ int main(int argc, char **argv)
> if (ret != 0 ) {
> if (enforce > 0) {
> /* SELinux in enforcing mode but load_policy failed */
> + char *path=policy_path();
> fprintf(stderr,
> - _("%s: Can't load policy and enforcing mode
> requested: %s\n"),
> - argv[0], strerror(errno));
> + _("%s: Can't load policy file %s and enforcing
> mode
> requested: %s\n"),
> + argv[0], path, strerror(errno));
> + free(path);
>
>> This assumes errno is set by selinux_init_load_policy() (more on this
>> below).
>
> exit(3);
> }
> }
> @@ -86,9 +98,16 @@ int main(int argc, char **argv)
> else {
> ret = selinux_mkload_policy(1);
> }
> - if (ret < 0) {
> - fprintf(stderr, _("%s: Can't load policy: %s\n"),
> - argv[0], strerror(errno));
> +
> + /* selinux_init_load_policy returns -1 if it did not load_policy
> + * On SELinux disabled system it will always return -1
> + * So check errno to see if anything went wrong
> + */
> + if (ret < 0 && errno != 0) {
> + char *path=policy_path();
> + fprintf(stderr, _("%s: Can't load policy file %s: %s\n"),
> + argv[0], path, strerror(errno));
> + free(path);
>
>> This assumes that errno is set properly by selinux_init_load_policy() or
>> selinux_mkload_policy(). It's not. For instance, if /selinux can't be
>> mounted (because SELinux is disabled), errno will be set to ENODEV. So, this
>> new errno check doesn't seem to help here. For instance, I booted my F14
>> system with selinux=0 on the kernel command-line. Then:
>
>> [root@f14 ~]# load_policy -i
>> load_policy: Can't load policy file /etc/selinux/targeted/policy/policy.-1:
>> No such file or directory
>
>> I'd say we either need a proper communication channel (e.g. return code or
>> start setting errno properly) between libselinux and load_policy, or we need
>> libselinux to handle everything (including logging) itself.
>
> Libraries should not log. We should just set errno within libselinux to
> 0 if SELinux is disabled. If the goal was to disable SELinux. Which
> might be the real problem is we are disabling SELInux with the
> load_policy call.
>> Thanks,
>> Chad
>
>
>
> exit(2);
> }
> exit(0);
>>>
>
> The original problem this is trying to solve was Eric was dealing with a
> new policy version then the system supported. Lets say the system
> supported 25, and the system supported 24, we would report an error
> message that said "No such File" With no indication of which file it
> tried to install.
>
> So this patch would help his situation. How about something like.
>
> char *policy_path(void) {
> char *path=NULL;
> int vers = security_policyvers();
> if (vers > 0) {
> if (asprintf(&path, "%s.%d", selinux_binary_policy_path(),
> security_policyvers()) < 0) {
> return NULL;
> }
> } else {
> if (asprintf(&path, "%s.*",
> selinux_binary_policy_path()) < 0) {
> return NULL;
> }
> }
> return path;
> }
>
- --
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov
with
the words "unsubscribe selinux" without quotes as the message.
Cancel this patch, it looks like libselinux should be doing the correct
thing encluding printing the error if policy does not exist.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk0KHS8ACgkQrlYvE4MpobO6ZwCg5wPhUSwbM1jYsZcwPI1yHwMq
FgUAoMq1ojBxSFyYGziimhXqmHKJGyHF
=8N+o
-----END PGP SIGNATURE-----
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-12-16 14:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-13 18:39 This patch adds some output to load_policy to say which policy file it tries to load Daniel J Walsh
2010-12-15 18:55 ` Chad Sellers
2010-12-16 13:59 ` Daniel J Walsh
2010-12-16 14:07 ` Daniel J Walsh
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.