All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH] testcases: fix file path to control access to cron
@ 2018-03-21  2:35 Dan Rue
  2018-03-21 15:13 ` Petr Vorel
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Dan Rue @ 2018-03-21  2:35 UTC (permalink / raw)
  To: ltp

From: Lucas Magasweran <lucas.magasweran@ieee.org>

crontab uses /etc/cron.{allow,deny} to control access on most distributions, as
well as RHEL. To maintain backwards compatibility with older still supported
distributions, the test setup will search the man page for the correct paths
(e.g. /var/spool/cron/allow). If the man page is not available (e.g. on an
embedded device), it will default to the /etc paths. This method supports
vixie-cron, cronie, etc...

The repeating code has been moved into a common shell script library to
simplify maintenance.

Reported-by: Myungho Jung <mhjungk@gmail.com>
Signed-off-by: Lucas Magasweran <lucas.magasweran@ieee.org>
Tested-by: Dan Rue <drue@therub.org>
---
 testcases/commands/cron/cron_allow01      | 12 +-----------
 testcases/commands/cron/cron_common.sh    | 15 +++++++++++++++
 testcases/commands/cron/cron_deny01       | 14 +-------------
 testcases/commands/cron/cron_neg_tests.sh |  2 +-
 testcases/commands/cron/cron_pos_tests.sh | 13 +------------
 5 files changed, 19 insertions(+), 37 deletions(-)
 create mode 100755 testcases/commands/cron/cron_common.sh

diff --git a/testcases/commands/cron/cron_allow01 b/testcases/commands/cron/cron_allow01
index 9a5e4d2406..7e05f7b7dd 100755
--- a/testcases/commands/cron/cron_allow01
+++ b/testcases/commands/cron/cron_allow01
@@ -26,17 +26,7 @@
 
 echo "This script contains bashism that needs to be fixed!"
 
-iam=`whoami`
-
-tvar=${MACHTYPE%-*}
-tvar=${tvar#*-}
-
-if [ "$tvar" = "redhat" -o "$tvar" = "redhat-linux" ]
-then
-CRON_ALLOW="/etc/cron.allow"
-else
-CRON_ALLOW="/var/spool/cron/allow"
-fi
+. cron_common.sh
 
 TEST_USER1="ca_user1"
 TEST_USER1_HOME="/home/$TEST_USER1"
diff --git a/testcases/commands/cron/cron_common.sh b/testcases/commands/cron/cron_common.sh
new file mode 100755
index 0000000000..5ed90c4039
--- /dev/null
+++ b/testcases/commands/cron/cron_common.sh
@@ -0,0 +1,15 @@
+#!/bin/sh
+
+iam=`whoami`
+
+tvar=${MACHTYPE%-*}
+tvar=${tvar#*-}
+
+setup_path() {
+	# Support paths used by older distributions of RHEL or SLES.
+	export CRON_DENY="$(man crontab 2>/dev/null | grep -m 1 -o '/[\/a-z.]*deny$' || echo "/etc/cron.deny")"
+	export CRON_ALLOW="$(man crontab 2>/dev/null | grep -m 1 -o '/[\/a-z.]*allow$' || echo "/etc/cron.allow")"
+}
+
+setup_path
+
diff --git a/testcases/commands/cron/cron_deny01 b/testcases/commands/cron/cron_deny01
index 9d32039251..263a7829b5 100755
--- a/testcases/commands/cron/cron_deny01
+++ b/testcases/commands/cron/cron_deny01
@@ -26,19 +26,7 @@
 
 echo "This script contains bashism that needs to be fixed!"
 
-iam=`whoami`
-
-tvar=${MACHTYPE%-*}
-tvar=${tvar#*-}
-
-if [ "$tvar" = "redhat" -o "$tvar" = "redhat-linux" ]
-then
-CRON_DENY="/etc/cron.deny"
-CRON_ALLOW="/etc/cron.allow"
-else
-CRON_DENY="/var/spool/cron/deny"
-CRON_ALLOW="/var/spool/cron/allow"
-fi
+. cron_common.sh
 
 TEST_USER1="cd_user1"
 TEST_USER1_HOME="/home/$TEST_USER1"
diff --git a/testcases/commands/cron/cron_neg_tests.sh b/testcases/commands/cron/cron_neg_tests.sh
index 9c3d6f6c7c..49c5c4e5d0 100755
--- a/testcases/commands/cron/cron_neg_tests.sh
+++ b/testcases/commands/cron/cron_neg_tests.sh
@@ -9,7 +9,7 @@
 #    12/03/04  Marty Ridgeway Pull RHEl4 tests out from script
 ########################################################
 
-iam=`whoami`
+. cron_common.sh
 
 if [ $iam = "root" ]; then
 	if [ $# -lt 1 ] ; then
diff --git a/testcases/commands/cron/cron_pos_tests.sh b/testcases/commands/cron/cron_pos_tests.sh
index ece114c84c..a0ddaea57b 100755
--- a/testcases/commands/cron/cron_pos_tests.sh
+++ b/testcases/commands/cron/cron_pos_tests.sh
@@ -2,18 +2,7 @@
 
 # Positive tests for cron, that means these tests have to pass
 
-iam=`whoami`
-
-tvar=${MACHTYPE%-*}
-tvar=${tvar#*-}
-
-if [ "$tvar" = "redhat" -o "$tvar" = "redhat-linux" ]
-then
-	CRON_ALLOW="/etc/cron.allow"
-else
-	CRON_ALLOW="/var/spool/cron/allow"
-fi
-
+. cron_common.sh
 
 if [ $iam = "root" ]; then
 	if [ $# -lt 1 ] ; then

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [LTP] [PATCH] testcases: fix file path to control access to cron
  2018-03-21  2:35 [LTP] [PATCH] testcases: fix file path to control access to cron Dan Rue
@ 2018-03-21 15:13 ` Petr Vorel
  2018-03-21 16:31 ` Cyril Hrubis
  2018-03-27 13:33 ` Petr Vorel
  2 siblings, 0 replies; 4+ messages in thread
From: Petr Vorel @ 2018-03-21 15:13 UTC (permalink / raw)
  To: ltp

Hi Dan,

thanks for your patch.
Cron scripts would deserver rewriting to use new shell API (+ cleanup).

> From: Lucas Magasweran <lucas.magasweran@ieee.org>

> crontab uses /etc/cron.{allow,deny} to control access on most distributions, as
> well as RHEL. To maintain backwards compatibility with older still supported
> distributions, the test setup will search the man page for the correct paths
> (e.g. /var/spool/cron/allow). If the man page is not available (e.g. on an
Do you know which distros and cron versions are really affected?
Is it RHEL 4? SLES 10 SP4 uses vixie cron 4.1 with files in /etc.
I wonder if we need still to support old path.

> embedded device), it will default to the /etc paths. This method supports
> vixie-cron, cronie, etc...
BTW (looking at busybox source) busybox crond does not support allow/deny, but that's
another issue.

> The repeating code has been moved into a common shell script library to
> simplify maintenance.

> Reported-by: Myungho Jung <mhjungk@gmail.com>
> Signed-off-by: Lucas Magasweran <lucas.magasweran@ieee.org>
> Tested-by: Dan Rue <drue@therub.org>
> ---
>  testcases/commands/cron/cron_allow01      | 12 +-----------
>  testcases/commands/cron/cron_common.sh    | 15 +++++++++++++++
>  testcases/commands/cron/cron_deny01       | 14 +-------------
>  testcases/commands/cron/cron_neg_tests.sh |  2 +-
>  testcases/commands/cron/cron_pos_tests.sh | 13 +------------
>  5 files changed, 19 insertions(+), 37 deletions(-)
>  create mode 100755 testcases/commands/cron/cron_common.sh

> diff --git a/testcases/commands/cron/cron_allow01 b/testcases/commands/cron/cron_allow01
> index 9a5e4d2406..7e05f7b7dd 100755
> --- a/testcases/commands/cron/cron_allow01
> +++ b/testcases/commands/cron/cron_allow01
> @@ -26,17 +26,7 @@

>  echo "This script contains bashism that needs to be fixed!"

> -iam=`whoami`
> -
> -tvar=${MACHTYPE%-*}
> -tvar=${tvar#*-}

> -
> -if [ "$tvar" = "redhat" -o "$tvar" = "redhat-linux" ]
> -then
> -CRON_ALLOW="/etc/cron.allow"
> -else
> -CRON_ALLOW="/var/spool/cron/allow"
> -fi
> +. cron_common.sh

>  TEST_USER1="ca_user1"
>  TEST_USER1_HOME="/home/$TEST_USER1"
> diff --git a/testcases/commands/cron/cron_common.sh b/testcases/commands/cron/cron_common.sh
> new file mode 100755
> index 0000000000..5ed90c4039
> --- /dev/null
> +++ b/testcases/commands/cron/cron_common.sh
> @@ -0,0 +1,15 @@
> +#!/bin/sh
> +
> +iam=`whoami`
> +
> +tvar=${MACHTYPE%-*}
> +tvar=${tvar#*-}
Can you please removed tvar? It's not needed any more and MACHTYPE is a bashism.
This will remove most of bashisms (the only left will be function keyword in
cron_pos_tests.sh).
> +
> +setup_path() {
> +	# Support paths used by older distributions of RHEL or SLES.
> +	export CRON_DENY="$(man crontab 2>/dev/null | grep -m 1 -o '/[\/a-z.]*deny$' || echo "/etc/cron.deny")"
> +	export CRON_ALLOW="$(man crontab 2>/dev/null | grep -m 1 -o '/[\/a-z.]*allow$' || echo "/etc/cron.allow")"
This has wrong quotation marks (although it's working. Better would be:
export CRON_DENY="$(man crontab 2>/dev/null | grep -m 1 -o '/[\/a-z.]*deny$' || echo '/etc/cron.deny')"
export CRON_ALLOW="$(man crontab 2>/dev/null | grep -m 1 -o '/[\/a-z.]*allow$' || echo '/etc/cron.allow')"

> +}
> +
> +setup_path
> +
> diff --git a/testcases/commands/cron/cron_deny01 b/testcases/commands/cron/cron_deny01
> index 9d32039251..263a7829b5 100755
> --- a/testcases/commands/cron/cron_deny01
> +++ b/testcases/commands/cron/cron_deny01
> @@ -26,19 +26,7 @@

>  echo "This script contains bashism that needs to be fixed!"

> -iam=`whoami`
> -
> -tvar=${MACHTYPE%-*}
> -tvar=${tvar#*-}
> -
> -if [ "$tvar" = "redhat" -o "$tvar" = "redhat-linux" ]
> -then
> -CRON_DENY="/etc/cron.deny"
> -CRON_ALLOW="/etc/cron.allow"
> -else
> -CRON_DENY="/var/spool/cron/deny"
> -CRON_ALLOW="/var/spool/cron/allow"
> -fi
> +. cron_common.sh

>  TEST_USER1="cd_user1"
>  TEST_USER1_HOME="/home/$TEST_USER1"
> diff --git a/testcases/commands/cron/cron_neg_tests.sh b/testcases/commands/cron/cron_neg_tests.sh
> index 9c3d6f6c7c..49c5c4e5d0 100755
> --- a/testcases/commands/cron/cron_neg_tests.sh
> +++ b/testcases/commands/cron/cron_neg_tests.sh
> @@ -9,7 +9,7 @@
>  #    12/03/04  Marty Ridgeway Pull RHEl4 tests out from script


> -iam=`whoami`
> +. cron_common.sh

>  if [ $iam = "root" ]; then
>  	if [ $# -lt 1 ] ; then
> diff --git a/testcases/commands/cron/cron_pos_tests.sh b/testcases/commands/cron/cron_pos_tests.sh
> index ece114c84c..a0ddaea57b 100755
> --- a/testcases/commands/cron/cron_pos_tests.sh
> +++ b/testcases/commands/cron/cron_pos_tests.sh
> @@ -2,18 +2,7 @@

>  # Positive tests for cron, that means these tests have to pass

> -iam=`whoami`
> -
> -tvar=${MACHTYPE%-*}
> -tvar=${tvar#*-}
> -
> -if [ "$tvar" = "redhat" -o "$tvar" = "redhat-linux" ]
> -then
> -	CRON_ALLOW="/etc/cron.allow"
> -else
> -	CRON_ALLOW="/var/spool/cron/allow"
> -fi
> -
> +. cron_common.sh

>  if [ $iam = "root" ]; then
>  	if [ $# -lt 1 ] ; then


Kind regards,
Petr

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [LTP] [PATCH] testcases: fix file path to control access to cron
  2018-03-21  2:35 [LTP] [PATCH] testcases: fix file path to control access to cron Dan Rue
  2018-03-21 15:13 ` Petr Vorel
@ 2018-03-21 16:31 ` Cyril Hrubis
  2018-03-27 13:33 ` Petr Vorel
  2 siblings, 0 replies; 4+ messages in thread
From: Cyril Hrubis @ 2018-03-21 16:31 UTC (permalink / raw)
  To: ltp

Hi!
>  TEST_USER1="ca_user1"
>  TEST_USER1_HOME="/home/$TEST_USER1"
> diff --git a/testcases/commands/cron/cron_common.sh b/testcases/commands/cron/cron_common.sh
> new file mode 100755
> index 0000000000..5ed90c4039
> --- /dev/null
> +++ b/testcases/commands/cron/cron_common.sh
> @@ -0,0 +1,15 @@
> +#!/bin/sh
> +
> +iam=`whoami`
> +
> +tvar=${MACHTYPE%-*}
> +tvar=${tvar#*-}
> +
> +setup_path() {
> +	# Support paths used by older distributions of RHEL or SLES.
> +	export CRON_DENY="$(man crontab 2>/dev/null | grep -m 1 -o '/[\/a-z.]*deny$' || echo "/etc/cron.deny")"
> +	export CRON_ALLOW="$(man crontab 2>/dev/null | grep -m 1 -o '/[\/a-z.]*allow$' || echo "/etc/cron.allow")"
> +}

FYI I do have three crontab manual pages installed on my system, two by
the cron package and one by man-pages-posix package. The 'man 1 crontab'
seems to be the one we want to parse though, it will be picked by
default since the predefined order starts with the section 1, but we may
as well specify it explicitly.

Also I will check my collection of virtual machines, with a bit of luck
all of the SLES versions that used /var/spool/cron/ are out of support
now.

-- 
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [LTP] [PATCH] testcases: fix file path to control access to cron
  2018-03-21  2:35 [LTP] [PATCH] testcases: fix file path to control access to cron Dan Rue
  2018-03-21 15:13 ` Petr Vorel
  2018-03-21 16:31 ` Cyril Hrubis
@ 2018-03-27 13:33 ` Petr Vorel
  2 siblings, 0 replies; 4+ messages in thread
From: Petr Vorel @ 2018-03-27 13:33 UTC (permalink / raw)
  To: ltp

> From: Lucas Magasweran <lucas.magasweran@ieee.org>

> crontab uses /etc/cron.{allow,deny} to control access on most distributions, as
> well as RHEL. To maintain backwards compatibility with older still supported
> distributions, the test setup will search the man page for the correct paths
> (e.g. /var/spool/cron/allow). If the man page is not available (e.g. on an
> embedded device), it will default to the /etc paths. This method supports
> vixie-cron, cronie, etc...

> The repeating code has been moved into a common shell script library to
> simplify maintenance.

> Reported-by: Myungho Jung <mhjungk@gmail.com>
> Signed-off-by: Lucas Magasweran <lucas.magasweran@ieee.org>
> Tested-by: Dan Rue <drue@therub.org>
> ---
<snip>
> -if [ "$tvar" = "redhat" -o "$tvar" = "redhat-linux" ]
> -then
> -CRON_ALLOW="/etc/cron.allow"
> -else
> -CRON_ALLOW="/var/spool/cron/allow"
> -fi
> +. cron_common.sh
We've decided to drop support for /var/spool/cron/{allow,deny} entirely.
Please change your patch.

If you wonder why we drop the support:
This old path used original Vixie-cron / ISC, cronie which is a fork of Vixie-cron version
4.1 used changed the path in 2007 [1], [2].
Original patch used for RHEL /etc/cron.{allow,deny} anyway, even SLE 9.3 used vixie-cron
with patched to the new path.
Debian changed the path in it's vixie-cron in 1999 [3]


Kind regards,
Petr

[1] https://github.com/cronie-crond/cronie/commit/7f6c24bc5aaddcc95556764fa3f801f2166e86b9
[2] https://github.com/cronie-crond/cronie/commit/9ff26cfe84d14af46dcf8af39fba9c25534df6f7
[3] https://anonscm.debian.org/cgit/pkg-cron/pkg-cron.git/commit/?id=546d93985f662362005109e2f069ebe15c0112aa

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-03-27 13:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-21  2:35 [LTP] [PATCH] testcases: fix file path to control access to cron Dan Rue
2018-03-21 15:13 ` Petr Vorel
2018-03-21 16:31 ` Cyril Hrubis
2018-03-27 13:33 ` Petr Vorel

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.