Feature/secure mode#271
Open
CyrilVanErsche wants to merge 2 commits intoDrive-Trust-Alliance:masterfrom
Open
Conversation
…prompt. Implemented only for linux.
hMcLauchlan
pushed a commit
to hMcLauchlan/sedutil
that referenced
this pull request
Aug 17, 2022
*BASICALLY CLEANED UP VERSION OF Drive-Trust-Alliance#271 This commit does what the linked PR says, and also fixes a few bugs in that original PR. I'm not sure what the right way to give credit is and it was very painful to resurrect CVE's original patches and roll my own on top, so the disclaimer here is that it's like 95% his code :). A few notable things: * We don't need to modify the makefiles, since we split that out in the prior commit. * We fixed his original makefile, which didn't quite work: that change is folded naturally into prior commit. * The generated makefiles don't need to change, because since CVE's original patchset, GetPassPhrase.o was introduced organically to the codebase, and ergo the makefiles. The most interesting thing here is we allow hashing to be forced off by `-n` even during secure mode. The key issue we ran into was that if a drive is originally set with no hashing, then hash'd invocations in the future will fail(obviously). As implemented, CVE's original patches will silently debug output, and then turn on hashing without telling the user. Not a domain expert in why hashing is necessary here, but in either case, I think we should support the case where a password was originally set without hashing, by allowing hashing to be turned off _if_ specified explicitly. We also do some sneaky business by ensuring -n is evaluated after -s, so -n will always override -s, if provided. Signed-off-by: Howard McLauchlan <hmclauchlan@fb.com>
kushal-kumaran
added a commit
to kushal-kumaran/sedutil
that referenced
this pull request
Jul 10, 2024
*BASICALLY CLEANED UP VERSION OF Drive-Trust-Alliance#271 This commit does what the linked PR says, and also fixes a few bugs in that original PR. I'm not sure what the right way to give credit is and it was very painful to resurrect CVE's original patches and roll my own on top, so the disclaimer here is that it's like 95% his code :). A few notable things: * We don't need to modify the makefiles, since we split that out in the prior commit. * We fixed his original makefile, which didn't quite work: that change is folded naturally into prior commit. * The generated makefiles don't need to change, because since CVE's original patchset, GetPassPhrase.o was introduced organically to the codebase, and ergo the makefiles. The most interesting thing here is we allow hashing to be forced off by `-n` even during secure mode. The key issue we ran into was that if a drive is originally set with no hashing, then hash'd invocations in the future will fail(obviously). As implemented, CVE's original patches will silently debug output, and then turn on hashing without telling the user. Not a domain expert in why hashing is necessary here, but in either case, I think we should support the case where a password was originally set without hashing, by allowing hashing to be turned off _if_ specified explicitly. We also do some sneaky business by ensuring -n is evaluated after -s, so -n will always override -s, if provided. Signed-off-by: Howard McLauchlan <hmclauchlan@fb.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add the secure mode (with -s option) to interactively ask all passwords with a prompt. Implemented only for linux.