From 6a3f401aec68db0510ca9e1006ae29196473951c Mon Sep 17 00:00:00 2001 From: Watson Sato Date: Thu, 31 Mar 2022 12:49:24 +0200 Subject: [PATCH 1/4] Only follow symlinks when they are the provided argument Otherwise, don't follow any symlink enconutered. Following symlinks is problematic as it can alter permissions of binaries in the system: '/lib/.build-id/a4/8e8ae0d029dbbc1c1b0bb0fcea424860a6c412' -> '../../../../usr/bin/sudo' --- shared/templates/file_groupowner/ansible.template | 2 +- shared/templates/file_groupowner/bash.template | 2 +- shared/templates/file_owner/ansible.template | 2 +- shared/templates/file_owner/bash.template | 2 +- shared/templates/file_permissions/ansible.template | 2 +- shared/templates/file_permissions/bash.template | 4 ++-- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/shared/templates/file_groupowner/ansible.template b/shared/templates/file_groupowner/ansible.template index c20742475845..452f7ae26e0a 100644 --- a/shared/templates/file_groupowner/ansible.template +++ b/shared/templates/file_groupowner/ansible.template @@ -15,7 +15,7 @@ {{%- endif %}} - name: Find {{{ path }}} file(s) matching {{{ FILE_REGEX[loop.index0] }}}{{% if RECURSIVE %}} recursively{{% endif %}} - command: 'find -L {{{ path }}} {{{ FIND_RECURSE_ARGS }}} -type f ! -gid {{{ FILEGID }}} -regex "{{{ FILE_REGEX[loop.index0] }}}"' + command: 'find -H {{{ path }}} {{{ FIND_RECURSE_ARGS }}} -type f ! -gid {{{ FILEGID }}} -regex "{{{ FILE_REGEX[loop.index0] }}}"' register: files_found changed_when: False failed_when: False diff --git a/shared/templates/file_groupowner/bash.template b/shared/templates/file_groupowner/bash.template index 49b1d2f5d219..b59e5be90392 100644 --- a/shared/templates/file_groupowner/bash.template +++ b/shared/templates/file_groupowner/bash.template @@ -15,7 +15,7 @@ {{%- if FILE_REGEX %}} find {{{ path }}} {{{ FIND_RECURSE_ARGS }}} -type f ! -gid {{{ FILEGID }}} -regex '{{{ FILE_REGEX[loop.index0] }}}' -exec chgrp {{{ FILEGID }}} {} \; {{% else %}} -find -L {{{ path }}} {{{ FIND_RECURSE_ARGS }}} -type d -exec chgrp {{{ FILEGID }}} {} \; +find -H {{{ path }}} {{{ FIND_RECURSE_ARGS }}} -type d -exec chgrp {{{ FILEGID }}} {} \; {{%- endif %}} {{%- else %}} chgrp {{{ FILEGID }}} {{{ path }}} diff --git a/shared/templates/file_owner/ansible.template b/shared/templates/file_owner/ansible.template index a7d1727a0452..28addf259f90 100644 --- a/shared/templates/file_owner/ansible.template +++ b/shared/templates/file_owner/ansible.template @@ -15,7 +15,7 @@ {{%- endif %}} - name: Find {{{ path }}} file(s) matching {{{ FILE_REGEX[loop.index0] }}}{{% if RECURSIVE %}} recursively{{% endif %}} - command: 'find -L {{{ path }}} {{{ FIND_RECURSE_ARGS }}} -type f ! -uid {{{ FILEUID }}} -regex "{{{ FILE_REGEX[loop.index0] }}}"' + command: 'find -H {{{ path }}} {{{ FIND_RECURSE_ARGS }}} -type f ! -uid {{{ FILEUID }}} -regex "{{{ FILE_REGEX[loop.index0] }}}"' register: files_found changed_when: False failed_when: False diff --git a/shared/templates/file_owner/bash.template b/shared/templates/file_owner/bash.template index 9552e7c1f10a..457fa503c749 100644 --- a/shared/templates/file_owner/bash.template +++ b/shared/templates/file_owner/bash.template @@ -15,7 +15,7 @@ {{%- if FILE_REGEX %}} find {{{ path }}} {{{ FIND_RECURSE_ARGS }}} -type f ! -uid {{{ FILEUID }}} -regex '{{{ FILE_REGEX[loop.index0] }}}' -exec chown {{{ FILEUID }}} {} \; {{%- else %}} -find -L {{{ path }}} {{{ FIND_RECURSE_ARGS }}} -type d -exec chown {{{ FILEUID }}} {} \; +find -H {{{ path }}} {{{ FIND_RECURSE_ARGS }}} -type d -exec chown {{{ FILEUID }}} {} \; {{%- endif %}} {{%- else %}} chown {{{ FILEUID }}} {{{ path }}} diff --git a/shared/templates/file_permissions/ansible.template b/shared/templates/file_permissions/ansible.template index 977dd5fdf6f4..071748d1ceb0 100644 --- a/shared/templates/file_permissions/ansible.template +++ b/shared/templates/file_permissions/ansible.template @@ -21,7 +21,7 @@ {{%- endif %}} - name: Find {{{ path }}} file(s){{% if RECURSIVE %}} recursively{{% endif %}} - command: 'find -L {{{ path }}} {{{ FIND_RECURSE_ARGS }}} {{{ PERMS }}} -type f -regex "{{{ FILE_REGEX[loop.index0] }}}"' + command: 'find -H {{{ path }}} {{{ FIND_RECURSE_ARGS }}} {{{ PERMS }}} -type f -regex "{{{ FILE_REGEX[loop.index0] }}}"' register: files_found changed_when: False failed_when: False diff --git a/shared/templates/file_permissions/bash.template b/shared/templates/file_permissions/bash.template index 34a62c644646..3cf45f086f65 100644 --- a/shared/templates/file_permissions/bash.template +++ b/shared/templates/file_permissions/bash.template @@ -19,9 +19,9 @@ {{% for path in FILEPATH %}} {{%- if IS_DIRECTORY %}} {{%- if FILE_REGEX %}} -find -L {{{ path }}} {{{ FIND_RECURSE_ARGS }}} {{{ PERMS }}} -type f -regex '{{{ FILE_REGEX[loop.index0] }}}' -exec chmod {{{ FILEMODE }}} {} \; +find -H {{{ path }}} {{{ FIND_RECURSE_ARGS }}} {{{ PERMS }}} -type f -regex '{{{ FILE_REGEX[loop.index0] }}}' -exec chmod {{{ FILEMODE }}} {} \; {{%- else %}} -find -L {{{ path }}} {{{ FIND_RECURSE_ARGS }}} {{{ PERMS }}} -type d -exec chmod {{{ FILEMODE }}} {} \; +find -H {{{ path }}} {{{ FIND_RECURSE_ARGS }}} {{{ PERMS }}} -type d -exec chmod {{{ FILEMODE }}} {} \; {{%- endif %}} {{%- else %}} chmod {{{ FILEMODE }}} {{{ path }}} From 2ec8f086f18c4e10ed8502c2f531edfb51efdaab Mon Sep 17 00:00:00 2001 From: Watson Sato Date: Thu, 31 Mar 2022 12:51:52 +0200 Subject: [PATCH 2/4] Add tests to check/ensure symlink behavior of OVAl Add various tests that setup symlinks to files and directories with correct and incorrect permissions. As noted in 6a3f401aec68db0510ca9e1006ae29196473951c, the check should not follow symlinks. These tests define the expected behavior of the check, witch is to not follow symlinks nor report symlinks as collected objects. --- .../dir_symlink_incorrect_dir_perm.pass.sh | 19 +++++++++++++++++++ .../dir_symlink_incorrect_file_perm.pass.sh | 19 +++++++++++++++++++ ...file_symlink_incorrect_target_perm.pass.sh | 15 +++++++++++++++ 3 files changed, 53 insertions(+) create mode 100644 linux_os/guide/system/permissions/files/permissions_within_important_dirs/file_permissions_library_dirs/tests/dir_symlink_incorrect_dir_perm.pass.sh create mode 100644 linux_os/guide/system/permissions/files/permissions_within_important_dirs/file_permissions_library_dirs/tests/dir_symlink_incorrect_file_perm.pass.sh create mode 100644 linux_os/guide/system/permissions/files/permissions_within_important_dirs/file_permissions_library_dirs/tests/file_symlink_incorrect_target_perm.pass.sh diff --git a/linux_os/guide/system/permissions/files/permissions_within_important_dirs/file_permissions_library_dirs/tests/dir_symlink_incorrect_dir_perm.pass.sh b/linux_os/guide/system/permissions/files/permissions_within_important_dirs/file_permissions_library_dirs/tests/dir_symlink_incorrect_dir_perm.pass.sh new file mode 100644 index 000000000000..1c40409bdb9d --- /dev/null +++ b/linux_os/guide/system/permissions/files/permissions_within_important_dirs/file_permissions_library_dirs/tests/dir_symlink_incorrect_dir_perm.pass.sh @@ -0,0 +1,19 @@ +#!/bin/bash + +useradd user_test +TESTDIR="/usr/lib/" + +# Ensure everything is all right +chmod -R u-s,g-ws,o-wt /lib /lib64 /usr/lib /usr/lib64 + +# Let's setup a symlink to a directory,whose permissions are incompliant + +# Directory with incorrect perms +mkdir /home/user_test/directory +chmod 0766 /home/user_test/directory + +# File with correct perms +touch /home/user_test/directory/test_file +chmod 0755 /home/user_test/directory/test_file + +ln -s /home/user_test $TESTDIR/user_test_home diff --git a/linux_os/guide/system/permissions/files/permissions_within_important_dirs/file_permissions_library_dirs/tests/dir_symlink_incorrect_file_perm.pass.sh b/linux_os/guide/system/permissions/files/permissions_within_important_dirs/file_permissions_library_dirs/tests/dir_symlink_incorrect_file_perm.pass.sh new file mode 100644 index 000000000000..64c0256e90ef --- /dev/null +++ b/linux_os/guide/system/permissions/files/permissions_within_important_dirs/file_permissions_library_dirs/tests/dir_symlink_incorrect_file_perm.pass.sh @@ -0,0 +1,19 @@ +#!/bin/bash + +useradd user_test +TESTDIR="/usr/lib/" + +# Ensure everything is all right +chmod -R u-s,g-ws,o-wt /lib /lib64 /usr/lib /usr/lib64 + +# Let's setup a symlink to a directory that contains an incomplient file + +# Directory with correct perms +mkdir /home/user_test/directory +chmod 0755 /home/user_test/directory + +# File with incorrect perms +touch /home/user_test/directory/test_file +chmod 0766 /home/user_test/directory/test_file + +ln -s /home/user_test $TESTDIR/user_test_home diff --git a/linux_os/guide/system/permissions/files/permissions_within_important_dirs/file_permissions_library_dirs/tests/file_symlink_incorrect_target_perm.pass.sh b/linux_os/guide/system/permissions/files/permissions_within_important_dirs/file_permissions_library_dirs/tests/file_symlink_incorrect_target_perm.pass.sh new file mode 100644 index 000000000000..423725c744e8 --- /dev/null +++ b/linux_os/guide/system/permissions/files/permissions_within_important_dirs/file_permissions_library_dirs/tests/file_symlink_incorrect_target_perm.pass.sh @@ -0,0 +1,15 @@ +#!/bin/bash + +useradd user_test +TESTDIR="/usr/lib/" + +# Ensure everything is all right +chmod -R u-s,g-ws,o-wt /lib /lib64 /usr/lib /usr/lib64 + +# Let's setup a symlink to a file, whose permissions are incompliant + +# File with incorrect perms +touch /home/user_test/test_file +chmod 0766 /home/user_test/test_file + +ln -s /home/user_test/test_file $TESTDIR/user_test_home From 6df9c35235c70707d2a8d7105a4f57c2485c690e Mon Sep 17 00:00:00 2001 From: Watson Sato Date: Thu, 31 Mar 2022 12:56:02 +0200 Subject: [PATCH 3/4] Use behaviors to recurse down the directories Recursing a directory using pattern match works, but this approach makes OVAL overlook that some paths are symlinks. So, although the check was not reporting symlinks, it was following them down. The OVAL was following symlinks to directories and collecting files in them, as evidenced by fails in test scenario dir_symlink_incorrect_file.pass.sh, which was failing. The way to recurse down a directory and avoid symlinks is by defining a behavior. --- shared/templates/file_groupowner/oval.template | 5 ++--- shared/templates/file_owner/oval.template | 5 ++--- shared/templates/file_permissions/oval.template | 5 ++--- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/shared/templates/file_groupowner/oval.template b/shared/templates/file_groupowner/oval.template index 5c1a8bfdf9a1..0a43e1108052 100644 --- a/shared/templates/file_groupowner/oval.template +++ b/shared/templates/file_groupowner/oval.template @@ -22,10 +22,9 @@ {{%- if IS_DIRECTORY -%}} {{%- if RECURSIVE %}} - ^{{{ filepath[:-1] }}} - {{%- else %}} - {{{ filepath[:-1] }}} + {{%- endif %}} + {{{ filepath[:-1] }}} {{%- if FILE_REGEX %}} {{{ FILE_REGEX[loop.index0] }}} {{%- else %}} diff --git a/shared/templates/file_owner/oval.template b/shared/templates/file_owner/oval.template index 82308fbd4d4a..f747762d0737 100644 --- a/shared/templates/file_owner/oval.template +++ b/shared/templates/file_owner/oval.template @@ -22,10 +22,9 @@ {{%- if IS_DIRECTORY -%}} {{%- if RECURSIVE %}} - ^{{{ filepath[:-1] }}} - {{%- else %}} - {{{ filepath[:-1] }}} + {{%- endif %}} + {{{ filepath[:-1] }}} {{%- if FILE_REGEX %}} {{{ FILE_REGEX[loop.index0] }}} {{%- else %}} diff --git a/shared/templates/file_permissions/oval.template b/shared/templates/file_permissions/oval.template index f314b5757b9e..ec44e7eacc24 100644 --- a/shared/templates/file_permissions/oval.template +++ b/shared/templates/file_permissions/oval.template @@ -26,10 +26,9 @@ {{%- if IS_DIRECTORY %}} {{%- if RECURSIVE %}} - ^{{{ filepath[:-1] }}} - {{%- else %}} - {{{ filepath[:-1] }}} + {{%- endif %}} + {{{ filepath[:-1] }}} {{%- if FILE_REGEX %}} {{{ FILE_REGEX[loop.index0] }}} {{%- else %}} From 50bfd55aab31e10c809e86b56151c8f6c04cb1cf Mon Sep 17 00:00:00 2001 From: Watson Sato Date: Thu, 31 Mar 2022 13:35:48 +0200 Subject: [PATCH 4/4] Fix simple typo in test scenario --- .../file_ownership_library_dirs/tests/incorrect_symlink.pass.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux_os/guide/system/permissions/files/permissions_within_important_dirs/file_ownership_library_dirs/tests/incorrect_symlink.pass.sh b/linux_os/guide/system/permissions/files/permissions_within_important_dirs/file_ownership_library_dirs/tests/incorrect_symlink.pass.sh index 51bc6fe2d717..ed21ba999e7d 100644 --- a/linux_os/guide/system/permissions/files/permissions_within_important_dirs/file_ownership_library_dirs/tests/incorrect_symlink.pass.sh +++ b/linux_os/guide/system/permissions/files/permissions_within_important_dirs/file_ownership_library_dirs/tests/incorrect_symlink.pass.sh @@ -5,5 +5,5 @@ useradd user_test TESTDIR="/usr/lib/" # The check ignores this symlink and results in pass -ln -s $TESTDIR/mising_test_file $TESTDIR/faulty_symlink +ln -s $TESTDIR/missing_test_file $TESTDIR/faulty_symlink chown -h user_test $TESTDIR/faulty_symlink