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

storage: Remember passphrases and export them to Anaconda #19924

Merged

Conversation

mvollmer
Copy link
Member

@mvollmer mvollmer commented Feb 1, 2024

No description provided.

pkg/storaged/anaconda.jsx Dismissed Show dismissed Hide dismissed
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When creating a device and then formatting it this works. However I create an encrypted partition directly the passphrase is not passed or at least not maintained inside the cockpit_passphrases.

AwesomeScreenshot-2_1_2024.5.55.00PM.mp4

@mvollmer
Copy link
Member Author

mvollmer commented Feb 2, 2024

When creating a device and then formatting it this works. However I create an encrypted partition directly the passphrase is not passed or at least not maintained inside the cockpit_passphrases.

Ah, yes, I forgot about this case. I was only looking for the Unlock() method... Thanks!

@mvollmer mvollmer force-pushed the storage-anaconda-password-export branch 2 times, most recently from 59b5340 to 1290a16 Compare February 2, 2024 08:19
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When creating an encrypted logical volume, I see:

Error exporting mount points

Screen Shot 2024-02-01 at 18 31 43

@mvollmer
Copy link
Member Author

mvollmer commented Feb 2, 2024

When creating an encrypted logical volume, I see:

Error exporting mount points

Yes, hmm, let's not hide all errors when exporting, I went too far with that. I don't want the page to crash when there is garbage in sessionStorage, but I think we should crash when there is a bug in the exporting code.

@mvollmer mvollmer force-pushed the storage-anaconda-password-export branch from 1290a16 to 1ecb7d8 Compare February 2, 2024 08:54
@mvollmer
Copy link
Member Author

mvollmer commented Feb 2, 2024

When creating a device and then formatting it this works. However I create an encrypted partition directly the passphrase is not passed or at least not maintained inside the cockpit_passphrases.

Ah, yes, I forgot about this case. I was only looking for the Unlock() method... Thanks!

Should be fixed.

@mvollmer mvollmer force-pushed the storage-anaconda-password-export branch from 1ecb7d8 to 396a7e4 Compare February 2, 2024 08:55
@mvollmer
Copy link
Member Author

mvollmer commented Feb 2, 2024

When creating an encrypted logical volume, I see:

Error exporting mount points

I can't reproduce this, unfortunately.

@mvollmer mvollmer force-pushed the storage-anaconda-password-export branch from 99f6d96 to e7eeeb1 Compare February 2, 2024 13:53
pkg/storaged/anaconda.jsx Fixed Show fixed Hide fixed
@mvollmer mvollmer force-pushed the storage-anaconda-password-export branch from e7eeeb1 to 4e29016 Compare February 2, 2024 15:05
@mvollmer mvollmer removed the blocked label Feb 5, 2024
@mvollmer mvollmer force-pushed the storage-anaconda-password-export branch 3 times, most recently from e6895fc to 3fbe954 Compare February 6, 2024 14:24
KKoukiou
KKoukiou previously approved these changes Feb 7, 2024
doc/anaconda.md Outdated Show resolved Hide resolved
@mvollmer mvollmer force-pushed the storage-anaconda-password-export branch from 3fbe954 to 54761d2 Compare February 7, 2024 08:57
@mvollmer mvollmer marked this pull request as ready for review February 7, 2024 08:57
martinpitt

This comment was marked as outdated.

@mvollmer
Copy link
Member Author

mvollmer commented Feb 7, 2024

Oops, we need tests, forgot about that.

@mvollmer mvollmer marked this pull request as draft February 7, 2024 09:20
@martinpitt martinpitt dismissed their stale review February 7, 2024 09:23

Retracing veto, missed the special case

@martinpitt
Copy link
Member

Ah, I missed that it skips the storing of passphrases in non-anaconda mode, this alleviates most of my concern. So I retract my veto.

@mvollmer mvollmer force-pushed the storage-anaconda-password-export branch 3 times, most recently from 8712a02 to 6cf61ca Compare February 7, 2024 12:52
@mvollmer mvollmer force-pushed the storage-anaconda-password-export branch from 6cf61ca to bddb7d0 Compare February 9, 2024 08:40
pkg/storaged/anaconda.jsx Fixed Show fixed Hide fixed
pkg/storaged/anaconda.jsx Fixed Show fixed Hide fixed
@mvollmer mvollmer force-pushed the storage-anaconda-password-export branch from bddb7d0 to 56676a4 Compare February 9, 2024 08:56
@mvollmer mvollmer removed the blocked label Feb 9, 2024
@mvollmer mvollmer force-pushed the storage-anaconda-password-export branch 2 times, most recently from 92bccde to 21a588b Compare February 12, 2024 15:17
@mvollmer mvollmer marked this pull request as ready for review February 12, 2024 15:19
@mvollmer mvollmer force-pushed the storage-anaconda-password-export branch from 21a588b to 967456b Compare February 12, 2024 16:15
This also changes everything from localStorage to sessionStorage, for
good measure.
@mvollmer mvollmer force-pushed the storage-anaconda-password-export branch from 967456b to e482442 Compare February 12, 2024 16:35
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned on matrix.

Merging prematurely this - even though it's not final solution - will make our collaboration more seemless as we can develop on top of existing tests to at least not break the functionality

Comment on lines +54 to +55
} catch {
console.warn("Can't record passphrases");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These 2 added lines are not executed by any test. Details

remember_passphrase(block, passphrase);
} else if (passphrase_type == "stored") {
await crypto.Unlock("", {});
} else if (passphrase_type == "clevis") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This added line is not executed by any test. Details

@mvollmer mvollmer merged commit 945914a into cockpit-project:main Feb 13, 2024
76 of 78 checks passed
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

4 participants