Re: /etc/rc.d/rc.subr; prefix ${pexp} with script interpretor path

2013-09-21 Thread Craig R. Skinner 
As the others here with brains have had a chance to sleep on this,
what's the current thinking?

As I understand it, there are 2 decisions to make:

1) How to decide if a $daemon is a script as opposed to a binary
(*) file(1)
(*) dd(d)
(*) sed(1)
  Could stat(1) be tasked to switch case on file attributes (e.g: size)?

2) Whether to check if a script's interpreter is valid

http://openbsd.7691.n7.nabble.com/etc-rc-d-rc-subr-prefix-pexp-with-script-interpretor-path-td234439.html

Yes/No/Other?
-- 
Craig Skinner | http://twitter.com/Craig_Skinner | http://linkd.in/yGqkv7

Re: /etc/rc.d/rc.subr; prefix ${pexp} with script interpretor path

2013-09-21 Thread Stuart Henderson 
On 2013/09/21 13:12, Craig R. Skinner wrote:
 As the others here with brains have had a chance to sleep on this,
 what's the current thinking?
 
 As I understand it, there are 2 decisions to make:
 
 1) How to decide if a $daemon is a script as opposed to a binary
   (*) file(1)

too big, too complex. file(1) has to parse a 500K /etc/magic each
time it runs, which would be 15+ times for this, and uses about 8MB
ram. definitely overkill for identifying the contents of the first
two characters of a file. remember this has to run on a wide range of
platforms.

   (*) dd(d)
   (*) sed(1)
   Could stat(1) be tasked to switch case on file attributes (e.g: size)?

how would size help?

 
 2) Whether to check if a script's interpreter is valid
 
 http://openbsd.7691.n7.nabble.com/etc-rc-d-rc-subr-prefix-pexp-with-script-interpretor-path-td234439.html
 
 Yes/No/Other?
 -- 
 Craig Skinner | http://twitter.com/Craig_Skinner | http://linkd.in/yGqkv7
 

How much does this really help anyway? Porters still need to check
the actual string used (for example, it's quite common for perl things
to use perl: programnanme), and it doesn't take much extra typing
to add a pexp line to the script in cases where it's needed.

I am rather wary of adding complication to rc.subr, iirc one of the
conditions when the subsystem was first added was that it remained
simple..

Re: /etc/rc.d/rc.subr; prefix ${pexp} with script interpretor path

2013-09-21 Thread Mark Kettenis 
 Date: Sat, 21 Sep 2013 14:37:21 +0100
 From: Stuart Henderson st...@openbsd.org
 
 How much does this really help anyway? Porters still need to check
 the actual string used (for example, it's quite common for perl things
 to use perl: programnanme), and it doesn't take much extra typing
 to add a pexp line to the script in cases where it's needed.
 
 I am rather wary of adding complication to rc.subr, iirc one of the
 conditions when the subsystem was first added was that it remained
 simple..

+1

Re: /etc/rc.d/rc.subr; prefix ${pexp} with script interpretor path

2013-09-17 Thread Craig R. Skinner 
On 2013-09-16 Mon 23:28 PM |, Alexander Hall wrote:
 
 sed can do it all. Really.

This is getting beyond me Alexander.

Is sed a mechanism to step away from using file(1) ?

 Notes:
 
 - I separate re_quote() cause I think it can be useful in other places.
 - I think re_quote() is (basic) regex complete.
 - I don't care if the interpreter is (or seems) nonexistant, as that
   shouldn't be a runtime error.
 - I'm sure sed may die horribly if you try to feed it a 9GB oneline
   file. However, if so, it should not produce any output anyway. ;)
   If this would ever be considered a real problem, dd(1) would help
   (as espie already mentioned).
 
   re_quote() { sed 's/\([]^$*.\\[]\)/\\\1/g'; }
 
   interpreter=$(
   sed -n 's/^#![[:space:]]*\(.*\)/\1 /p;q' ${daemon} |
   re_quote)
   pexp=$interpreter$pexp
 
 Moreover,
 
 - you probably want to unset $interpreter when done.
 - we might want to re_quote the entire $pexp later instead.

Re: /etc/rc.d/rc.subr; prefix ${pexp} with script interpretor path

2013-09-17 Thread Alexander Hall 

On 09/17/13 13:49, Craig R. Skinner wrote:

On 2013-09-16 Mon 23:28 PM |, Alexander Hall wrote:


sed can do it all. Really.


This is getting beyond me Alexander.

Is sed a mechanism to step away from using file(1) ?


Heh, sorry about that. :)

Nah, it's merely a way to combine `head | grep | sed | cut | ...` pipes 
since sed is often capable to cope with it all.





Notes:

- I separate re_quote() cause I think it can be useful in other places.
- I think re_quote() is (basic) regex complete.
- I don't care if the interpreter is (or seems) nonexistant, as that
   shouldn't be a runtime error.
- I'm sure sed may die horribly if you try to feed it a 9GB oneline
   file. However, if so, it should not produce any output anyway. ;)
   If this would ever be considered a real problem, dd(1) would help
   (as espie already mentioned).

re_quote() { sed 's/\([]^$*.\\[]\)/\\\1/g'; }

interpreter=$(
sed -n 's/^#![[:space:]]*\(.*\)/\1 /p;q' ${daemon} |


In this case I make sure sed only looks at the first line (unconditional 
'q'uit at the end), and prints it, followed by a space, but only if it 
was able to withdraw a shebang and optional following whitespace from 
the start of the line (-n, s/^...\(.*\)/\1 /p).


What is known and discussed though, is that sed could potentially crash 
on a *really* long first line in that file. In this case, a pre-check 
with file(1), or input truncation with dd, would help.


Anyway, my $.02 is running out, so I leave it up to the rc.d 
maintainer(s) to determine if they consider it a real problem or if it 
can go the way of the hash-collision discussion...


/Alexander


re_quote)
pexp=$interpreter$pexp

Moreover,

- you probably want to unset $interpreter when done.
- we might want to re_quote the entire $pexp later instead.

/etc/rc.d/rc.subr; prefix ${pexp} with script interpretor path

2013-09-16 Thread Craig R. Skinner 
For scripts (perl, shell, whatever...), prefix ${pexp} with the script's
interpretor path as defined by the script.

No need to override ${pexp} in the daemon's rc file.


Index: rc.subr
===
RCS file: /cvs/src/etc/rc.d/rc.subr,v
retrieving revision 1.70
diff -u -r1.70 rc.subr
--- rc.subr 11 Jul 2013 09:34:33 -  1.70
+++ rc.subr 16 Sep 2013 10:26:09 -
@@ -221,4 +221,9 @@
 unset _rcflags _rcuser
 
 pexp=${daemon}${daemon_flags:+ ${daemon_flags}}
+file ${daemon} | fgrep -q script 
+{
+   shebang=$(head -n 1 ${daemon} | cut -d! -f2)
+   pexp=${shebang} ${pexp}
+}
 rcexec=su -l -c ${daemon_class} -s /bin/sh ${daemon_user} -c





e.g. Remove pexp= from /etc/rc.d/greyscanner:


--- greyscanner.pkg Mon Aug 19 14:46:01 2013
+++ greyscanner Mon Sep 16 11:30:33 2013
@@ -6,7 +6,6 @@
 
 . /etc/rc.d/rc.subr
 
-pexp=/usr/bin/perl ${daemon}
 rc_reload=NO
 
 rc_cmd $1




$ sudo /etc/rc.d/greyscanner restart
greyscanner(ok)
greyscanner(ok)

$ cat /var/run/rc.d/greyscanner
/usr/bin/perl /usr/local/sbin/greyscanner

$ ps auxwww | fgrep greyscanner
root 25280  0.0  0.6  4896  2920 ??  Is11:35AM0:00.04 /usr/bin/perl 
/usr/local/sbin/greyscanner

Cheers,
-- 
Craig Skinner | http://twitter.com/Craig_Skinner | http://linkd.in/yGqkv7

Re: /etc/rc.d/rc.subr; prefix ${pexp} with script interpretor path

2013-09-16 Thread Antoine Jacoutot 
On Mon, Sep 16, 2013 at 11:50:50AM +0100, Craig R. Skinner wrote:
 For scripts (perl, shell, whatever...), prefix ${pexp} with the script's
 interpretor path as defined by the script.
 
 No need to override ${pexp} in the daemon's rc file.

Heh, very interesting trick ;-)
But I don't think that is 100% full proof as is.

e.g.
$ head -n 1 /usr/local/bin/xml2-config | cut -d! -f2
 /bin/sh
You have a white space before the interpreter.

If you can improve that and make sure it works with all similar rc scripts then 
I think it is definitely something that should be looked into.
Thanks.

-- 
Antoine

Re: /etc/rc.d/rc.subr; prefix ${pexp} with script interpretor path

2013-09-16 Thread Craig R. Skinner 
On 2013-09-16 Mon 13:00 PM |, Antoine Jacoutot wrote:
 
 Heh, very interesting trick ;-)
 But I don't think that is 100% full proof as is.
 
 e.g.
 $ head -n 1 /usr/local/bin/xml2-config | cut -d! -f2
  /bin/sh
 You have a white space before the interpreter.
 
 If you can improve that and make sure it works with all similar rc scripts 
 then I think it is definitely something that should be looked into.
 Thanks.
 

Well spotted Antoine.

I wrote a test script with various shebang lines of:

#![space]/bin/ksh
#![space][space]/bin/ksh
#![space][tab]/bin/ksh -x
#![tab]/bin/ksh -x
#![space]/usr/bin/perl
#![space][space]/usr/bin/perl
#![space][tab]/usr/bin/perl -T
#![tab][tab][tab]/usr/bin/perl -T

This seems to work with these test scenarios
(as seen in /var/run/rc.d/rcshebangtester):

Index: rc.subr
===
RCS file: /cvs/src/etc/rc.d/rc.subr,v
retrieving revision 1.70
diff -u -r1.70 rc.subr
--- rc.subr 11 Jul 2013 09:34:33 -  1.70
+++ rc.subr 16 Sep 2013 12:09:42 -
@@ -221,4 +221,9 @@
 unset _rcflags _rcuser
 
 pexp=${daemon}${daemon_flags:+ ${daemon_flags}}
+file ${daemon} | fgrep -q script 
+{
+   shebang=$(head -n 1 ${daemon} | cut -d! -f2 | sed 's/^[[:blank:]]*//')
+   pexp=${shebang} ${pexp}
+}
 rcexec=su -l -c ${daemon_class} -s /bin/sh ${daemon_user} -c


Would it also be worthwhile verifying the 1st element of $shebang is
executable before prefixing $pexp?


Cheers,
-- 
Craig Skinner | http://twitter.com/Craig_Skinner | http://linkd.in/yGqkv7

Re: /etc/rc.d/rc.subr; prefix ${pexp} with script interpretor path

2013-09-16 Thread Marc Espie 
On Mon, Sep 16, 2013 at 08:57:11PM +0200, Antoine Jacoutot wrote:
  Any other thoughts?
 
 Is there any way we could use something else than file(1) ?

Well, you can look for a script using dd and matching it against
the two possible signatures. e.g.,

dd 2/dev/null if=file bs=4 count=1|grep -qe '#!/./ -e '#! /'

I'm not sure that's too much of a progress.

There might be another way to grab the first 4 bytes of a file.
Off-hand I don't see any.

Re: /etc/rc.d/rc.subr; prefix ${pexp} with script interpretor path

2013-09-16 Thread Antoine Jacoutot 
 Any other thoughts?

Is there any way we could use something else than file(1) ?

-- 
Antoine

Re: /etc/rc.d/rc.subr; prefix ${pexp} with script interpretor path

2013-09-16 Thread Craig R. Skinner 
On 2013-09-16 Mon 15:12 PM |, Paul de Weerd wrote:
 Hi Craig,
 
 --- cat bad_script.sh 
 # This is a VERY BAD example of a script!  This will break your
 # shebang thingambob
 
 echo Now what...
 --
 
 I think you'd be better of making sure the first two characters in the
 file are actually #!:
 
   head -n1 ${FILE} | grep '^#!' | sed 's/^#![[:blank:]]*//'
 

Good idea Paul.

Implemented below, along with rudimentary testing for a valid
interpreter:


Index: rc.subr
===
RCS file: /cvs/src/etc/rc.d/rc.subr,v
retrieving revision 1.70
diff -u -r1.70 rc.subr
--- rc.subr 11 Jul 2013 09:34:33 -  1.70
+++ rc.subr 16 Sep 2013 18:19:14 -
@@ -221,4 +221,15 @@
 unset _rcflags _rcuser
 
 pexp=${daemon}${daemon_flags:+ ${daemon_flags}}
+file ${daemon} | fgrep -q script 
+{
+   shebang=$(head -n 1 ${daemon} | grep '^#!' | sed 's/^#![[:blank:]]*//')
+   interpreter=$(echo ${shebang} | cut -d' ' -f1)
+   if [[ -f ${interpreter}  -x ${interpreter} ]]
+   then
+   pexp=${shebang} ${pexp}
+   else
+   rc_err $0: invalid interpreter: ${interpreter}
+   fi
+}
 rcexec=su -l -c ${daemon_class} -s /bin/sh ${daemon_user} -c


Test scripts:

#-=-= /etc/rc.d/rcshebangtester -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=

#!/bin/sh

#daemon=/home/me/bin/rcshebangtester.dud
#daemon=/home/me/bin/rcshebangtester.ksh
daemon=/home/me/bin/rcshebangtester.pl

. /etc/rc.d/rc.subr

rc_bg=YES
#pexp=/bin/ksh ${daemon}
#pexp=/usr/bin/perl -T ${daemon}
#pexp=/usr/bin/perl ${daemon}

rc_cmd $1

#-=-= /home/me/bin/rcshebangtester.dud -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

#!/var/empty
#!   /dev/null
#! /usr/lib/libc.a
# swap about above

echo 'Busted!'

#-=-= /home/me/bin/rcshebangtester.ksh =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=

#! /bin/ksh -x
#!  /bin/ksh
# swap about above

while true
do
uptime
sleep 1
done

#-=-= /home/me/bin/rcshebangtester.pl =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

#!   /usr/bin/perl -T
#!/usr/bin/perl
# swap about above

use strict;
use warnings;

for(;;)
{
print time(), \n;
sleep 1;
}

#-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

$ sudo /etc/rc.d/rcshebangtester -d -f start; \
cat /var/run/rc.d/rcshebangtester; echo; sleep 5; \
sudo /etc/rc.d/rcshebangtester -d -f stop
doing rc_read_runfile
doing rc_check
rcshebangtester
doing rc_start
1379357218
1379357219
doing rc_wait start
doing rc_check
doing rc_write_runfile
(ok)
/usr/bin/perl -T /home/me/bin/rcshebangtester.pl
1379357220
1379357221
1379357222
1379357223
1379357224
doing rc_read_runfile
doing rc_check
rcshebangtester
doing rc_stop
doing rc_wait stop
doing rc_check
doing rc_rm_runfile
(ok)


#-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-


Any other thoughts?
-- 
Craig Skinner | http://twitter.com/Craig_Skinner | http://linkd.in/yGqkv7

Re: /etc/rc.d/rc.subr; prefix ${pexp} with script interpretor path

2013-09-16 Thread Alexander Hall 

On 09/16/13 20:48, Craig R. Skinner wrote:

On 2013-09-16 Mon 15:12 PM |, Paul de Weerd wrote:

Hi Craig,

--- cat bad_script.sh 
# This is a VERY BAD example of a script!  This will break your
# shebang thingambob

echo Now what...
--

I think you'd be better of making sure the first two characters in the
file are actually #!:

head -n1 ${FILE} | grep '^#!' | sed 's/^#![[:blank:]]*//'



Good idea Paul.

Implemented below, along with rudimentary testing for a valid
interpreter:


Index: rc.subr
===
RCS file: /cvs/src/etc/rc.d/rc.subr,v
retrieving revision 1.70
diff -u -r1.70 rc.subr
--- rc.subr 11 Jul 2013 09:34:33 -  1.70
+++ rc.subr 16 Sep 2013 18:19:14 -
@@ -221,4 +221,15 @@
  unset _rcflags _rcuser

  pexp=${daemon}${daemon_flags:+ ${daemon_flags}}
+file ${daemon} | fgrep -q script 
+{
+   shebang=$(head -n 1 ${daemon} | grep '^#!' | sed 's/^#![[:blank:]]*//')
+   interpreter=$(echo ${shebang} | cut -d' ' -f1)


sed can do it all. Really. Notes:

- I separate re_quote() cause I think it can be useful in other places.
- I think re_quote() is (basic) regex complete.
- I don't care if the interpreter is (or seems) nonexistant, as that
  shouldn't be a runtime error.
- I'm sure sed may die horribly if you try to feed it a 9GB oneline
  file. However, if so, it should not produce any output anyway. ;)
  If this would ever be considered a real problem, dd(1) would help
  (as espie already mentioned).

re_quote() { sed 's/\([]^$*.\\[]\)/\\\1/g'; }

interpreter=$(
sed -n 's/^#![[:space:]]*\(.*\)/\1 /p;q' ${daemon} |
re_quote)
pexp=$interpreter$pexp

Moreover,

- you probably want to unset $interpreter when done.
- we might want to re_quote the entire $pexp later instead.

/Alexander


+   if [[ -f ${interpreter}  -x ${interpreter} ]]
+   then
+   pexp=${shebang} ${pexp}
+   else
+   rc_err $0: invalid interpreter: ${interpreter}
+   fi
+}
  rcexec=su -l -c ${daemon_class} -s /bin/sh ${daemon_user} -c


Test scripts:

#-=-= /etc/rc.d/rcshebangtester -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=

#!/bin/sh

#daemon=/home/me/bin/rcshebangtester.dud
#daemon=/home/me/bin/rcshebangtester.ksh
daemon=/home/me/bin/rcshebangtester.pl

. /etc/rc.d/rc.subr

rc_bg=YES
#pexp=/bin/ksh ${daemon}
#pexp=/usr/bin/perl -T ${daemon}
#pexp=/usr/bin/perl ${daemon}

rc_cmd $1

#-=-= /home/me/bin/rcshebangtester.dud -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

#!/var/empty
#!   /dev/null
#! /usr/lib/libc.a
# swap about above

echo 'Busted!'

#-=-= /home/me/bin/rcshebangtester.ksh =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=

#! /bin/ksh -x
#!  /bin/ksh
# swap about above

while true
do
uptime
sleep 1
done

#-=-= /home/me/bin/rcshebangtester.pl =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

#!   /usr/bin/perl -T
#!/usr/bin/perl
# swap about above

use strict;
use warnings;

for(;;)
{
print time(), \n;
sleep 1;
}

#-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

$ sudo /etc/rc.d/rcshebangtester -d -f start; \
cat /var/run/rc.d/rcshebangtester; echo; sleep 5; \
sudo /etc/rc.d/rcshebangtester -d -f stop
doing rc_read_runfile
doing rc_check
rcshebangtester
doing rc_start
1379357218
1379357219
doing rc_wait start
doing rc_check
doing rc_write_runfile
(ok)
/usr/bin/perl -T /home/me/bin/rcshebangtester.pl
1379357220
1379357221
1379357222
1379357223
1379357224
doing rc_read_runfile
doing rc_check
rcshebangtester
doing rc_stop
doing rc_wait stop
doing rc_check
doing rc_rm_runfile
(ok)


#-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-


Any other thoughts?

Re: /etc/rc.d/rc.subr; prefix ${pexp} with script interpretor path

2013-09-16 Thread Alexander Hall 

On 09/16/13 23:32, Marc Espie wrote:

On Mon, Sep 16, 2013 at 11:28:06PM +0200, Alexander Hall wrote:

sed can do it all. Really. Notes:

- I separate re_quote() cause I think it can be useful in other places.
- I think re_quote() is (basic) regex complete.
- I don't care if the interpreter is (or seems) nonexistant, as that
   shouldn't be a runtime error.
- I'm sure sed may die horribly if you try to feed it a 9GB oneline
   file. However, if so, it should not produce any output anyway. ;)
   If this would ever be considered a real problem, dd(1) would help
   (as espie already mentioned).


  ^
   here (see below)



re_quote() { sed 's/\([]^$*.\\[]\)/\\\1/g'; }

interpreter=$(
sed -n 's/^#![[:space:]]*\(.*\)/\1 /p;q' ${daemon} |
re_quote)
pexp=$interpreter$pexp


Ah, but what happens when you run that thru a pure binary file that
spans a single line ?...


That is mentioned just above re_quote... :-)

/Alexander

Re: /etc/rc.d/rc.subr; prefix ${pexp} with script interpretor path

2013-09-16 Thread Marc Espie 
On Mon, Sep 16, 2013 at 11:28:06PM +0200, Alexander Hall wrote:
 sed can do it all. Really. Notes:
 
 - I separate re_quote() cause I think it can be useful in other places.
 - I think re_quote() is (basic) regex complete.
 - I don't care if the interpreter is (or seems) nonexistant, as that
   shouldn't be a runtime error.
 - I'm sure sed may die horribly if you try to feed it a 9GB oneline
   file. However, if so, it should not produce any output anyway. ;)
   If this would ever be considered a real problem, dd(1) would help
   (as espie already mentioned).
 
   re_quote() { sed 's/\([]^$*.\\[]\)/\\\1/g'; }
 
   interpreter=$(
   sed -n 's/^#![[:space:]]*\(.*\)/\1 /p;q' ${daemon} |
   re_quote)
   pexp=$interpreter$pexp

Ah, but what happens when you run that thru a pure binary file that
spans a single line ?...