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

S3 sleep support #190

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

badicsalex
Copy link

A solution to #90.
This is more of a review version, the code mostly works, but I'm pretty sure it could be better.
The first two patches could be merged separately, as those alone would help people, and they are not much of a code change.

@jnohlgard
Copy link

Which kernel configuration flags are required for using the prepare for s3 sleep functionality?

@cristim
Copy link

cristim commented Nov 25, 2017

None, just run 4.14 or later

@cristim cristim mentioned this pull request Nov 25, 2017
@jnohlgard
Copy link

So no need to have CONFIG_BLK_SED_OPAL enabled?
I have been wondering what the userspace interface for the in-kernel SED/OPAL extensions is. Do you know if there is any tool that actually provides a way to use the recently added SED support in the Linux kernel?

@oliverjd
Copy link

I'm unable to build this on 4.14.2 (Ubuntu) - make gives:

linux/DtaDevLinuxDrive.cpp:22:28: fatal error: linux/sed-opal.h: No such file or directory

Any different configure/make parameters necessary?

@cristim
Copy link

cristim commented Nov 27, 2017

Do you have the kernel headers installed for the same kernel version?

@oliverjd
Copy link

Yes, the necessary headers (e.g. sed-opal.h) exist in /usr/src/linux-headers-4.14.2-041402/include but not in /usr/include. Is updating the kernel supposed to add the new headers to /usr/include for GCC to use?

@badicsalex
Copy link
Author

I guess use CXXFLAGS=-I/usr/src/linux-headers-4.14.2-041402/include to make it then. It worked out of the box for me on Arch, I have no idea how it's done on ubuntu-land. How did you even get a 4.14 image?

@cristim
Copy link

cristim commented Nov 28, 2017

On my Ubuntu machine the Linux kernel headers installed under /usr/include/linux come from the linux-libc-dev package:

$ dpkg -S  /usr/include/linux/sed-opal.h
linux-libc-dev:amd64: /usr/include/linux/sed-opal.h

@oliverjd
Copy link

Thanks @cristim, updating linux-libc-dev to the 4.13 zesty headers solved it.

@zbraniecki
Copy link

what's the roadmap for this to land? I see two people committing to this repository - @r0m30 and @kendmerry and they both don't have any activity on github in last quarter.

@davidrohr
Copy link

Hi, thanks for this piece of code. I'd like to see this upsteam as soon as possible. Even when other OS are not supported, it at least solves a part of the S3 sleep problem for Linux.

One suggestion however: I do not like plaintext passwords in boot scripts, how about an option to pass the already hashed password in?

@cristim
Copy link

cristim commented Dec 1, 2017

The systemd configuration file will contain the hash, but the clear text password will be logged in your shell history.

Ideally sedutil should read the clear text once from a password prompt, write the hash to a configuration file and keep reusing the configuration file for all subsequent operations.

This would be nice to also have in the PBA rescue but is especially useful for the sleep use case.

I am not familiar with the Opal specifications but it would be nice if the kernel would automatically detect the unlocked drive and thereafter maintain and rotate an additional random password for the suspend use case, so that the main admin password would only be used for cold boot and the kernel would automatically maintain and rotate an additional password.

@davidrohr
Copy link

I do not really understand. The --prepareForS3Sleep parameter needs the cleartext password, because it hashes the password before parsing it to the kernel.

I am not using systemd, but whatever initsystem is used, it needs to pass the cleartext password to sedutil-cli --prepareForS3Sleep in order to store the password in the kernel, or do I miss something?

@cristim
Copy link

cristim commented Dec 1, 2017

@davidrohr The kernel assumes a cleartext, but that's actually the hash of your password created in the same way as how it was set by sedutil on the drive when setting it up.

@davidrohr
Copy link

Sure, but that is not what I mean, sorry for being not clear.
Let's assume I use the password "foo" for the disk, then I have to run
sedutil-cli --prepareForS3Sleep 0 foo /dev/...
at startup.
Instead, I would prefer to use sedutil-cli --printPasswordHash foo /dev/... to get the hash, and then issue sedutil-cli --prepareForS3Sleep 0 [HASH] /dev/...

@cristim
Copy link

cristim commented Dec 2, 2017

I think this is actually how it's supposed to be done using @badiscalex 's patch. You first generate the hash and then you should pass the same hash to the kernel as a clear text value, otherwise sedutil would be hashing it again.

@davidrohr
Copy link

This is definitely not how it works right now. When I pass in the hashed pw via --prepareForS3Sleep it fails, because the check for the correct pw fails. When I pass in the cleartext pw, it works.

@badicsalex
Copy link
Author

Be sure to also use the "-n -x" flags, meaning "no hash, hex", as seen here:

[Service]
Type=oneshot
ExecStart=sedutil-cli -n -x --prepareForS3Sleep 0 92389312893ababab128912892 /dev/nvme0n1

@davidrohr
Copy link

OK, that makes totally sense. I didn't read the full help text. From the description of --prepareForS3Sleep it wasn't clear. Sorry for bothering.

@TimFW
Copy link

TimFW commented Dec 8, 2017

clear text no way I would never load that. Sleep in and of itself is a big security hole. This it a convenience driven item plain and simple that significantly increases attack surface area and vectors.

As fast as SSD drive systems are these days it would be much more worthwhile and much more secure to look at hibernate where you reenter password. Then if you want make that more convenient enable yubikey support for two factor and then you can shorten secondary password or even go key only if you are ok with the issues it brings.

My system boots cold in 15 seconds if you subtract password typing to a full operating desktop. Even my Qubes Xen based Vir system is less than 30 seconds and its heavy in my current config max security setup.

@jnohlgard
Copy link

Would it be possible to implement a password prompt in the kernel when it resumes instead of having the key in memory all the time?

@davidrohr
Copy link

I totally agree, having a password prompt from the kernel after suspend would be definitely the best solution. This was already discussed in some of the issues.

However, it is probably not a problem of sedutil, but rather the kernel would need to support it.
Basically, this needs to happen before all the userland comes up, so I am not sure how much access the kernel has to the screen and to USB/keyboard in order to do this prompt.

I still think the current S3 sleep support for sedutil in this PR is a great step ahead.
For sure, one has to keep in mind that the only way to really lock the disk again, is shut down the system or to hibernate, but not STR.

@abjugard
Copy link

abjugard commented Jan 15, 2018

Would it be possible for the kernel to just block on IO to the locked drive until said drive has been unlocked? Sort of like how NFS hard mounts work. Then a userland application could be called from RAM to unlock the disk.

@brianbalerno
Copy link

brianbalerno commented Feb 20, 2018

Thanks for this. I've got it building but am getting this error running the command (either with the plain or hashed password):

"Error saving the password to the kernel errno = 25"

eg. sudo sedutil-cli --prepareForS3Sleep 0 mypassword /dev/nvme0n1

Kernel is 4.14.0-3-amd64 and I have libata allow_tpm=1. Any ideas what I'm doing wrong?

@brianbalerno
Copy link

Answering my own question - CONFIG_BLK_SED_OPAL isn't enabled in debian kernel builds. A rebuild with this configured works. I've asked debian to turn it on. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=891213

Once again, many thanks for this effort. :-)

@nickbroon
Copy link

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=891213 has now been merged and resolved, so the debian kernel now has support for OPAL.
It would be great to see this PR merged.

@coldBug
Copy link

coldBug commented Oct 29, 2018

Thanks a lot @badicsalex for your S3 patches - they work flawless for a recent arch with nvme drive. If someone else struggles to build sedutils with this patches, like i did - you have to update the nbproject buildfile 'Makefile-Release_x86_64.mk' like this: patch.txt

@rallyemax
Copy link

For those, like me, who needed @coldBug 's patch to make this build (I am also on an Arch system, x64 and up to date as of the date of this post), and who wish to build the entire project (including LinuxPBA), here is the corresponding LinuxPBA patchfile:
linuxpba_patch.txt

My question is how are other distros' linkers configured that they allow the project to build without entries for DtaDevLinuxDrive in their makefiles? And the reason I'm curious is that if there is a way to force my linker into being a little more laid back, as opposed to the ultra-rectally-retentive killjoy that it is, I'm very much interested.

Thanks for this work, @badicsalex!!! I had mostly abandoned using sedutil due to the sleep issue, I just reactivate it for travelling with my laptop. Now I can just keep it on and sleep easier. (And thanks @coldBug for the patch!)

callahad added a commit to callahad/nixpkgs that referenced this pull request Feb 21, 2019
To quote block/Kconfig:

> Builds Logic for interfacing with Opal enabled controllers.
> Enabling this option enables users to setup/unlock/lock
> Locking ranges for SED devices using the Opal protocol.

Without `BLK_SED_OPAL`, it is impossible to resume from sleep when using
a locked self-encrypting drive.

This configuration option appeared in earlier kernels, but only reached
maturity in 4.14 according to discussion at:

- Drive-Trust-Alliance/sedutil#90 and
- Drive-Trust-Alliance/sedutil#190

This kernel option is enabled in the default kernels shipped with
Fedora, Debian, and other mainstream Linux distributions.
@ckamm
Copy link

ckamm commented Jun 2, 2019

Thanks for the patch!

Note that the hex_passwords change currently breaks the pba image. Initialize it to false in UnlockSEDs.cpp.

@mabachel
Copy link

@badicsalex could you please rebase this PR against https://github.com/ChubbyAnt/sedutil/.
Note: @ChubbyAnt version of sedutil switched from SHA1 to SHA512.

See also: ChubbyAnt#6 and https://sedutil.com/

@badicsalex
Copy link
Author

@mabachel : Unfortunately I have no way to test it. My only OPAL2 drive is the one I use as a system drive, and I don'T want to experiment with it.

@mabachel
Copy link

I see. Since I have too little knwo how of C(++) Development to do it myself, I wondered if you could help. I can test and can do Documentation.

I mergred your commits with @ChubbyAnt code and compiled it successfully. But during setup I could not set the admin1pwd because I got no more access. I guess it is because the your DTAHashPwd.h has to be modified in a proper way to dapat to @ChubbyAnt recent code base. I just copied his SHA512 code in order to merge. But the function arguments have changed as well, as I noticed while taking a closer look.

Maybe you can still try to port S3 Sleep over to ChubbyAnt's SHA512 code? I will test it.

@azazel75
Copy link

azazel75 commented Nov 3, 2021

I'm using this since 3 years ant two different OPAL2 drives and works like a charm. Can some good soul rebase this on current master?

@badicsalex
Copy link
Author

Wtf, after 4 years of radio silence, there was activity on this project? And they didn't merge this PR? Shame.

Alex added 3 commits November 3, 2021 13:39
This is to allow proper '-h' (no password hashing) usage, where the user
might have saved the hash itself, or used a different hashing mechanism
altogether, and ends up with 0 bytes or control characters in the
resulting string.
Useful for users, who want to use different utilities with their SED
drives, and also in combination  with "-x -n", meaning "no hash", "hex",
so no plaintext passwords are saved to scripts or shell history.
The new command is --prepareForS3Sleep, and it should be called every
new boot, as it stores the drive key (password hash) in kernel memory.
@badicsalex
Copy link
Author

I did a dumb rebase, it compiled, but I haven't actually tried it out.

@azazel75
Copy link

azazel75 commented Nov 3, 2021

Thanks @badicsalex , I've just patched 1.20 release with it and it works pretty well!

@fornellas
Copy link

Ping @r0m30 any chance we can get this merged? It seems your last look on this was a ~5 years ago, but things matured since then.

It seems there are multiple reports of this PR being functional (at #90), it'd be nice to see this merged, as to avoid the (already too many) forks out there to support this.

@wolfsoft
Copy link

wolfsoft commented Nov 4, 2023

Nice work, @badicsalex !

I have found a small issue however:

$ sudo sedutil-cli --prepareForS3Sleep 1 password /dev/nvme0
Error saving the password to  the kernel errno = 25

but on the same device with namespace (nvme0n1) it works well:

$ sudo sedutil-cli --prepareForS3Sleep 1 password /dev/nvme0n1

P.S. Linux Mint, kernel 6.2.0-36-generic #37~22.04.1-Ubuntu. sedutil-cli --setLockingRange 1 RW works well both for nvme0 and nvme0n1.

@tdolega
Copy link

tdolega commented Jan 23, 2024

Why can this pull request still not be merged? @r0m30

@darkbasic
Copy link

Why can this pull request still not be merged?

Not sure, but it drove me away from sedutil.

@youk
Copy link

youk commented Jan 24, 2024

Why can this pull request still not be merged? @r0m30

Why should it be merged? It is a security hole in protecting data-at-rest, which TCG SSCs provide.

@Blacklands
Copy link

Blacklands commented Jan 24, 2024

Nothing is getting merged here anyway, this project has been dead for a good while. :(
Look into the various forks.

I also don't know if S3 sleep can really be safely implemented.
But then to be fair, not everyone's threat model expects that someone is after them who would have the capability to read out RAM like that (what's that called again, a Cold Boot Attack?).
But also, isn't S3 sleep dying anyway? A lot of more recent laptops and other mobile devices, at least, don't seem to even offer it anymore. Seems it's all about S0 sleep ("Modern Standby"?) now. Which has the same problem though. In the end, only Hibernation and Shutdown are really safe, probably.

@youk
Copy link

youk commented Jan 24, 2024

I also don't know if S3 sleep can really be safely implemented.

It can. At least much more safer than leaving secrets in memory – see EDK II / Opal Password

But then to be fair, not everyone's threat model expects that someone is after them who would have the capability to read out RAM like that (what's that called again, a Cold Boot Attack?).

It's not CBA since there is no cold boot. Anyways, there's no need to read memory. It is enough to get past login screen.

But also, isn't S3 sleep dying anyway?

IDK if it is dying, but the times when I saw it as a necessary convenience are long gone. My laptop resumes from S4 sleep in a couple of seconds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet