Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

passing args to 'cryptsetup open' #317

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/luks/clevis-luks-unlock
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ function usage() {
echo " -t SLT Test the passphrase for the given slot without unlocking"
echo " the device"
echo
echo " -o OPTS Pass options to underlying 'cryptsetup open'; be sure"
echo " to quote the OPTS you pass, if they contain a space,"
echo " etc."
echo
exit 2
}

Expand All @@ -43,11 +47,12 @@ if [ $# -eq 1 ] && [ "$1" == "--summary" ]; then
exit 0
fi

while getopts ":d:n:t:" o; do
while getopts ":d:n:t:o:" o; do
case "$o" in
d) DEV="$OPTARG";;
n) NAME="$OPTARG";;
t) SLT="$OPTARG";;
o) OPENARGS="$OPTARG";;
*) usage;;
esac
done
Expand Down Expand Up @@ -75,5 +80,5 @@ else
exit 1
fi

echo -n "${pt}" | cryptsetup open -d- "${DEV}" "${NAME}"
echo -n "${pt}" | cryptsetup ${OPENARGS} open -d- "${DEV}" "${NAME}"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered putting the new ${OPENARGS} in double-quotes, but was concerned about what would happen if someone wanted to include a space in their parameter string, like passing two different parameters (eg. -a -b, as a contrived example), or a space-separated name/value pair. From my tests, it seemed as though enclosing ${OPENARGS} in double-quotes, made cryptsetup consider it a single parameter that happened to contain spaces.

For my personal use, I only had one parameter to pass, so it would work either way. I just wanted to make a solution for a more general use case.

If there's a better way to handle this, I'm happy to take suggestions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I had not considered about the issues when using quotes. So change looks good to me.

Copy link
Contributor Author

@somewhere-or-other somewhere-or-other Jun 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm being honest, I did put it in quotes in a private/dev repo, and discovered the problem, before removing them, and subsequently squashing/rebasing everything into a single pull request.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. In my opinion, it can be merged, but let's wait for @sergio-correia to review it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As additional improvement, it would be nice to add a test to src/luks/tests to simulate a call to clevis-luks-unlock with a cryptsetup option

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sarroutbi I've just submitted an initial attempt at a test. Basically it just uses a bash function to hijack cryptsetup temporarily, and verifies that the arbitrary value makes it through.

It's very possible I've done something incorrectly, though, so don't just take my word for it. Took me a while to figure out the testing/development environment. And that's why I'm mostly a sysadmin, not a developer.

fi
3 changes: 3 additions & 0 deletions src/luks/clevis-luks-unlock.1.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ provisioned Clevis policy. For example:
* *-t* _SLT_ :
Test the passphrase for the given slot without unlocking the device

* *-o* _PARAMS_ :
Pass arbitrary parameters to cryptsetup; quote parameters as necessary

== SEE ALSO

link:clevis-luks-bind.1.adoc[*clevis-luks-bind*(1)]
3 changes: 3 additions & 0 deletions src/luks/tests/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,6 @@ if luksmeta_data.get('OLD_CRYPTSETUP') == '0'
test('backup-restore-luks2', find_program('backup-restore-luks2'), env: env, timeout: 120)
test('pass-tang-luks2', find_program('pass-tang-luks2'), env: env, timeout: 60)
endif

test('unlock-arbitrary-parameter', find_program('unlock-arbitrary-parameter'), env: env)

68 changes: 68 additions & 0 deletions src/luks/tests/unlock-arbitrary-parameter
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
#!/bin/bash -ex
# vim: set ts=8 shiftwidth=4 softtabstop=4 expandtab smarttab colorcolumn=80:
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

TEST=$(basename "${0}")
. tests-common-functions

. clevis-luks-common-functions

on_exit() {
[ ! -d "${TMP}" ] && return 0
tang_stop "${TMP}"
rm -rf "${TMP}"
}

trap 'on_exit' EXIT
trap 'on_exit' ERR

TMP="$(mktemp -d)"

port=$(tang_new_random_port)
tang_run "${TMP}" "${port}"

url="http://localhost:${port}"
adv="${TMP}/adv"
tang_get_adv "${port}" "${adv}"

cfg=$(printf '{"url":"%s","adv":"%s"}' "$url" "$adv")

DEV="${TMP}/luks1-device"
new_device "luks1" "${DEV}"

if ! clevis luks bind -f -d "${DEV}" tang "${cfg}" <<< "${DEFAULT_PASS}"; then
error "${TEST}: Bind should have succeeded."
fi

TESTPARAM="arbitrarytestparameter"

#set up a "cryptsetup" function, to hijack the command
cryptsetup () {
#need to handle "cryptsetup isLuks" from clevis-luks-unlock, among others
if [[ $1 == "isLuks" ]]; then
exit 0;
elif [[ $1 == "luksUUID" ]]; then
echo "TESTINGLUKSUUID"
exit 0;
else
echo "$*" | grep -q -- "${TESTPARAM}"
exit $?
fi
}
export -f cryptsetup

if ! clevis-luks-unlock -o "$TESTPARAM" -d ${DEV} -n clevis_unlock_test; then
error "${TEST}: clevis luks unlock did not match arbitrary test parameter \"$TESTPARAM\"."
fi