Skip to content

WIP: remove master key from core dump#359

Open
salieri11 wants to merge 84 commits intochrisphoffman:wip-fscryptfrom
salieri11:wip-igolikov-fscrypt-69205
Open

WIP: remove master key from core dump#359
salieri11 wants to merge 84 commits intochrisphoffman:wip-fscryptfrom
salieri11:wip-igolikov-fscrypt-69205

Conversation

@salieri11
Copy link

@salieri11 salieri11 commented Feb 6, 2025 •
edited
Loading

This PR removes the master key when the user space encryption is used with CEPH FS.

Fixes https://tracker.ceph.com/issues/69205

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows
  • jenkins test rook e2e

@github-actions github-actions bot added the cephfs label Feb 6, 2025
chrisphoffman
chrisphoffman reviewed Feb 7, 2025
View reviewed changes
Copy link
Owner

@chrisphoffman chrisphoffman left a comment

Choose a reason for hiding this comment

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

I wanted to verify with pointers and padding that key is preserved. I'm able to write on ceph-fuse and read correctly on kernel client.

Can you share your steps to verify the key was excluded in core dump? Is it possible you can include it as a test?

@salieri11
Copy link
Author

salieri11 commented Feb 9, 2025

I wanted to verify with pointers and padding that key is preserved. I'm able to write on ceph-fuse and read correctly on kernel client.

Can you share your steps to verify the key was excluded in core dump? Is it possible you can include it as a test?

Well for manual testing I can use gcore to generate core dump, or to kill the process with SIGABRT, which will trigger core dump generation.
Then I can use gdb to find strings in the coredump (since the key is char* and its just a text) I think I will be able to find it. And if they key is excluded from the dump, we should not be able to find this string

@chrisphoffman chrisphoffman force-pushed the wip-fscrypt branch 3 times, most recently from 83c27c9 to a827329 Compare March 12, 2025 21:50
@chrisphoffman chrisphoffman force-pushed the wip-fscrypt branch 2 times, most recently from f330840 to 53006c4 Compare April 7, 2025 13:53
yehudasa and others added 20 commits April 7, 2025 16:22
@yehudasa @chrisphoffman
client,test,osdc: add beginnings of fscrypt support
25ebec9 
Signed-off-by: Yehuda Sadeh <ysadehwe@ibm.com>
@chrisphoffman
Add multi user support on a unlock claim.
080b6a9 
Signed-off-by: Christopher Hoffman <choffman@redhat.com>
@chrisphoffman
client: Allow setpolicy only on dir and when it is empty.
e3b2252 
Signed-off-by: Christopher Hoffman <choffman@redhat.com>
@chrisphoffman
fscrypt: Add tests for add key, remove key, set policy.
c30d899 
Signed-off-by: Christopher Hoffman <choffman@redhat.com>
@chrisphoffman
client: On fscrypt enabled directories, only allow read/write to files
219e36f 
in dir when unlocked. Client should not be able to read/write the encrypted payload.

Signed-off-by: Christopher Hoffman <choffman@redhat.com>
@chrisphoffman
test: Add test for semantics
1c1f74d 
Signed-off-by: Christopher Hoffman <choffman@redhat.com>
@chrisphoffman
Populate add key return
bc96bca 
Signed-off-by: Christopher Hoffman <choffman@redhat.com>
@chrisphoffman
client: rmw doesn't get_cap_ref like it should. After rmw,
1930750 
it put_cap_ref and it didn't happen. Issue with bool need_read.

Fixes: https://tracker.ceph.com/issues/64307
Signed-off-by: Christopher Hoffman <choffman@redhat.com>
@chrisphoffman
client: Ensure symlink plaintext is set
7c6a061 
When creating symlink, ensure symlink_plain is set in fscrypt and
non-fscrypt cases.

Fixes: https://tracker.ceph.com/issues/64691
Signed-off-by: Christopher Hoffman <choffman@redhat.com>
@chrisphoffman
Client: fix O_TRUNC issue
fe58c46 
Signed-off-by: Christopher Hoffman <choffman@redhat.com>
@chrisphoffman
client: After a rebase, no lock is necessary.
e4591e0 
Signed-off-by: Christopher Hoffman <choffman@redhat.com>
@chrisphoffman
client: Fix logic in need read start/end to account for fscrypt.
6365e56 
Fix the logic in need read start/end. We need to make sure that a
whole block is read when a rmw is issued, regardless if it starts
at offset 0 or not. Change size that may be read from where offset
starts to the whole fscrypt block.

Fixes: https://tracker.ceph.com/issues/64819
Signed-off-by: Christopher Hoffman <choffman@redhat.com>
@chrisphoffman
client: During lookup of fscrypt symlink, use target fscrypt info.
7dca59f 
During a lookup of fscrypt enabled symlink, use target fscrypt
info. This must be used because enc key for each file is derived
from master_key+nonce.

Fixes: https://tracker.ceph.com/issues/65615
Signed-off-by: Christopher Hoffman <choffman@redhat.com>
@chrisphoffman
client: fscrypt rmw fails when endoff end of block or file
3389c3e 
Fscrypt rmw fails when end of a write lines up with end of
a block or end of the file.

Fixes: https://tracker.ceph.com/issues/65745
Signed-off-by: Christopher Hoffman <choffman@redhat.com>
@chrisphoffman
client: fix non-encrypted case in read_sync
0ef4e89 
Only append pbl to bl if encrypted case.

Fixes: https://tracker.ceph.com/issues/65964
Signed-off-by: Christopher Hoffman <choffman@redhat.com>
@chrisphoffman
qa: Allow fscrypt testing on fuse
82aa091 
Signed-off-by: Christopher Hoffman <choffman@redhat.com>
@chrisphoffman
qa: Added workunits for testing problem snippets on rmw workloads
8e6583f 
Fixes: https://tracker.ceph.com/issues/66038
Signed-off-by: Christopher Hoffman <choffman@redhat.com>
@chrisphoffman
src/test/libcephfs: add test cases for fscrypt key removal busy case
bd0e0ea 
Signed-off-by: Christopher Hoffman <choffman@redhat.com>
@chrisphoffman
fuse: enable ioctl on dir for fscrypt
637bda5 
Signed-off-by: Christopher Hoffman <choffman@redhat.com>
@chrisphoffman
client: Add busy case on key removal
7dc4c63 
Fixes: https://tracker.ceph.com/issues/64159
Signed-off-by: Christopher Hoffman <choffman@redhat.com>
chrisphoffman and others added 4 commits May 7, 2025 09:50
@chrisphoffman
client: map ENOKEY return value to itself
331af61 
Map ENOKEY return value from fuse_ll_open to itself, instead of returning EIO by default and printing misleading error message to the user.

Fixes: https://tracker.ceph.com/issues/71230
@salieri11
first working version
8fa79b9 
Signed-off-by: Igor Golikov <igolikov@ibm.com>
@salieri11
trying with mmap
4872a29 
Signed-off-by: Igor Golikov <igolikov@ibm.com>
@salieri11
client: add mlock to fscrypt init
62e58e5 
to make sure the memory region with master key is not swapped to the
disk

Signed-off-by: Igor Golikov <igolikov@ibm.com>
@salieri11 salieri11 force-pushed the wip-igolikov-fscrypt-69205 branch from 3782765 to 62e58e5 Compare May 14, 2025 13:34
@github-actions
Copy link

github-actions bot commented Jun 2, 2025

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

@chrisphoffman
Copy link
Owner

chrisphoffman commented Aug 8, 2025

Any updates on this @salieri11?

@chrisphoffman chrisphoffman force-pushed the wip-fscrypt branch 4 times, most recently from d546e04 to ef6003e Compare September 25, 2025 17:37
@chrisphoffman chrisphoffman force-pushed the wip-fscrypt branch 7 times, most recently from a3a72ec to 78dca14 Compare October 23, 2025 19:18
@chrisphoffman chrisphoffman force-pushed the wip-fscrypt branch 5 times, most recently from 6e0df0c to 48ad676 Compare October 29, 2025 19:19
@chrisphoffman chrisphoffman force-pushed the wip-fscrypt branch 2 times, most recently from 0fc7547 to 6ec36b4 Compare November 5, 2025 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chrisphoffman chrisphoffman chrisphoffman left review comments

Assignees

No one assigned

Labels

cephfs needs-rebase

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@salieri11 @chrisphoffman @yehudasa @batrick @vshankar