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

Add passphrase_file to mount options (recreated) #266

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

Conversation

donmor
Copy link

@donmor donmor commented May 14, 2024

Recreated #253 as cmd_mount.rs is renamed.

Biggest bonus is the simplification of the if-else-if-else flow. Instead
we look for the Ok(()) returned from
key::unlock_master_key_using_pasphrase_file
and return early if we find it.

Signed-off-by: Roland Vet <[email protected]>
@RlndVt
Copy link

RlndVt commented May 17, 2024

Hi donmor,

Thank you for this work. Could you update your commit messages to be more descriptive? A commit message 'update' doesn't give any insight into what actually happened in the commit. In addition, Kent requires/strongly prefers that you sign your commits.

I've created a PR on your fork that does some refactoring, let me know what you think.

@donmor
Copy link
Author

donmor commented May 18, 2024

Hi donmor,

Thank you for this work. Could you update your commit messages to be more descriptive? A commit message 'update' doesn't give any insight into what actually happened in the commit. In addition, Kent requires/strongly prefers that you sign your commits.

I've created a PR on your fork that does some refactoring, let me know what you think.

I see... But the non-latest commits is not likely to be editable(-ι_- )

@RlndVt
Copy link

RlndVt commented May 18, 2024

I see... But the non-latest commits is not likely to be editable(-ι_- )

You can perform a interactive rebase to edit the commit message; followed by a force push.

Add -o passphrase_file= mount option. This allows unlocking the volume in fstab.
Update help message, adding descriptions for -o passphrase_file mounting option.
Do if-else simplification in another way, making less changes to the original
formatting
Refactor master key unlocking
@donmor
Copy link
Author

donmor commented May 18, 2024

I see... But the non-latest commits is not likely to be editable(-ι_- )

You can perform a interactive rebase to edit the commit message; followed by a force push.

Got it. Merged ur PR with some modifications.

@RlndVt
Copy link

RlndVt commented May 18, 2024

I considered the solution you are presenting now; attempt_unlock_master_key_with_passphrase_file becomes merely a wrapper for key::unlock_master_key_using_passphrase_file, handling the Err/Ok return cases. This still leaves the messy if/elseif/else/if situation. (Note I'm the author starting the if/else/if mess. :) )

The solution I presented reduces the complexity, reduces the exaggerating amount of indent levels. attempt_unlock_master_key has a clear function and is more than just a wrapper.

Further the current state of the branch does not build.

error[E0425]: cannot find value `passphrase_file` in this scope
   --> src/commands/mount.rs:363:88
    |
363 |             attempt_unlock_master_key_with_passphrase_file(&block_devices_to_mount[0], passphrase_file)
    |                                                                                        ^^^^^^^^^^^^^^^ not found in this scope

error[E0023]: this pattern has 0 fields, but the corresponding tuple variant has 1 field
   --> src/commands/mount.rs:360:48
    |
360 |         let fallback_to_unlock_policy = if let Some() = &opt.passphrase_file {
    |                                                ^^^^^^ expected 1 field, found 0
   --> /builddir/build/BUILD/rustc-1.78.0-src/library/core/src/option.rs:580:56
    |
    = note: tuple variant has 1 field
    |
help: use `_` to explicitly ignore each field
    |
360 |         let fallback_to_unlock_policy = if let Some(_) = &opt.passphrase_file {
    |                                                     +
help: use `..` to ignore all fields
    |
360 |         let fallback_to_unlock_policy = if let Some(..) = &opt.passphrase_file {
    |                                                     ++

error[E0308]: mismatched types
   --> src/commands/mount.rs:363:60
    |
363 |             attempt_unlock_master_key_with_passphrase_file(&block_devices_to_mount[0], passphrase_file)
    |             ---------------------------------------------- ^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `bch_sb_handle`, found `&bch_sb_handle`
    |             |
    |             arguments to this function are incorrect
    |
note: function defined here
   --> src/commands/mount.rs:398:4
    |
398 | fn attempt_unlock_master_key_with_passphrase_file(block_device: bch_sb_handle, passphrase_file: PathBuf) -> bool {
    |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ---------------------------
help: consider removing the borrow
    |
363 -             attempt_unlock_master_key_with_passphrase_file(&block_devices_to_mount[0], passphrase_file)
363 +             attempt_unlock_master_key_with_passphrase_file(block_devices_to_mount[0], passphrase_file)
    |

error[E0308]: arguments to this function are incorrect
   --> src/commands/mount.rs:367:13
    |
367 |             attempt_unlock_master_key_with_passphrase_file(&block_devices_to_mount[0], passphrase_file)
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ --------------------------  --------------- expected `PathBuf`, found `&PathBuf`
    |                                                            |
    |                                                            expected `bch_sb_handle`, found `&bch_sb_handle`
    |
note: function defined here
   --> src/commands/mount.rs:398:4
    |
398 | fn attempt_unlock_master_key_with_passphrase_file(block_device: bch_sb_handle, passphrase_file: PathBuf) -> bool {
    |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ---------------------------  ------------------------
help: consider removing the borrow
    |
367 -             attempt_unlock_master_key_with_passphrase_file(&block_devices_to_mount[0], passphrase_file)
367 +             attempt_unlock_master_key_with_passphrase_file(block_devices_to_mount[0], passphrase_file)
    |
help: try using a conversion method
    |
367 |             attempt_unlock_master_key_with_passphrase_file(&block_devices_to_mount[0], passphrase_file.to_path_buf())
    |                                                                                                       ++++++++++++++

error[E0308]: mismatched types
   --> src/commands/mount.rs:399:56
    |
399 |     match key::unlock_master_key_using_passphrase_file(block_device, passphrase_file.as_path()) {
    |           -------------------------------------------- ^^^^^^^^^^^^ expected `&bch_sb_handle`, found `bch_sb_handle`
    |           |
    |           arguments to this function are incorrect
    |
note: function defined here
   --> src/key.rs:153:8
    |
153 | pub fn unlock_master_key_using_passphrase_file(block_device: &bch_sb_handle, passphrase_file: &std::path::Path) -> anyhow::Result<()> {
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ----------------------------
help: consider borrowing here
    |
399 |     match key::unlock_master_key_using_passphrase_file(&block_device, passphrase_file.as_path()) {
    |                                                        +

Some errors have detailed explanations: E0023, E0308, E0425.
For more information about an error, try `rustc --explain E0023`.
error: could not compile `bcachefs-tools` (bin "bcachefs") due to 5 previous errors

I'll create a alternative PR here.

PS.
You didn't sign your commits.

@donmor
Copy link
Author

donmor commented May 18, 2024

I considered the solution you are presenting now; attempt_unlock_master_key_with_passphrase_file becomes merely a wrapper for key::unlock_master_key_using_passphrase_file, handling the Err/Ok return cases. This still leaves the messy if/elseif/else/if situation. (Note I'm the author starting the if/else/if mess. :) )

The solution I presented reduces the complexity, reduces the exaggerating amount of indent levels. attempt_unlock_master_key has a clear function and is more than just a wrapper.

Further the current state of the branch does not build.

error[E0425]: cannot find value `passphrase_file` in this scope
   --> src/commands/mount.rs:363:88
    |
363 |             attempt_unlock_master_key_with_passphrase_file(&block_devices_to_mount[0], passphrase_file)
    |                                                                                        ^^^^^^^^^^^^^^^ not found in this scope

error[E0023]: this pattern has 0 fields, but the corresponding tuple variant has 1 field
   --> src/commands/mount.rs:360:48
    |
360 |         let fallback_to_unlock_policy = if let Some() = &opt.passphrase_file {
    |                                                ^^^^^^ expected 1 field, found 0
   --> /builddir/build/BUILD/rustc-1.78.0-src/library/core/src/option.rs:580:56
    |
    = note: tuple variant has 1 field
    |
help: use `_` to explicitly ignore each field
    |
360 |         let fallback_to_unlock_policy = if let Some(_) = &opt.passphrase_file {
    |                                                     +
help: use `..` to ignore all fields
    |
360 |         let fallback_to_unlock_policy = if let Some(..) = &opt.passphrase_file {
    |                                                     ++

error[E0308]: mismatched types
   --> src/commands/mount.rs:363:60
    |
363 |             attempt_unlock_master_key_with_passphrase_file(&block_devices_to_mount[0], passphrase_file)
    |             ---------------------------------------------- ^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `bch_sb_handle`, found `&bch_sb_handle`
    |             |
    |             arguments to this function are incorrect
    |
note: function defined here
   --> src/commands/mount.rs:398:4
    |
398 | fn attempt_unlock_master_key_with_passphrase_file(block_device: bch_sb_handle, passphrase_file: PathBuf) -> bool {
    |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ---------------------------
help: consider removing the borrow
    |
363 -             attempt_unlock_master_key_with_passphrase_file(&block_devices_to_mount[0], passphrase_file)
363 +             attempt_unlock_master_key_with_passphrase_file(block_devices_to_mount[0], passphrase_file)
    |

error[E0308]: arguments to this function are incorrect
   --> src/commands/mount.rs:367:13
    |
367 |             attempt_unlock_master_key_with_passphrase_file(&block_devices_to_mount[0], passphrase_file)
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ --------------------------  --------------- expected `PathBuf`, found `&PathBuf`
    |                                                            |
    |                                                            expected `bch_sb_handle`, found `&bch_sb_handle`
    |
note: function defined here
   --> src/commands/mount.rs:398:4
    |
398 | fn attempt_unlock_master_key_with_passphrase_file(block_device: bch_sb_handle, passphrase_file: PathBuf) -> bool {
    |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ---------------------------  ------------------------
help: consider removing the borrow
    |
367 -             attempt_unlock_master_key_with_passphrase_file(&block_devices_to_mount[0], passphrase_file)
367 +             attempt_unlock_master_key_with_passphrase_file(block_devices_to_mount[0], passphrase_file)
    |
help: try using a conversion method
    |
367 |             attempt_unlock_master_key_with_passphrase_file(&block_devices_to_mount[0], passphrase_file.to_path_buf())
    |                                                                                                       ++++++++++++++

error[E0308]: mismatched types
   --> src/commands/mount.rs:399:56
    |
399 |     match key::unlock_master_key_using_passphrase_file(block_device, passphrase_file.as_path()) {
    |           -------------------------------------------- ^^^^^^^^^^^^ expected `&bch_sb_handle`, found `bch_sb_handle`
    |           |
    |           arguments to this function are incorrect
    |
note: function defined here
   --> src/key.rs:153:8
    |
153 | pub fn unlock_master_key_using_passphrase_file(block_device: &bch_sb_handle, passphrase_file: &std::path::Path) -> anyhow::Result<()> {
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ----------------------------
help: consider borrowing here
    |
399 |     match key::unlock_master_key_using_passphrase_file(&block_device, passphrase_file.as_path()) {
    |                                                        +

Some errors have detailed explanations: E0023, E0308, E0425.
For more information about an error, try `rustc --explain E0023`.
error: could not compile `bcachefs-tools` (bin "bcachefs") due to 5 previous errors

I'll create a alternative PR here.

PS.
You didn't sign your commits.

My bad (-ι_- ) It must be me who do drag'n drop without pressing Ctrl, leading to the missing passphrase_file at line 360.

@donmor
Copy link
Author

donmor commented May 27, 2024

Modified a few lines to resolve conflicts.

Fix incorrect args
@RlndVt
Copy link

RlndVt commented Jul 26, 2024

Because Kent has indicated he does not want this functionality included directly; I found an alternative way of decrypting the bcachefs array and mounting it at boot:

https://gist.github.com/RlndVt/7055be264c9492064d3523c8e74ea0a3

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.

2 participants