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.

Sign up for GitHub

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 3 commits into from
Feb 6, 2023
Merged

passing args to 'cryptsetup open' #317

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c58e2ec
passing args to 'crypsetup open'
somewhere-or-other Jun 10, 2021
0e74c26
initial test of passing args to 'crypsetup open'
somewhere-or-other Jun 24, 2021
0e256ef
documenting parameter to pass args to cryptsetup
somewhere-or-other Jan 18, 2023
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
View file
Open in desktop
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 •
edited
Loading

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