Skip to content

Conversation

kevans91
Copy link
Contributor

Motivation and Context

In trying to scope out how I might be able to incorporate a TPM into a new pool design, it wasn't very clear how I might be able to fit the pieces together to minimize the amount of hacking I might need to do on ZFS itself. The biggest source of confusion for me, personally, was the description of keylocation (and that "prompt" is maybe not the best name; something like 'adhoc' would have probably been vague enough for me to realize immediately that it wasn't just an interactive thing, prompting is only half of the actual behavior. 'adhoc' is also not a great name so I'm not even proposing anything there). The end result is that I think all of the pieces are there for me to have something wrap zfs load-key that derives a key from the TPM and passes it along with keylocation=prompt, but it seems worth restructuring the description a bit to make it more obvious that this would work.

Description

The current description is somewhat difficult to parse through, and in some cases is a little unclear as to the behavior.

Split it into a paragraphs based on the three distinct behaviors you may get: prompt, file URL, HTTP(S) URL. The descriptions of the file and HTTP(s) behavior seems fine, but prompt is a little vague- expand on it and make it clear that the behavior is actively based on whether the inquisitor of key-data is provided with a tty for stdin or not.

Also clarify why one shouldn't "place keys which should be kept secret on the command line" and note that you have to supply the key via stdin if it's a raw key, just to be sure.

How Has This Been Tested?

I read it again after a shallow dive into libzfs_crypto to confirm the behavior.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Quality assurance (non-breaking change which makes the code more robust against bugs)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

The current description is somewhat difficult to parse through, and in
some cases is a little unclear as to the behavior.

Split it into a paragraphs based on the three distinct behaviors you
may get: prompt, file URL, HTTP(S) URL.  The descriptions of the file
and HTTP(s) behavior seems fine, but prompt is a little vague- expand
on it and make it clear that the behavior is actively based on whether
the inquisitor of key-data is provided with a tty for stdin or not.

Also clarify *why* one shouldn't "place keys which should be kept secret
on the command line" and note that you *have* to supply the key via
stdin if it's a raw key, just to be sure.

Signed-off-by: Kyle Evans <[email protected]>
@amotin amotin added the Status: Accepted Ready to integrate (reviewed, tested) label Sep 15, 2025
@behlendorf behlendorf merged commit 5c46baa into openzfs:master Sep 15, 2025
17 of 18 checks passed
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Sep 15, 2025
The current description is somewhat difficult to parse through, and in
some cases is a little unclear as to the behavior.

Split it into a paragraphs based on the three distinct behaviors you
may get: prompt, file URL, HTTP(S) URL.  The descriptions of the file
and HTTP(s) behavior seems fine, but prompt is a little vague- expand
on it and make it clear that the behavior is actively based on whether
the inquisitor of key-data is provided with a tty for stdin or not.

Also clarify *why* one shouldn't "place keys which should be kept secret
on the command line" and note that you *have* to supply the key via
stdin if it's a raw key, just to be sure.

Reviewed-by: Allan Jude <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: George Melikov <[email protected]>
Signed-off-by: Kyle Evans <[email protected]>
Closes openzfs#17742
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants