koverstreet / bcachefs-tools

http://bcachefs.org
GNU General Public License v2.0
116 stars 88 forks source link

Add passphrase_file to mount options (recreated) #266

Open donmor opened 3 months ago

donmor commented 3 months ago

Recreated #253 as cmd_mount.rs is renamed.

RlndVt commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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.

donmor commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

Modified a few lines to resolve conflicts.

RlndVt commented 1 month ago

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