[5/6] package/checksec: new package

Message ID 20180711143113.11927-6-matthew.weber@rockwellcollins.com
State Superseded
Headers show
Series
  • Hardening Flag Bugfix/Enhancement
Related show

Commit Message

Matthew Weber July 11, 2018, 2:31 p.m.
From: Paresh Chaudhary <paresh.chaudhary@rockwellcollins.com>

This patch added host-checksec package support. This tool
provides a script to offline check the properties of a
security hardened elf file.

REF: https://github.com/slimm609/checksec.sh

Signed-off-by: Paresh Chaudhary <paresh.chaudhary@rockwellcollins.com>
Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com>
---
 package/Config.in.host                        |  1 +
 ...cksec-Fixed-issue-with-relative-path.patch | 43 +++++++++++++++++++
 package/checksec/Config.in.host               | 16 +++++++
 package/checksec/checksec.hash                |  3 ++
 package/checksec/checksec.mk                  | 16 +++++++
 5 files changed, 79 insertions(+)
 create mode 100644 package/checksec/0001-checksec-Fixed-issue-with-relative-path.patch
 create mode 100644 package/checksec/Config.in.host
 create mode 100644 package/checksec/checksec.hash
 create mode 100644 package/checksec/checksec.mk

Comments

Thomas Petazzoni Aug. 10, 2018, 8:58 p.m. | #1
Matt, Paresh,

On Wed, 11 Jul 2018 09:31:12 -0500, Matt Weber wrote:
> From: Paresh Chaudhary <paresh.chaudhary@rockwellcollins.com>
> 
> This patch added host-checksec package support. This tool

added -> adds

> diff --git a/package/checksec/0001-checksec-Fixed-issue-with-relative-path.patch b/package/checksec/0001-checksec-Fixed-issue-with-relative-path.patch
> new file mode 100644
> index 0000000000..43a882d991
> --- /dev/null
> +++ b/package/checksec/0001-checksec-Fixed-issue-with-relative-path.patch
> @@ -0,0 +1,43 @@
> +From b48a2dfae26fa3b4af8e65fb5953b3caf62c137b Mon Sep 17 00:00:00 2001
> +From: Paresh Chaudhary <paresh.chaudhary@rockwellcollins.com>
> +Date: Mon, 21 May 2018 14:34:23 -0500
> +Subject: [PATCH] checksec: Fixed issue with relative path
> +
> +Before this patch script was not able find exists directory when user pass
> +relative directory path with '--dir' or '-d' option and also we faced this error
> +when we execute script with relative path.

The english wording seems weird here, even though I'm not a native
speaker. Perhaps:

"""
Before this patch, the checksec script was not able to find existing
directories when the user passes a relative path with --dir/-d,
aborting with a "No such file or directory". The same error was
reported when the script is executed through a relative path.
"""

I'm sure Matt, as a native speaker, can come up with an even better
wording.

> diff --git a/package/checksec/Config.in.host b/package/checksec/Config.in.host
> new file mode 100644
> index 0000000000..7f86f46b50
> --- /dev/null
> +++ b/package/checksec/Config.in.host
> @@ -0,0 +1,16 @@
> +config BR2_PACKAGE_HOST_CHECKSEC
> +	bool "host checksec"
> +	help
> +	  This tool provides a shell script to check the
> +	  properties of the executables
> +	  (like PIE,RELRO,PaX,Canaries,ASLR,Fortify Source).
> +
> +	  https://github.com/slimm609/checksec.sh.git
> +
> +	  NOTE: This tool has a hard-coded path to the standard
> +	  libraries for some of the fortify test cases and
> +	  requires you to either test the local filesystem or be
> +	  in a chroot'd environment.  The tool can still be used
> +	  against a folder of files but requires discretion of
> +	  which the tests may not report consistently vs
> +	  chroot/on-target.

When I look at this and the comment from the maintainer at [0], I am
not sure about the usefulness of such a tool in the context of
Buildroot. Chrooting into the target filesystem is generally not
possible, because the target architecture is different than the build
system architecture. To me, this limitation makes the tool essentially
useless in the context of Buildroot. Could you comment on this a bit
more ?

Also, the formulation "requires discretion of which the test may not
report consistently vs chroot/on-target" doesn't make any sense to me.

[0] https://github.com/slimm609/checksec.sh/issues/62#issuecomment-389880584

> diff --git a/package/checksec/checksec.hash b/package/checksec/checksec.hash
> new file mode 100644
> index 0000000000..e3d1ffd5d1
> --- /dev/null
> +++ b/package/checksec/checksec.hash
> @@ -0,0 +1,3 @@
> +# Locally calculated
> +sha256 510b0b0528f15d0bf13fa1ae7140d2b9fc9261323c98ff76c011bef475a69c14 checksec-cdefe53eb72e6e8f23308417d2fc6b68cba9dbac.tar.gz
> +sha256 c5e2a8e188040fc34eb9362084778a2e25f8d1f888e47a2be09efa7cecd9c70d LICENSE.txt
> diff --git a/package/checksec/checksec.mk b/package/checksec/checksec.mk
> new file mode 100644
> index 0000000000..31ceb43e21
> --- /dev/null
> +++ b/package/checksec/checksec.mk
> @@ -0,0 +1,16 @@
> +################################################################################
> +#
> +# checksec
> +#
> +################################################################################
> +
> +CHECKSEC_VERSION = cdefe53eb72e6e8f23308417d2fc6b68cba9dbac
> +CHECKSEC_SITE = $(call github,slimm609,checksec.sh,$(CHECKSEC_VERSION))
> +CHECKSEC_LICENSE = BSD-3-Clause
> +CHECKSEC_LICENSE_FILES = LICENSE.txt
> +
> +define HOST_CHECKSEC_INSTALL_CMDS
> +	$(INSTALL) -D -m 0755 $(@D)/checksec $(HOST_DIR)/bin/

There should be a full destination path here.

Thomas
Matthew Weber Aug. 11, 2018, 12:57 a.m. | #2
Thomas,
On Fri, Aug 10, 2018 at 3:59 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Matt, Paresh,
>
> On Wed, 11 Jul 2018 09:31:12 -0500, Matt Weber wrote:
> > From: Paresh Chaudhary <paresh.chaudhary@rockwellcollins.com>
> >
> > This patch added host-checksec package support. This tool
>
> added -> adds

Noted.

>
> > diff --git a/package/checksec/0001-checksec-Fixed-issue-with-relative-path.patch b/package/checksec/0001-checksec-Fixed-issue-with-relative-path.patch
> > new file mode 100644
> > index 0000000000..43a882d991
> > --- /dev/null
> > +++ b/package/checksec/0001-checksec-Fixed-issue-with-relative-path.patch
> > @@ -0,0 +1,43 @@
> > +From b48a2dfae26fa3b4af8e65fb5953b3caf62c137b Mon Sep 17 00:00:00 2001
> > +From: Paresh Chaudhary <paresh.chaudhary@rockwellcollins.com>
> > +Date: Mon, 21 May 2018 14:34:23 -0500
> > +Subject: [PATCH] checksec: Fixed issue with relative path
> > +
> > +Before this patch script was not able find exists directory when user pass
> > +relative directory path with '--dir' or '-d' option and also we faced this error
> > +when we execute script with relative path.
>
> The english wording seems weird here, even though I'm not a native
> speaker. Perhaps:
>
> """
> Before this patch, the checksec script was not able to find existing
> directories when the user passes a relative path with --dir/-d,
> aborting with a "No such file or directory". The same error was
> reported when the script is executed through a relative path.
> """
>
> I'm sure Matt, as a native speaker, can come up with an even better
> wording.

Thanks, yes will rework.

>
> > diff --git a/package/checksec/Config.in.host b/package/checksec/Config.in.host
> > new file mode 100644
> > index 0000000000..7f86f46b50
> > --- /dev/null
> > +++ b/package/checksec/Config.in.host
> > @@ -0,0 +1,16 @@
> > +config BR2_PACKAGE_HOST_CHECKSEC
> > +     bool "host checksec"
> > +     help
> > +       This tool provides a shell script to check the
> > +       properties of the executables
> > +       (like PIE,RELRO,PaX,Canaries,ASLR,Fortify Source).
> > +
> > +       https://github.com/slimm609/checksec.sh.git
> > +
> > +       NOTE: This tool has a hard-coded path to the standard
> > +       libraries for some of the fortify test cases and
> > +       requires you to either test the local filesystem or be
> > +       in a chroot'd environment.  The tool can still be used
> > +       against a folder of files but requires discretion of
> > +       which the tests may not report consistently vs
> > +       chroot/on-target.
>
> When I look at this and the comment from the maintainer at [0], I am
> not sure about the usefulness of such a tool in the context of
> Buildroot. Chrooting into the target filesystem is generally not
> possible, because the target architecture is different than the build
> system architecture. To me, this limitation makes the tool essentially
> useless in the context of Buildroot. Could you comment on this a bit
> more ?

The tool tests a lot of items related to hardening and we were
originally trying to get the full set working.  In reality we only
needed the core items that show us ASLR related items.  The tool is
made up of scripts and uses readelf for the ASLR piece.  Thus it works
fine for a host (offline)target filesystem check of executable ALSR
requirements.  However, I can add a note stating what doesn't work
correctly.  There are test cases it has that use live proc information
and the system libraries, etc.

>
> Also, the formulation "requires discretion of which the test may not
> report consistently vs chroot/on-target" doesn't make any sense to me.

I can make a list do this is definitive.

>
> [0] https://github.com/slimm609/checksec.sh/issues/62#issuecomment-389880584
>
> > diff --git a/package/checksec/checksec.hash b/package/checksec/checksec.hash
> > new file mode 100644
> > index 0000000000..e3d1ffd5d1
> > --- /dev/null
> > +++ b/package/checksec/checksec.hash
> > @@ -0,0 +1,3 @@
> > +# Locally calculated
> > +sha256 510b0b0528f15d0bf13fa1ae7140d2b9fc9261323c98ff76c011bef475a69c14 checksec-cdefe53eb72e6e8f23308417d2fc6b68cba9dbac.tar.gz
> > +sha256 c5e2a8e188040fc34eb9362084778a2e25f8d1f888e47a2be09efa7cecd9c70d LICENSE.txt
> > diff --git a/package/checksec/checksec.mk b/package/checksec/checksec.mk
> > new file mode 100644
> > index 0000000000..31ceb43e21
> > --- /dev/null
> > +++ b/package/checksec/checksec.mk
> > @@ -0,0 +1,16 @@
> > +################################################################################
> > +#
> > +# checksec
> > +#
> > +################################################################################
> > +
> > +CHECKSEC_VERSION = cdefe53eb72e6e8f23308417d2fc6b68cba9dbac
> > +CHECKSEC_SITE = $(call github,slimm609,checksec.sh,$(CHECKSEC_VERSION))
> > +CHECKSEC_LICENSE = BSD-3-Clause
> > +CHECKSEC_LICENSE_FILES = LICENSE.txt
> > +
> > +define HOST_CHECKSEC_INSTALL_CMDS
> > +     $(INSTALL) -D -m 0755 $(@D)/checksec $(HOST_DIR)/bin/
>
> There should be a full destination path here.

Noted.

Thanks for the review.

Matt
Thomas Petazzoni Aug. 11, 2018, 10:30 a.m. | #3
Hello Matt,

On Fri, 10 Aug 2018 19:57:06 -0500, Matthew Weber wrote:

> > When I look at this and the comment from the maintainer at [0], I am
> > not sure about the usefulness of such a tool in the context of
> > Buildroot. Chrooting into the target filesystem is generally not
> > possible, because the target architecture is different than the build
> > system architecture. To me, this limitation makes the tool essentially
> > useless in the context of Buildroot. Could you comment on this a bit
> > more ?  
> 
> The tool tests a lot of items related to hardening and we were
> originally trying to get the full set working.  In reality we only
> needed the core items that show us ASLR related items.  The tool is
> made up of scripts and uses readelf for the ASLR piece.  Thus it works
> fine for a host (offline)target filesystem check of executable ALSR
> requirements.  However, I can add a note stating what doesn't work
> correctly.  There are test cases it has that use live proc information
> and the system libraries, etc.

Yes, something more specific than the vague explanation in the proposed
Config.in help text would be good.

> > Also, the formulation "requires discretion of which the test may not
> > report consistently vs chroot/on-target" doesn't make any sense to me.  
> 
> I can make a list do this is definitive.

OK, good.

Thanks!

Thomas

Patch

diff --git a/package/Config.in.host b/package/Config.in.host
index 7838ffc219..0c21b11bd0 100644
--- a/package/Config.in.host
+++ b/package/Config.in.host
@@ -5,6 +5,7 @@  menu "Host utilities"
 	source "package/cargo/Config.in.host"
 	source "package/cbootimage/Config.in.host"
 	source "package/checkpolicy/Config.in.host"
+	source "package/checksec/Config.in.host"
 	source "package/cmake/Config.in.host"
 	source "package/cramfs/Config.in.host"
 	source "package/cryptsetup/Config.in.host"
diff --git a/package/checksec/0001-checksec-Fixed-issue-with-relative-path.patch b/package/checksec/0001-checksec-Fixed-issue-with-relative-path.patch
new file mode 100644
index 0000000000..43a882d991
--- /dev/null
+++ b/package/checksec/0001-checksec-Fixed-issue-with-relative-path.patch
@@ -0,0 +1,43 @@ 
+From b48a2dfae26fa3b4af8e65fb5953b3caf62c137b Mon Sep 17 00:00:00 2001
+From: Paresh Chaudhary <paresh.chaudhary@rockwellcollins.com>
+Date: Mon, 21 May 2018 14:34:23 -0500
+Subject: [PATCH] checksec: Fixed issue with relative path
+
+Before this patch script was not able find exists directory when user pass
+relative directory path with '--dir' or '-d' option and also we faced this error
+when we execute script with relative path.
+
+This patch fixed  "No such file or directory" error in both cases.
+
+https://github.com/slimm609/checksec.sh/issues/54
+
+Signed-off-by: Paresh Chaudhary <paresh.chaudhary@rockwellcollins.com>
+---
+ checksec | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/checksec b/checksec
+index 24b521f..baf8d63 100755
+--- a/checksec
++++ b/checksec
+@@ -1193,7 +1193,7 @@ do
+     echo_message "RELRO           STACK CANARY      NX            PIE             RPATH      RUNPATH      FORTIFY Checked         Total   Filename\n" '' "<dir name='$tempdir'>\n" "{ \"dir\": { \"name\":\"$tempdir\" },"
+     fdircount=0
+     fdirtotal=0
+-    for N in $(find $tempdir -type f); do
++    for N in $(find . -type f); do
+       if [[ "$N" != "[A-Za-z1-0]*" ]]; then
+         out=$(file "$N")
+         if [[  $out =~ ELF ]] ; then
+@@ -1201,7 +1201,7 @@ do
+         fi
+       fi
+     done
+-    for N in $(find $tempdir -type f); do
++    for N in $(find . -type f); do
+       if [[ "$N" != "[A-Za-z1-0]*" ]]; then
+     # read permissions?
+     if [[ ! -r "$N" ]]; then
+-- 
+1.9.1
+
diff --git a/package/checksec/Config.in.host b/package/checksec/Config.in.host
new file mode 100644
index 0000000000..7f86f46b50
--- /dev/null
+++ b/package/checksec/Config.in.host
@@ -0,0 +1,16 @@ 
+config BR2_PACKAGE_HOST_CHECKSEC
+	bool "host checksec"
+	help
+	  This tool provides a shell script to check the
+	  properties of the executables
+	  (like PIE,RELRO,PaX,Canaries,ASLR,Fortify Source).
+
+	  https://github.com/slimm609/checksec.sh.git
+
+	  NOTE: This tool has a hard-coded path to the standard
+	  libraries for some of the fortify test cases and
+	  requires you to either test the local filesystem or be
+	  in a chroot'd environment.  The tool can still be used
+	  against a folder of files but requires discretion of
+	  which the tests may not report consistently vs
+	  chroot/on-target.
diff --git a/package/checksec/checksec.hash b/package/checksec/checksec.hash
new file mode 100644
index 0000000000..e3d1ffd5d1
--- /dev/null
+++ b/package/checksec/checksec.hash
@@ -0,0 +1,3 @@ 
+# Locally calculated
+sha256 510b0b0528f15d0bf13fa1ae7140d2b9fc9261323c98ff76c011bef475a69c14 checksec-cdefe53eb72e6e8f23308417d2fc6b68cba9dbac.tar.gz
+sha256 c5e2a8e188040fc34eb9362084778a2e25f8d1f888e47a2be09efa7cecd9c70d LICENSE.txt
diff --git a/package/checksec/checksec.mk b/package/checksec/checksec.mk
new file mode 100644
index 0000000000..31ceb43e21
--- /dev/null
+++ b/package/checksec/checksec.mk
@@ -0,0 +1,16 @@ 
+################################################################################
+#
+# checksec
+#
+################################################################################
+
+CHECKSEC_VERSION = cdefe53eb72e6e8f23308417d2fc6b68cba9dbac
+CHECKSEC_SITE = $(call github,slimm609,checksec.sh,$(CHECKSEC_VERSION))
+CHECKSEC_LICENSE = BSD-3-Clause
+CHECKSEC_LICENSE_FILES = LICENSE.txt
+
+define HOST_CHECKSEC_INSTALL_CMDS
+	$(INSTALL) -D -m 0755 $(@D)/checksec $(HOST_DIR)/bin/
+endef
+
+$(eval $(host-generic-package))