[PATCH v2 0/2] Add Secure boot support warnings

[Dave Young]

Changes from v1:
Use the error message from Vivek.
echo detail error in check_kdump_feasibility

Dave Young (2):
  kdumpctl: claim that kdump does not support secure boot when service
    start
  kdumpctl: status function cleanup

kdumpctl | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 53 insertions(+), 15 deletions(-)

[PATCH v2 1/2] kdumpctl: claim that kdump does not support secure boot when service start

[Dave Young]

Kdump does not support secure boot yet, so let's claim it is not supported
at the begginning of service start function.

In this patch for checking secure boot status I'm checking the efivars per
suggestion from pjones. see in code comments for the details.

Tested in Fedora 19 + qemu ovmf with secure boot enabled.

Signed-off-by: Dave Young 
---
 kdumpctl | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/kdumpctl b/kdumpctl
index abcdffd..aef3875 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -500,6 +500,43 @@ selinux_relabel()
    done
 }

+# Check if secure boot is being enforced.
+#
+# Per Peter Jones, we need check efivar SecureBoot-$(the UUID) and
+# SetupMode-$(the UUID), they are both 5 bytes binary data. The first four
+# bytes are the attributes associated with the variable and can safely be
+# ignored, the last bytes are one-byte true-or-false variables. If SecureBoot
+# is 1 and SetupMode is 0, then secure boot is being enforced.
+#
+# Assume efivars is mounted at /sys/firmware/efi/efivars.
+function is_secure_boot_enforced()
+{
+	local secure_boot_file setup_mode_file
+	local secure_boot_byte setup_mode_byte
+
+	secure_boot_file=$(find /sys/firmware/efi/efivars -name SecureBoot-* 2>/dev/null)
+	setup_mode_file=$(find /sys/firmware/efi/efivars -name SetupMode-* 2>/dev/null)
+
+	if [ -f "$secure_boot_file" ] && [ -f "$setup_mode_file" ]; then
+		secure_boot_byte=$(hexdump -v -e '/1 "%d\ "' $secure_boot_file|cut -d' ' -f 5)
+		setup_mode_byte=$(hexdump -v -e '/1 "%d\ "' $setup_mode_file|cut -d' ' -f 5)
+
+		if [ "$secure_boot_byte" = "1" ] && [ "$setup_mode_byte" = "0" ]; then
+			return 0
+		fi
+	fi
+
+	return 1
+}
+
+function check_kdump_feasibility()
+{
+	if is_secure_boot_enforced; then
+		echo "Secure Boot is Enabled. Kdump service can't be started. Disable Secure Boot and retry"
+		return 1;
+	fi
+}
+
 function start()
 {
    check_config
@@ -517,6 +554,12 @@ function start()
    	return 1
    fi

+	check_kdump_feasibility
+	if [ $? -ne 0 ]; then
+		echo "Starting kdump: [FAILED]"
+		return 1
+	fi
+
    status 
    rc=$?
    if [ $rc == 2 ]; then

[PATCH v2 2/2] kdumpctl: status function cleanup

[Dave Young]

Move the code to check /sys/kernel/kexec_crash_loaded to function
check_kdump_feasibility(). And rename status() to check_current_kdump_status()
so these two functions become clearer.

Tested with kernel without CONFIG_KEXEC
Tested with run kdumpctl start two times.

Signed-off-by: Dave Young 
---
 kdumpctl | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/kdumpctl b/kdumpctl
index aef3875..925a3b0 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -384,12 +384,8 @@ function propagate_ssh_key()
    	
 }

-function status()
+function check_current_kdump_status()
 {
-	if [ ! -e /sys/kernel/kexec_crash_loaded ]
-	then
-		return 2 
-	fi
    rc=`cat /sys/kernel/kexec_crash_loaded`
    if [ $rc == 1 ]; then
    	return 0
@@ -535,6 +531,11 @@ function check_kdump_feasibility()
    	echo "Secure Boot is Enabled. Kdump service can't be started. Disable Secure Boot and retry"
    	return 1;
    fi
+
+	if [ ! -e /sys/kernel/kexec_crash_loaded ]; then
+		echo "Kdump is not supported on this kernel"
+		return 1
+	fi
 }

function start()
@@ -560,16 +561,10 @@ function start()
    	return 1
    fi

-	status 
-	rc=$?
-	if [ $rc == 2 ]; then
-		echo "Kdump is not supported on this kernel: [WARNING]"
-		return 1;
-	else
-		if [ $rc == 0 ]; then
-			echo "Kdump already running: [WARNING]"
-			return 0
-		fi
+	check_current_kdump_status
+	if [ $? == 0 ]; then
+		echo "Kdump already running: [WARNING]"
+		return 0
    fi

if check_ssh_config; then

[PATCH v2 2/2] kdumpctl: status function cleanup

[Dave Young]

Hi,

When I recheck the code I found that I missed the kdumpctl status code patch
Will resend the 2/2 soon.

On 02/12/14 at 10:31am, Dave Young wrote:
> Move the code to check /sys/kernel/kexec_crash_loaded to function
> check_kdump_feasibility(). And rename status() to check_current_kdump_status()
> so these two functions become clearer.
>
> Tested with kernel without CONFIG_KEXEC
> Tested with run kdumpctl start two times.
> Signed-off-by: Dave Young
>
> kdumpctl | 25 ++++++++++---------------
>  1 file changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/kdumpctl b/kdumpctl
> index aef3875..925a3b0 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -384,12 +384,8 @@ function propagate_ssh_key()
>    	
>  }
>
> -function status()
> +function check_current_kdump_status()
>  {
> if [ ! -e /sys/kernel/kexec_crash_loaded ]
> then
> return 2 
> fi
> rc=`cat /sys/kernel/kexec_crash_loaded`
> if [ $rc == 1 ]; then
> return 0
>
> @@ -535,6 +531,11 @@ function check_kdump_feasibility()
>    	echo "Secure Boot is Enabled. Kdump service can't be started. Disable Secure Boot and retry"
>    	return 1;
>    fi
> if [ ! -e /sys/kernel/kexec_crash_loaded ]; then
> echo "Kdump is not supported on this kernel"
> return 1
> fi
>
> }
>
> function start()
> @@ -560,16 +561,10 @@ function start()
>    	return 1
>    fi
> status
> rc=$?
> if [ $rc == 2 ]; then
> echo "Kdump is not supported on this kernel: [WARNING]"
> return 1;
> else
> if [ $rc == 0 ]; then
> 	echo "Kdump already running: [WARNING]"
> 	return 0
> fi
>
> check_current_kdump_status
>
> if [ $? == 0 ]; then
> echo "Kdump already running: [WARNING]"
> return 0
>
> fi
>
> if check_ssh_config; then
>
> -- 
> 1.8.3.1

[PATCH v2 2/2 update] kdumpctl: status function cleanup

[Dave Young]

Move the code to check /sys/kernel/kexec_crash_loaded to function
check_kdump_feasibility(). And rename status() to check_current_kdump_status()
so these two functions become clearer.

cleanup kdumpctl status path as well.

Tested with kernel without CONFIG_KEXEC
Tested with run kdumpctl start two times.

Signed-off-by: Dave Young 
---
 kdumpctl |   31 +++++++++++--------------------
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/kdumpctl b/kdumpctl
index aef3875..f72020d 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -384,12 +384,8 @@ function propagate_ssh_key()
    	
 }

-function status()
+function check_current_kdump_status()
 {
-	if [ ! -e /sys/kernel/kexec_crash_loaded ]
-	then
-		return 2 
-	fi
    rc=`cat /sys/kernel/kexec_crash_loaded`
    if [ $rc == 1 ]; then
    	return 0
@@ -535,6 +531,11 @@ function check_kdump_feasibility()
    	echo "Secure Boot is Enabled. Kdump service can't be started. Disable Secure Boot and retry"
    	return 1;
    fi
+
+	if [ ! -e /sys/kernel/kexec_crash_loaded ]; then
+		echo "Kdump is not supported on this kernel"
+		return 1
+	fi
 }

function start()
@@ -560,16 +561,10 @@ function start()
    	return 1
    fi

-	status 
-	rc=$?
-	if [ $rc == 2 ]; then
-		echo "Kdump is not supported on this kernel: [WARNING]"
-		return 1;
-	else
-		if [ $rc == 0 ]; then
-			echo "Kdump already running: [WARNING]"
-			return 0
-		fi
+	check_current_kdump_status
+	if [ $? == 0 ]; then
+		echo "Kdump already running: [WARNING]"
+		return 0
    fi

if check_ssh_config; then
@@ -628,7 +623,7 @@ main ()
    	;;
      status)
    	EXIT_CODE=0
-		status
+		check_current_kdump_status
    	case "$?" in
    	  0)
    		echo "Kdump is operational"
@@ -638,10 +633,6 @@ main ()
    		echo "Kdump is not operational"
    		EXIT_CODE=3
    		;;
-		  2)
-			echo "Kdump is unsupported on this kernel"
-			EXIT_CODE=3
-			;;
    	esac
    	exit $EXIT_CODE
    	;;

[PATCH v2 2/2 update] kdumpctl: status function cleanup

[Vivek Goyal]

On Thu, Feb 13, 2014 at 11:23:58AM +0800, Dave Young wrote:

[..]
>  status)
> EXIT_CODE=0
> status
> check_current_kdump_status
> case "$?" in
>  0)
> 	echo "Kdump is operational"
>
> @@ -638,10 +633,6 @@ main ()
>    		echo "Kdump is not operational"
>    		EXIT_CODE=3
>    		;;
>   2)
> 	echo "Kdump is unsupported on this kernel"
> 	EXIT_CODE=3
> 	;;

Dave this change looks good. What is EXIT_CODE=3? Who parses it and
what's the significance of value 3?

Thanks
Vivek

[PATCH v2 2/2 update] kdumpctl: status function cleanup

[Dave Young]

On 02/13/14 at 08:58am, Vivek Goyal wrote:
> On Thu, Feb 13, 2014 at 11:23:58AM +0800, Dave Young wrote:
>
> [..]
> >  status)
> > EXIT_CODE=0
> > status
> > check_current_kdump_status
> > case "$?" in
> >  0)
> > 	echo "Kdump is operational"
> >
> > @@ -638,10 +633,6 @@ main ()
> >    		echo "Kdump is not operational"
> >    		EXIT_CODE=3
> >    		;;
> >   2)
> > 	echo "Kdump is unsupported on this kernel"
> > 	EXIT_CODE=3
> > 	;;
>
> Dave this change looks good. What is EXIT_CODE=3? Who parses it and
> what's the significance of value 3?

Vivek, I have no idea about the EXIT_CODE, it probably a historic code..

Thanks
Dave

[PATCH v2 2/2 update] kdumpctl: status function cleanup

[Vivek Goyal]

On Fri, Feb 14, 2014 at 10:28:51AM +0800, Dave Young wrote:
> On 02/13/14 at 08:58am, Vivek Goyal wrote:
> > On Thu, Feb 13, 2014 at 11:23:58AM +0800, Dave Young wrote:
> >
> > [..]
> > >  status)
> > > EXIT_CODE=0
> > > status
> > > check_current_kdump_status
> > > case "$?" in
> > >  0)
> > > 	echo "Kdump is operational"
> > >
> > > @@ -638,10 +633,6 @@ main ()
> > >    		echo "Kdump is not operational"
> > >    		EXIT_CODE=3
> > >    		;;
> > >   2)
> > > 	echo "Kdump is unsupported on this kernel"
> > > 	EXIT_CODE=3
> > > 	;;
> >
> > Dave this change looks good. What is EXIT_CODE=3? Who parses it and
> > what's the significance of value 3?
>
> Vivek, I have no idea about the EXIT_CODE, it probably a historic code..

Ok, following commit introduced it.

commit 10c91a1493d152da608c0e5fdc0ced79113229d5
Author: Neil Horman 
Date:   Wed Jul 6 15:25:34 2011 -0400

Removing sysvinit files

I think this is a vestige of old init system. I have tried to read and
nowhere I can find if exit code 3 has special meaning.

I suspect that if we return 0 for success and 1 for failure, things
would be just fine.

If you like, post a patch for cleanup in Fedora. We can keep it in fedora
for some time and see if something breaks. I don't expect anything to
break though.

Thanks
Vivek

[Vivek Goyal]

On Wed, Feb 12, 2014 at 10:31:40AM +0800, Dave Young wrote:
> Changes from v1:
> Use the error message from Vivek.
> echo detail error in check_kdump_feasibility
>
> Dave Young (2):
>   kdumpctl: claim that kdump does not support secure boot when service
>     start
>   kdumpctl: status function cleanup
>
> kdumpctl | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++--------------

This patch series looks good to me.

Acked-by: Vivek Goyal

Thanks for fixing this Dave.

Vivek