mds: add protection from clients without fscrypt support by lxbsz · Pull Request #54725 · ceph/ceph

lxbsz · 2023-11-30T01:23:14Z

Clients that do not support fscrypt can execute operations that may cause unrecoverable data loss. Add protection on the MDS so that it prevents these clients from executing some operations.

Note, however, that clients will still be able corrupt encrypted files by appending data to them. And they will still be able to read encrypted data from those files.

This PR originally from @luis-henrix 's PR #45073. And I just fix the lock order issue, and will run more tests for it.

Please note this PR will fix one issue from Luis' previous PR:

1, MDS crash issue https://tracker.ceph.com/issues/63685

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

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)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

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

lxbsz · 2023-12-01T03:58:55Z

Locally the xfstests passed.

github-actions · 2024-01-16T12:46:59Z

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

github-actions · 2024-01-16T12:46:59Z

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

vshankar · 2024-01-17T15:54:46Z

@lxbsz Is this ready for review?

lxbsz · 2024-01-18T00:22:43Z

@lxbsz Is this ready for review?

@vshankar Yeah, it's ready.

chrisphoffman

I'm happy to see there's been discussion about server side semantics enforcement.

Do we need any tests included in this PR to ensure our intended behavior doesn't change? We can't one day inadvertently allow older or non-friendly clients to be able to do known malicious things.

lxbsz · 2024-02-26T01:22:38Z

I'm happy to see there's been discussion about server side semantics enforcement.

Do we need any tests included in this PR to ensure our intended behavior doesn't change?

Yeah, we need to run qa.

chrisphoffman · 2024-02-26T15:17:19Z

I'm happy to see there's been discussion about server side semantics enforcement.
Do we need any tests included in this PR to ensure our intended behavior doesn't change?

Yeah, we need to run qa.

To be more specific, I was proposing new tests. For example testing a client working on a fscrypt tree that the client doesn't know about fscrypt. We want to make sure server enforced behavior fails on unaware clients. What do you think?

Do we already test this? If so, can you point me to it?

lxbsz · 2024-02-27T01:59:15Z

I'm happy to see there's been discussion about server side semantics enforcement.
Do we need any tests included in this PR to ensure our intended behavior doesn't change?

Yeah, we need to run qa.

To be more specific, I was proposing new tests. For example testing a client working on a fscrypt tree that the client doesn't know about fscrypt. We want to make sure server enforced behavior fails on unaware clients. What do you think?

Do we already test this? If so, can you point me to it?

Hmm, sounds reasonable to me. Currently there has not thus test yet.

github-actions · 2024-03-22T07:35:30Z

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

vshankar · 2024-09-17T04:40:32Z

This PR is under test in https://tracker.ceph.com/issues/68092.

* refs/pull/54725/head: qa/fscrypt: add fscrypt protection test cases qa/fscrypt: fix the incorrect option name qa/fscrypt: add the dedicated dir for fscrypt basic tests mds: add client dedicated features macro client: fix incorrectly handling the fscrypt_file bug mds: add open trunc protection from clients without fscrypt support mds: add protection from clients without fscrypt support Reviewed-by: Venky Shankar <vshankar@redhat.com> Reviewed-by: Christopher Hoffman <choffman@redhat.com>

vshankar · 2024-10-07T05:24:21Z

Dropping my testing tag since I'm still running into failures which seem related. Will have to debug.

github-actions · 2025-01-04T13:04:21Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

dparmar18 · 2025-01-06T06:03:07Z

Dropping my testing tag since I'm still running into failures which seem related. Will have to debug.

@vshankar are we moving ahead with this?

vshankar · 2025-01-06T06:21:12Z

Dropping my testing tag since I'm still running into failures which seem related. Will have to debug.

@vshankar are we moving ahead with this?

We need to, but the failure that I saw in the test run isn't debugged and @chrisphoffman will have a look.

vshankar · 2025-02-21T03:58:59Z

bumping this up for @chrisphoffman

github-actions · 2025-02-24T19:58:25Z

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

chrisphoffman · 2025-02-25T13:21:19Z

bumping this up for @chrisphoffman

Got it, added to fscrypt master tracker todo list: https://tracker.ceph.com/issues/65217

github-actions · 2025-04-26T14:01:26Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

github-actions · 2025-05-26T14:01:41Z

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

dparmar18 · 2025-05-26T14:18:47Z

@chrisphoffman @vshankar @batrick are we still working on this?

vshankar · 2025-05-26T14:55:33Z

@chrisphoffman @vshankar @batrick are we still working on this?

Not sure what you mean. We need this change to ensure that non-fscrypt clients cannot mangle encrypted files/directories.

vshankar · 2025-05-26T14:55:48Z

(closed by mistake -- reopened)

github-actions · 2025-07-25T16:01:57Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

github-actions · 2025-08-24T18:01:47Z

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

lxbsz requested review from a team, luis-henrix and vshankar November 30, 2023 01:23

github-actions bot added the cephfs Ceph File System label Nov 30, 2023

lxbsz force-pushed the fscrypt-pro-clients branch from cfc098d to a8643cd Compare November 30, 2023 01:24

lxbsz mentioned this pull request Nov 30, 2023

mds: add protection from clients without fscrypt support #45073

Closed

lxbsz force-pushed the fscrypt-pro-clients branch from a8643cd to d3de83d Compare November 30, 2023 09:44

github-actions bot added the needs-rebase label Jan 16, 2024

lxbsz force-pushed the fscrypt-pro-clients branch from d3de83d to 83f1e0e Compare January 17, 2024 07:28

github-actions bot removed the needs-rebase label Jan 17, 2024

chrisphoffman reviewed Feb 22, 2024

View reviewed changes

chrisphoffman mentioned this pull request Mar 13, 2024

tools/cephfs: recover alternate_name of dentries from journal #55792

Merged

14 tasks

lxbsz force-pushed the fscrypt-pro-clients branch 6 times, most recently from 035f6e3 to fdc897f Compare March 21, 2024 09:52

github-actions bot added tests needs-rebase labels Mar 21, 2024

lxbsz force-pushed the fscrypt-pro-clients branch from fdc897f to 06b547d Compare March 26, 2024 01:47

vshankar removed the wip-vshankar-testing4 label Oct 7, 2024

chrisphoffman self-requested a review October 8, 2024 12:59

vshankar removed their assignment Nov 5, 2024

github-actions bot added the stale label Jan 4, 2025

dparmar18 removed the stale label Jan 6, 2025

github-actions bot added the needs-rebase label Feb 24, 2025

github-actions bot added the stale label Apr 26, 2025

github-actions bot closed this May 26, 2025

dparmar18 removed the stale label May 26, 2025

dparmar18 reopened this May 26, 2025

vshankar closed this May 26, 2025

vshankar reopened this May 26, 2025

github-actions bot added the stale label Jul 25, 2025

github-actions bot closed this Aug 24, 2025

Conversation

lxbsz commented Nov 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

lxbsz commented Dec 1, 2023

Uh oh!

github-actions bot commented Jan 16, 2024

Uh oh!

github-actions bot commented Jan 16, 2024

Uh oh!

vshankar commented Jan 17, 2024

Uh oh!

lxbsz commented Jan 18, 2024

Uh oh!

chrisphoffman left a comment

Choose a reason for hiding this comment

Uh oh!

lxbsz commented Feb 26, 2024

Uh oh!

chrisphoffman commented Feb 26, 2024

Uh oh!

lxbsz commented Feb 27, 2024

Uh oh!

github-actions bot commented Mar 22, 2024

Uh oh!

vshankar commented Sep 17, 2024

Uh oh!

vshankar commented Oct 7, 2024

Uh oh!

github-actions bot commented Jan 4, 2025

Uh oh!

dparmar18 commented Jan 6, 2025

Uh oh!

vshankar commented Jan 6, 2025

Uh oh!

vshankar commented Feb 21, 2025

Uh oh!

github-actions bot commented Feb 24, 2025

Uh oh!

chrisphoffman commented Feb 25, 2025

Uh oh!

github-actions bot commented Apr 26, 2025

Uh oh!

github-actions bot commented May 26, 2025

Uh oh!

dparmar18 commented May 26, 2025

Uh oh!

vshankar commented May 26, 2025

Uh oh!

vshankar commented May 26, 2025

Uh oh!

github-actions bot commented Jul 25, 2025

Uh oh!

github-actions bot commented Aug 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lxbsz commented Nov 30, 2023 •

edited

Loading