LinuxQuestions.org

LinuxQuestions.org (/questions/)
-   Slackware (https://www.linuxquestions.org/questions/slackware-14/)
-   -   Re-write of crypttab/cryptsetup handling - Request for peer review, wider testing. (https://www.linuxquestions.org/questions/slackware-14/re-write-of-crypttab-cryptsetup-handling-request-for-peer-review-wider-testing-4175522498/)

GazL 10-18-2014 04:05 AM

Re-write of crypttab/cryptsetup handling - Request for peer review, wider testing.
 
I don't know whether Pat will be interested in this or not (but if he is, he's welcome to it).

After the shellshock stuff occurred I started looking at the Slackware system scripts for bashisms with an eye to making them shell agnostic.

One of the first I noticed was the crypttab handling code in rc.S which uses arrays. What I expected to be a quick fix, ended up as an extensive rewrite, including adding support for the more useful subset of the options on the freedesktop.org/systemd crypttab page.

Anyway, the results of my labour are here, for any who feel adventurous and want to help me out with feedback and testing,(but don't apply it untested to any boxes you care about, just in case I got something wrong. ;) ).


UPDATE 20/10: Updated the rc.cryptsetup to include the safety-check suggested by Eric. Now split into two separate patch files, one per package.

Alien Bob 10-18-2014 09:17 AM

I like this, and you have my blessing :-) Pat will hopefully also be convinced and apply it.

The one thing that has always been missing in my LUKS implementation in rc.S, was a check right before encrypting a swap volume and enabling it. I have had one bug report in the past where someone added an extra harddisk to his computer, as a result the disk numbering changed and at boot, rc.S overwrote a data partition and turned it into swap.

Can you add a check for the existence of partitions with filesystems on the volume right before LUKS-ifying it as an encrypted swap?

Eric

GazL 10-18-2014 10:06 AM

Thanks for the feedback.

The unconditionally nuke-it if its tagged 'swap' approach has always concerned me too. Might be able to do something with 'blkid' in order to preform a sanity check. Leave it with me. :)

metaschima 10-18-2014 10:32 AM

You seem to be good at bash, but if you cannot solve it, post it in the programming forum and solution will be found.

GazL 10-18-2014 11:02 AM

I think adding a construct like this at the appropriate place will do the job:
Code:

blkid -p -n noswap $DEV && { echo "Not on your nelly!" ; continue ;}
A Normal filesystem will match the noswap filter and blkid will return 0.
A Swap partition or an empty partition will return 2.

The only thing that could be an issue is that as the partition may be left containing random data under some circumstances, though not in normal operation, its possible that blkid could be fooled into thinking that its a filesystem and trigger the fail-safe.

Need to do a bit more testing, but I think this is the best I'm going to come up with. If anyone has a better approach, I'm open to suggestions.

rknichols 10-18-2014 11:18 AM

Quote:

Originally Posted by Alien Bob (Post 5255589)
I have had one bug report in the past where someone added an extra harddisk to his computer, as a result the disk numbering changed and at boot, rc.S overwrote a data partition and turned it into swap.

The way I avoid that problem is by using /dev/disk/by-id/xxxx to identify the partition. That ID includes the drive serial number. Conversion to GPT can also eliminate the issue, since that gives each partition a UUID independent of the partition's content.

metaschima 10-18-2014 03:25 PM

Quote:

Originally Posted by GazL (Post 5255622)
I think adding a construct like this at the appropriate place will do the job:
Code:

blkid -p -n noswap $DEV && { echo "Not on your nelly!" ; continue ;}
A Normal filesystem will match the noswap filter and blkid will return 0.
A Swap partition or an empty partition will return 2.

The only thing that could be an issue is that as the partition may be left containing random data under some circumstances, though not in normal operation, its possible that blkid could be fooled into thinking that its a filesystem and trigger the fail-safe.

Need to do a bit more testing, but I think this is the best I'm going to come up with. If anyone has a better approach, I'm open to suggestions.

I think adding this check is the fastest and best fix ATM. Using disk UUIDs would be a slightly better solution, but would be harder to implement.

GazL 10-20-2014 06:38 AM

Updated the rc.cryptsetup attached to post #1 to include the safety-check suggested by Eric.

gnashley 03-02-2015 01:03 AM

Can you re-attach your files? The link no longer works.

GazL 03-02-2015 06:33 AM

Yep, I tend to treat LQ attachments as transitory and housekeep them after a few months to avoid leaving 'stale' stuff around. I really should sign up with github for these sorts of things.

I don't have the nicely formatted patch files anymore, but here's the raw rc.cryptsetup taken from my system.

metaschima 03-02-2015 12:42 PM

There's a bug in that version with quoting. Here is a patch to fix it and to allow it to run with /bin/ash:
Code:

--- rc.cryptsetup.txt        2015-03-02 12:40:10.992298601 -0600
+++ rc.cryptsetup.new        2015-03-02 12:38:18.045710627 -0600
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/ash
 #  rc.cryptsetup [start|stop]
 #
 #  Use cryptsetup to open or close any encrypted volumes that are
@@ -27,8 +27,8 @@ do
  for word in $line
  do
    [ -z "$a" ]  &&  a="$word"  ||  a="$a $word"
-    [ "$a" != "${a#\"}" -a "$a" = "${a%\"}" ]  &&  continue
-    [ "$a" != "${a#"'"}" -a "$a" = "${a%"'"}" ]  &&  continue
+    [ "$a" != "${a#\"}" ] && [ "$a" = "${a%\"}" ]  &&  continue
+    [ "$a" != "${a#\'}" ] && [ "$a" = "${a%\'}" ]  &&  continue
    [ -z "$LUKS" ]  &&  { LUKS="$a" ; a="" ; }  &&  continue
    [ -z "$DEV" ]  &&  { DEV="$a" ; a="" ; }  &&  continue
    [ -z "$PASS" ]  &&  { PASS="$a" ; a="" ; }  &&  continue


GazL 03-02-2015 01:42 PM

That section runs fine with both bash and ash here as is. What symptoms are you seeing and for what input?

Code:

root@ws1:/tmp# cat crypttab
test /dev/test1 "passphrase one"
test2 /dev/test2 'passphrase two' ro
test3 /dev/test3 "What's the passphrase" ro
test4 /dev/test4 unquoted
root@ws1:/tmp# ./rc.crypttest
luks: test
dev: /dev/test1
pass: "passphrase one"
opts:

luks: test2
dev: /dev/test2
pass: 'passphrase two'
opts: ro

luks: test3
dev: /dev/test3
pass: "What's the passphrase"
opts: ro

luks: test4
dev: /dev/test4
pass: unquoted
opts:


GazL 03-02-2015 02:16 PM

P.S. I chose the quoting carefully as the different shell implementations can be inconsistent when escaping within double quotes. Here, see what happens when you use your "${a#\'}" in ash:
Code:

bash-4.2$ a="'word'"
bash-4.2$ export a
bash-4.2$ bash
bash-4.2$ echo "${a#"'"}"
word'
bash-4.2$ echo "${a#\'}"
word'
bash-4.2$ ash
$ echo "${a#"'"}"
word'
$ echo "${a#\'}"
'word'
$ ksh
$ echo "${a#"'"}"
word'
$ echo "${a#\'}"
word'
$

"${a#"'"}" may look a little odd -- well, ok, to be fair, it looks a lot odd :) -- but it works consistently across shells.

metaschima 03-02-2015 03:19 PM

Well, the original seems to run, but in geany the quoting is reported as being off. checkbashisms also reports this:
Code:

bash-4.2$ checkbash -p -x rc.cryptsetup
possible bashism in rc.cryptsetup line 30 (test -a/-o):
    [ "$a" != "${a#\"}" -a "$a" = "${a%\"}" ]  &&  continue
possible bashism in rc.cryptsetup line 31 (test -a/-o):
    [ "$a" != "${a#"'"}" -a "$a" = "${a%"'"}" ]  &&  continue
error: rc.cryptsetup: Unterminated quoted string found, EOF reached. Wanted: <'>

but, I guess it runs as is.

GazL 03-02-2015 04:10 PM

Yeah, I'm not surprised that syntax highlighters/checkers are having a hard time with it.

Though my original code works, that quoting is definitely ugly. Thinking about it, using a couple of intermediate variables should avoid the need for double-quoting and ought to work.

So,
Code:

  for word in $line
  do
    [ -z "$a" ]  &&  a="$word"  ||  a="$a $word"
  b=${a#\'} ; e=${a%\'} ; [ "$a" != "$b" ]  &&  [ "$a" = "$e" ]  &&  continue
    b=${a#\"} ; e=${a%\"} ; [ "$a" != "$b" ]  &&  [ "$a" = "$e" ]  &&  continue

    [ -z "$LUKS" ]  &&  { LUKS="$a" ; a="" ; }  &&  continue
    [ -z "$DEV" ]  &&  { DEV="$a" ; a="" ; }  &&  continue
    [ -z "$PASS" ]  &&  { PASS="$a" ; a="" ; }  &&  continue
    OPTS="$a"
  done
  unset a b e word

That looks cleaner, and seems to work on first viewing. Need to test it out a little more though.


All times are GMT -5. The time now is 05:09 PM.