On Tue, 2007-11-13 at 11:05 -0500, Joshua Brindle wrote:
> Chad Sellers wrote:
> > On 11/7/07 4:26 PM, "Stephen Smalley" <sds@xxxxxxxxxxxxx> wrote:
> >
> >
> >> On Wed, 2007-11-07 at 16:17 -0500, Chad Sellers wrote:
> >>
> >>> The below patch adds a -i option to load_policy to perform the initial
> >>> policy load. The inital policy load is currently done in systems using
> >>> sysvinit by init itself, which then re-exec's itself. Ubuntu uses
> >>> upstart instead of sysvinit. In talks with the Ubuntu folks, they'd
> >>> prefer to load policy from initramfs before upstart starts rather than
> >>> patching upstart.
> >>>
> >>> Signed-off-by: Chad Sellers <csellers@xxxxxxxxxx>
> >>>
>
> Is this ready to be merged or are there outstanding issues?
I'd still suggest adding an error message to that code path - silently
exiting with an error status considered confusing, even if the initial
user will be checking the exit status and providing an error message
itself.
>
> >>> ---
> >>>
> >>> load_policy.8 | 19 ++++++++++++++++++-
> >>> load_policy.c | 29 +++++++++++++++++++++++++----
> >>> 2 files changed, 43 insertions(+), 5 deletions(-)
> >>>
> >>> Index: policycoreutils/load_policy/load_policy.c
> >>> ===================================================================
> >>> --- policycoreutils/load_policy/load_policy.c (revision 2679)
> >>> +++ policycoreutils/load_policy/load_policy.c (working copy)
> >>> @@ -19,13 +19,13 @@
> >>>
> >>> void usage(char *progname)
> >>> {
> >>> - fprintf(stderr, _("usage: %s [-q]\n"), progname);
> >>> + fprintf(stderr, _("usage: %s [-qi]\n"), progname);
> >>> exit(1);
> >>> }
> >>>
> >>> int main(int argc, char **argv)
> >>> {
> >>> - int ret, opt, quiet = 0, nargs;
> >>> + int ret, opt, quiet = 0, nargs, init=0, enforce=0;
> >>>
> >>> #ifdef USE_NLS
> >>> setlocale(LC_ALL, "");
> >>> @@ -33,7 +33,7 @@
> >>> textdomain(PACKAGE);
> >>> #endif
> >>>
> >>> - while ((opt = getopt(argc, argv, "bq")) > 0) {
> >>> + while ((opt = getopt(argc, argv, "bqi")) > 0) {
> >>> switch (opt) {
> >>> case 'b':
> >>> fprintf(stderr, "%s: Warning! The -b option is no longer
> >>> supported, booleans are always preserved across reloads. Continuing...\n",
> >>> @@ -43,6 +43,9 @@
> >>> quiet = 1;
> >>> sepol_debug(0);
> >>> break;
> >>> + case 'i':
> >>> + init = 1;
> >>> + break;
> >>> default:
> >>> usage(argv[0]);
> >>> }
> >>> @@ -62,7 +65,25 @@
> >>> argv[0], argv[optind++]);
> >>> }
> >>>
> >>> - ret = selinux_mkload_policy(1);
> >>> + if (init) {
> >>> + if (is_selinux_enabled() == 1) {
> >>> + /* SELinux is already enabled, we should not do an initial
> >>> load again */
> >>> + fprintf(stderr,
> >>> + _("%s: Policy is already loaded and initial load
> >>> requested\n"),
> >>> + argv[0]);
> >>> + exit(2);
> >>> + }
> >>> + ret = selinux_init_load_policy(&enforce);
> >>> + if (ret != 0 ) {
> >>> + if (enforce > 0) {
> >>> + /* SELinux in enforcing mode but load_policy failed */
> >>>
> >> An error message here would be helpful, assuming that such error
> >> messages are displayed at all on the console.
> >>
> >>
> > I was planning to just display the error in the caller, as the caller will
> > be the one to halt the system (not load_policy).
> >
> >
> >> How do you plan to handle an error in the caller? System should be
> >> halted in this case.
> >>
> >>
> > I plan to check the return value in the caller and halt in this case. That's
> > why I added a new return value (3). Basically, something like this:
> >
> > set +e
> > chroot /root load_policy -i
> > RET=$?
> > if [ $RET -eq 3 ]; then echo "SELinux policy load failed and enforcing mode
> > requested, halting now"; halt;
> > elif [ $RET -ne 0 ]; then echo "SELinux policy load failed, continuing";
> > fi
> >
> >
> >>> + exit(3);
> >>> + }
> >>> + }
> >>> + }
> >>> + else {
> >>> + ret = selinux_mkload_policy(1);
> >>> + }
> >>> if (ret < 0) {
> >>> fprintf(stderr, _("%s: Can't load policy: %s\n"),
> >>> argv[0], strerror(errno));
> >>> Index: policycoreutils/load_policy/load_policy.8
> >>> ===================================================================
> >>> --- policycoreutils/load_policy/load_policy.8 (revision 2679)
> >>> +++ policycoreutils/load_policy/load_policy.8 (working copy)
> >>> @@ -4,7 +4,7 @@
> >>>
> >>> .SH SYNOPSIS
> >>> .B load_policy
> >>> -[-q]
> >>> +[-qi]
> >>> .br
> >>> .SH DESCRIPTION
> >>> .PP
> >>> @@ -17,7 +17,24 @@
> >>> .TP
> >>> .B \-q
> >>> suppress warning messages.
> >>> +.TP
> >>> +.B \-i
> >>> +inital policy load. Only use this if this is the first time policy is
> >>> being loaded since boot (usually called from initramfs).
> >>>
> >>> +.SH "EXIT STATUS"
> >>> +.TP
> >>> +.B 0
> >>> +Success
> >>> +.TP
> >>> +.B 1
> >>> +Invalid option
> >>> +.TP
> >>> +.B 2
> >>> +Policy load failed
> >>> +.TP
> >>> +.B 3
> >>> +Initial policy load failed and enforcing mode requested
> >>> +
> >>> .SH SEE ALSO
> >>> .B booleans
> >>> (8),
> >>>
> >>>
> >>> --
> >>> This message was distributed to subscribers of the selinux mailing list.
> >>> If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with
> >>> the words "unsubscribe selinux" without quotes as the message.
> >>>
> >
> >
> >
> > --
> > This message was distributed to subscribers of the selinux mailing list.
> > If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with
> > the words "unsubscribe selinux" without quotes as the message.
> >
> >
>
>
>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with
> the words "unsubscribe selinux" without quotes as the message.
--
Stephen Smalley
National Security Agency
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with
the words "unsubscribe selinux" without quotes as the message.
