clevis icon indicating copy to clipboard operation
clevis copied to clipboard

Please refine the various shell scripts for clarity, robustness, and portability

Open cbiedl opened this issue 5 years ago • 1 comments

Looking into various scripts, I found a few things I'd like to improve:

These scripts are written for bash. Since I want to include clevis in initrds with size constraints, this creates a problem.

The jose invocations are hard to understand since they use the short form.

There are several different coding styles around, for example sometimes using jose -j"$VAR" and sometimes jose -j- <<< "$VAR". Likewise for loading the output of external programs. And some constructions look sloppy. They might be safe but it eases code review if you use additional safeguards.

Suggestions:

  • In jose invocations, always use the long parameter form. Only exception: -UUU but see https://github.com/latchset/jose/issues/80 for a suggestion about this.

  • Have a little style guide and re-write the scripts accordingly. For me, the rules are:

  • (At least) scripts running in the early userland must be executable by busybox' ash

  • Run with "set -e" and "set -u". The latter gives a bad time initially but since the scripts are run in a sensitive situation, it's worth it.

  • Use $(...) instead of backticks.

  • Always put quotes around expansion unless you really know what you're doing.

  • In strings, use double quotes only when expansion is necessary, single quotes else.

  • And over all: Be consistent in how you do things.

This is some work, I know. I created two prototypes, see the next issues, so you can get am impression how the results will look. I can to do this for the existing scripts as well. But only if you signalize you're fine with the idea and are willing include the result.

To be honest, I failed to replace the read -d . idiom used in various unlockers. I was wondering however whether that feature - reading and printing until the first dot in the input stream - could perhaps be implemented as a new jose subcommand, something like jose jwe hdr. This is https://github.com/latchset/jose/issues/81.

cbiedl avatar May 24 '20 19:05 cbiedl

Looking into various scripts, I found a few things I'd like to improve:

These scripts are written for bash. Since I want to include clevis in initrds with size constraints, this creates a problem.

The jose invocations are hard to understand since they use the short form.

There are several different coding styles around, for example sometimes using jose -j"$VAR" and sometimes jose -j- <<< "$VAR". Likewise for loading the output of external programs. And some constructions look sloppy. They might be safe but it eases code review if you use additional safeguards.

Suggestions:

* In jose invocations, always use the long parameter form. Only exception: `-UUU` but see [latchset/jose#80](https://github.com/latchset/jose/issues/80) for a suggestion about this.

* Have a little style guide and re-write the scripts accordingly. For me, the rules are:

* (At least) scripts running in the early userland must be executable by busybox' ash

* Run with "set -e" and "set -u". The latter gives a bad time initially but since the scripts are run in a sensitive situation, it's worth it.

* Use `$(...)` instead of backticks.

* _Always_ put quotes around expansion unless you really know what you're doing.

* In strings, use double quotes only when expansion is necessary, single quotes else.

* And over all: Be consistent in how you do things.

This is some work, I know. I created two prototypes, see the next issues, so you can get am impression how the results will look. I can to do this for the existing scripts as well. But only if you signalize you're fine with the idea and are willing include the result.

I like the ideas here, and the end result of improving portability/readability/consistency is also very interesting. So, feel free to submit changes to existing scripts to incorporate these suggestions.

I have just submitted #205 changing clevis-luks-common-functions and its user. I'd appreciate if you could take a look at it.

sergio-correia avatar May 26 '20 12:05 sergio-correia