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

fix: smb: don't fail hard if we can't load acls for a file #49288

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

icewind1991
Copy link
Member

Instead we log the error and behave as if no ACL was found.

Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Nov 14, 2024
@icewind1991 icewind1991 added this to the Nextcloud 31 milestone Nov 14, 2024
@icewind1991 icewind1991 requested review from a team, ArtificialOwl, nfebe and come-nc and removed request for a team November 14, 2024 15:02
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

That looks dangerous, it means when unable to get acls it gives default rights to the user? So it may result in more rights because of some system hiccup, no?

@icewind1991
Copy link
Member Author

the acl check only hides folders that users don't have access to from the file listing. Any attempt to enter the folder when the acl check is disabled/not working will result in the user seeing an empty folder.

@come-nc
Copy link
Contributor

come-nc commented Nov 18, 2024

the acl check only hides folders that users don't have access to from the file listing. Any attempt to enter the folder when the acl check is disabled/not working will result in the user seeing an empty folder.

And seeing the folder name is not already an information leak?

@icewind1991
Copy link
Member Author

Windows has the same behavior of showing the non-readable folders

@icewind1991 icewind1991 requested a review from artonge December 3, 2024 18:11
@icewind1991 icewind1991 merged commit 0e10bb5 into master Dec 4, 2024
189 checks passed
@icewind1991 icewind1991 deleted the smb-acl-fail-soft branch December 4, 2024 17:22
@icewind1991
Copy link
Member Author

/backport to stable30

@icewind1991
Copy link
Member Author

/backport to stable29

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants