Closed jtmoree-github-com closed 3 years ago
This work can be reviewed at https://jtmoree-github-com.github.io/heads-wiki/Install-and-Configure
@jtmoree-github-com I will repass on all the text and propose direct textual changes replacement for each changes, otherwise I don't know how to handle those PRs.
@jtmoree-github-com I will repass on all the text and propose direct textual changes replacement for each changes, otherwise I don't know how to handle those PRs.
Please do.
@jtmoree-github-com Reviewed!
@jtmoree-github-com rest is without comment, liking it. For future PR, please keep scope smaller. As we can see here with 92 comments, this is becoming complicated and would not even know if Heads community would agree with proposed changeset/commit log, having been refused merge for less then that. For sure, nobody else will review.. And there is where the problem lies.
I personally have no problem for the changelog (even less in the heads-wiki) but preferably, a change should be targeting only one feature or required change for a feature/point, limiting the number of files targeted and more importantly, the number of commits linked to the change. This is now targeting 13 files. Nobody else will review this i'm pretty sure. Changes NEED to be smaller, that is for sure. As of right now, If I was merging this as is, it would be all 22 commits in this PR, Developers don't like this. Normally, developers like to see one commit per file/feature. Personally, I do not know how to handle properly documentation change, and have nothing against giving you ownership of the changes (otherwise, I would have to "squash" the changes, giving me ownership of the changes, linking to this PR. In the developer world, squashing gives credit to the merger, not the commiter. I do not mind, where Heeads community developers have insisted many times that commit logs should be the smallest possible. But hat removes the trace of commits linked to editions. I would permit this this time, since its documentatio nand not code contribution, but would not permit this in the future., this case being used as a reference of why this is not working).
For next documentation changes, I would suggest cryptpad/riseup pad (ethereal) so changes are more dynamic and only the consequences results into a PR.
https://pad.riseup.net/ would be a good place to discuss changeset on specific files (22 commits? Really? (I hear others staying) ). Free, spontaneous. Anonymous. Then the result could be from you, claiming ownership for the changes, which I would not have anything against.
I can understand having documentation that discusses unlocking disks with help from the TPM, as that's a feature that is in Heads. Anything outside of that (unlocking LUKS via normal means, via a smart card, or anything else) seems out of scope for Heads documentation, in my opinion.
On Tue, Apr 06, 2021 at 02:56:46PM -0700, tlaurion wrote:
@tlaurion requested changes on this pull request.
@@ -58,17 +58,17 @@ Your other partitions may be encrypted or not based on your preferences and this
Using TPM for decryption of drives
-Heads can seal a Disk Unlock Key using the TPM. This will ensure that upon booting only a disk unlock passphrase and the proper PCR state can access the data on the root partition. This approach uses a Security Dongle for disk decryption and may be used to lock the encrypted data to the hardware. +Heads can protect a LUKS Disk Unlock Key using the TPM. This will ensure that upon booting with Heads only a TPM disk unlock passphrase and the proper PCR state can access the data on the root partition. In this model the TPM enforces rate limits to prevent brute force attacks. If the disk is moved to a different system the LUKS disk unlock passphrase/recovery key in slot 0 may be used to access the encrypted data. This model is similar to the Security Dongle model.
Switching to direct change requests in the goal of merging one day @jtmoree-github-com :
Heads can protect a LUKS Disk Unlock Key slot using the TPM. This will ensure that upon booting with Heads, only a TPM Disk Unlock Key passphrase with the proper TPM measurements (PCRs states) can access the data on the root partition. The state currently measured are the LUKS header itself (slots existing), firmware measurements (refer to https://osresearch.net/Keys/#tpm-pcrs).
In this model the TPM enforces rate limits to prevent brute force attacks when entering Disk Unlock Key passphrase, and will prevent further passphrase attempts after 3 bad attempts, requiring to wait or boot the system with Disk Recovery Key passphrase.
If the disk is moved to a different system the LUKS Disk Recovery Key passphrase in slot 0 may be used to access the encrypted data. This model is similar to the Security Dongle model.
-If this feature is enabled, heads will ask for a Disk Unlock Passphrase when saving the default boot partition configuration. This is not the same as the passphrase/key used by LUKS. The Disk Unlock passphrase will be used to generate a Disk Unlock Key to be placed in the second LUKS keyslot for the root partition. The disk unlock key is sealed in the TPM along with measurements taken at that time. +If the Heads TPM drive decryption feature is enabled, Heads will ask for a Disk Unlock Passphrase (which might be a numerical PIN) when saving the default boot partition configuration. This is not the same as the passphrase/key used by LUKS. The TPM Disk Unlock passphrase will be used to generate a LUKS Disk Unlock Key to be placed in keyslot 1 for the root partition. The LUKS disk unlock key is sealed in the TPM along with measurements taken at that time.
If the Heads TPM drive decryption feature is not disabled, Heads will ask for a Disk Unlock Key passphrase (which might be a PIN: A short passphrase/password) when saving the default boot partition configuration. This is not the same as the passphrase used by LUKS. The TPM Disk Unlock passphrase will be used to generate a LUKS Disk Unlock Key to be placed in LUKS key slot 1 for the root partition. The LUKS Disk Unlock Key is sealed in the TPM along with measurements (LUKS header, firmware measurement, Disk Unlock Key passphrase unlocking TPM NV reserved memory space) taken at that time.
-On boot, heads will ask for the disk unlock passphrase that will release the Disk Unlock Key. This will only succeed if the current measurements match the ones taken when the Key was created.
+On boot, Heads will ask for the TPM disk unlock passphrase that will release the LUKS Disk Unlock Key. This will only succeed if the current measurements match the ones taken when the keys were created.On boot, Heads will ask for the TPM Disk Unlock Key passphrase that will release the LUKS Disk Unlock Key from TPM NV reserved memory. This will only succeed if the current measurements (LUKS header, firmware measurements, Disk Unlock Key passphrase) match the ones taken when the keys were created.
-This option adds an extra layer of security. The TPM will only release the Disk Unlock key if the measurements match and generally an attacker would not have the Disk Unlock passphrase. If an attacker clones the drive and compromises the disk unlock passphrase he will still not have access to the decrypted data. This is due to this passphrase being valid only on this laptop with this firmware and these measured files.
+This option adds an extra layer of security. The TPM will only release the LUKS Disk Unlock key if the measurements match. Generally, an attacker would not have the LUKS Disk Unlock key in keyslot 1 because it was generated by Heads and would not have access to the LUKS disk unlock passphrase in keyslot 0 because it was only used at install. Even if the attacker clones the drive he will still not have access to the decrypted data.This option adds an extra layer of security. The TPM will only release the LUKS Disk Unlock key if the measurements match (again, LUKS header, firmware measurements and Disk Unlock Key passphrase releasing TPM VN stored Disk Unlocked key if everything is congruent).
Generally, an attacker would not have the LUKS Disk Unlock key in key slot 1, because it was generated by Heads and would not have access to the LUKS Disk Recovery Key passphrase stored under keyslot 0 because it was only used at install, and provided when defining new boot default to nenew/change the Disk Unlock Key passphrase. Even if the attacker clones the drive he will still not have access to the decrypted data, unless he eavesdropped the Disk Recovery Key passphrase.
@@ -79,17 +79,34 @@ This feature may be turned off during configuration of heads. The following wil
Using only a LUKS passphrase
-In this model heads is not responsible for any of the encryption of data on disk. The linux kernel decrypts and mounts LUKS containers by asking the user for a passphrase. The correct passphrases allows boot to finish. +In this model heads is not responsible for any of the decryption of data on disk. The linux kernel decrypts and mounts LUKS containers by asking the user for a passphrase. The correct passphrases allows boot to finish.
In this model heads is not responsible for any of the decryption of data on disk. The linux kernel decrypts and mounts LUKS containers by asking the user for a passphrase. The correct passphrases allows boot to finish.
WRONG
Under the previous Disk Unlock Key scenario, Heads is not responsible to unlock Disk Unlock Key. It actually creates an additional initrd file pushed to QubesOS so that the OS is using that key to decrypt content without further user interaction.
In the Disk Recovery Key scenario, Heads doesn't attempt to release the Disk Unlock Key. It simply relies on OS mechanisms to prompt the user for the Disk Recovery Key passphrase at standard OS prompt.
-The first advantage to this approach is that the user only needs to type a PIN on boot rather than a passphrase. Unlike the heads TPM model this model does not lock the data on disk to the hardware. The hard drive may be moved to a different system and used with the security dongle and/or a LUKS passphrase. Also, the security dongle enforces limits which prevent brute force attacks. +The first difference in this approach compared to the heads TPM model is that the user may be limited to typing ONLY a short number sequence (PIN) rather than a full passphrase depending on smartcard hardware used. The hard drive may be moved to a different system and used with the security dongle and/or a LUKS passphrase without modification. Like the TPM, the security dongle enforces limits which prevent brute force attacks.
Sorry. A PIN is not consensual. Namely, and basically, it simply means a short passphrase/password. For the rest of this, I will leave this unreviewed until @mrchromebox @kylerankin review. As per previous reviews, we both agreed this wiki should not be the place to document this. But you seem to want this in this PR, for which I want to encourage you to continue documenting, while seeing this process really involving and not wanting to do this too often.
@kylerankin please comment.
-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/osresearch/heads-wiki/pull/66#pullrequestreview-629385254
@jtmoree-github-com following @kylerankin comment https://github.com/osresearch/heads-wiki/pull/66#issuecomment-814470332 please link to documentation and delete everything else. I think we made ourselves clear multiple times in this PR discussions.
heads is responsible for booting operating systems. As such, documentation that discusses operating systems and how heads may or may not be involved is in scope of heads. I am perfectly happy to create pages outside of the heads wiki with this content and link to it. That will take some time. I will push updates when that is done.
SPECIFICALLY. The objection seems to be that heads should document NONE of the OS components? To that end I will remove almost all of the content on the OS index page, the PureOS page, and leave the Generic and Qubes pages which are based on the current wiki.
We have both stated that a PR is a poor place to manage changes to documentation. In the future I will use https://pad.riseup.net/ to collaborate.
heads is responsible for booting operating systems. As such, documentation that discusses operating systems and how heads may or may not be involved is in scope of heads. I am perfectly happy to create pages outside of the heads wiki with this content and link to it. That will take some time. I will push updates when that is done.
SPECIFICALLY. The objection seems to be that heads should document NONE of the OS components? To that end I will remove almost all of the content on the OS index page, the PureOS page, and leave the Generic and Qubes pages which are based on the current wiki.
@jtmoree-github-com I'm not sure how to properly give better direction.
The original articles inside of Heads wiki were aimed at differences in ways different Linux Operating Systems needed some tweaking depending on certain Linux flavors to properly boot. Those generic advice are in the scope of what Heads should give in the scope of user documentation, in my opinion. The best example of that is how different Linux flavor require different options to be able to boot from ISO. This is relevant to Heads, since those tweaks may need to be added to be able to properly boot the desired operating system. The same applies to the ways LUKS keys need to be passed to OS, which varies from QubesOS (fedora base) from a debian base. Differences in how to properly install guix with a seperate /boot partition would be welcome, for example, since guix considers having everything under /, which Heads doesn't play with, since it requires a distinct /boot partition. That fact is relevant for Heads. Having screen captures for all different operating systems isn't.
Covering in details some operating system installation opens the door for other PRs to be proposed by users wanting other OS being documented, and the goal of the docs is to be as minimal possible, to pass the buck to Operating Systems installation instructions where necessary, only specifying the changes that applies when using Heads.
In my opinion, general instructions, as for the QubesOS instructions, were aimed at creating originally a different partitioning scheme in the goal of having QubesOS installation mostly in read only, where dmverity could be used inside of Heads to measure the disk content in between templates upgrades, but that never happened officially. The idea was there, the documentation was partial, waiting for upstream changes that never happened. The QubesOS installation page was then kept, but the perks leading to those required perks were removed, leaving the guide with instructions.... which actually guide the user through a normal installation. I personally do not see any added value into having this inside of the documentation. But this is only my opinion. In this case, like others, those documentation pages require changes over time, which most of the time are not done in time and become deprecated. This is why minimal, required, user documentation, seems to be the best fit for Heads project.
I would disagree about removing everything and my goal here is not to discourage contributions. But the scope needs to be more limited for next PRs. Adding X and having only X in a PR, where limited files being touched by a PR in the scope of X. I think that the removal of other pages should be in another PR. Collaboration can also happens inside of the heads slack channel.
I prefer personally to discuss changes that are planned to be done in issues, then once agreement there, create related PR which closes an issue. If there is matter for debate, or uncertainty, that debate/discussion should happen over slack.
I like etherpad for drafting and direct collaboration.
@jtmoree-github-com following @kylerankin comment #66 (comment) please link to documentation and delete everything else. I think we made ourselves clear multiple times in this PR discussions.
My comment here was not to delete everything. But following kyle replies (2), there is disagreement also from his side of documenting what should be documented on their website. Maybe what is missing should be included in an issue? We tend to like things being fixed upstream instead of patching what will need to be removed later on.
@tlaurion @kylerankin I just reviewed the official PureOS install guide (linked in this PR) and docs.puri.sm. Neither addresses installation of PureOS while using heads. PureOS install guide doesn't mention heads/Pureboot and Pureboot instructions ignore installation of PureOS (assuming due to factory installation). I have attempted to get Purism to improve content within those resources and will continue to do so.
In the mean time the PureOS page in this PR has
1) content not available anywhere else
2) information specific to heads and installing PureOS
3) NO screenshots or extra information that is out of scope for heads
I can understand having documentation that discusses unlocking disks with help from the TPM, as that's a feature that is in Heads. Anything outside of that (unlocking LUKS via normal means, via a smart card, or anything else) seems out of scope for Heads documentation, in my opinion.
This point of view seems to discourage any text/copy/information in the wiki which would compare the use of the heads+TPM for unlocking disk with other methods such as using a smart card. If this is the intention of the heads project I will migrate those comparisons to an external site and reference in the wiki. The OS/index page is where this heads vs other methods content sits in the PR.
I split the changes into two groups. The OS portions are the ones we have had the most 'challenges'. To that end I created a new branch that excludes those changes. You can see what would be pulled into a PR here https://github.com/osresearch/heads-wiki/compare/master...jtmoree-github-com:docs-refresh2.
I did not create a PR because we have no agreement on how to merge these changes. I offer this as another approach. If we can get that merged I will then do the same for the OS folder which also replaces install-os.md.
@jtmoree-github-com There is a lot of repetition under current documentation, since I explained on the spot each things that were invalid in individual text changes in the past 25 commits.
I see you opened another branch https://github.com/osresearch/heads-wiki/pull/66#issuecomment-832656886
My concerns were about information being technically right. I think we are at that point now. But the content is long and repetitive, not sure users would read it all.
What are the next steps here/there? Please tag me in, else I do not automatically see where PR are updated.
@tlaurion I made the outstanding changes. I made a second branch to clean up the commit history.
Where to go from here depends on how much cleaning we want. We can merge this branch but it's a mess of multiple edits on edits. If we merge the other 'clean' branch then I will create another one with just the OS sections that is also cleaner.
Closing this PR as it was messy and split into two other branches. The first replacement was merged as PR 67 and the second is in the works.
re-organized OS installations. Add PUREOS. split qubes and generic into separate pages. update and clarify generic instructions.