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.

Sign up for GitHub

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

Implement IsClientAuthorized() for FreeBSD and stop failing open #209

Closed
wants to merge 2 commits into from
Closed

Implement IsClientAuthorized() for FreeBSD and stop failing open #209

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ad94750
auth.c: fail the build if we don't have an IsClientAuthorized()
kevans91 Sep 5, 2024
fa4225c
auth.c: implement polkit support for FreeBSD
kevans91 Sep 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
auth.c: fail the build if we don't have an IsClientAuthorized() 
The current version fails open if we don't have some analog for the Linux
SO_PEERCRED functionality implemented for the platform we're building on.

This is incredibly surprising if polkit has been specifically requested, as
the default implementation will just allow all access without consulting
polkit at all.
  • Loading branch information
@kevans91
kevans91 committed Sep 5, 2024
commit ad947504bb91ef974976bf79571d3cc1d59dffc0
4 changes: 4 additions & 0 deletions src/auth.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ unsigned IsClientAuthorized(int socket, const char* action, const char* reader)
return ret;
}

#elif defined(HAVE_POLKIT)

#error polkit is enabled, but no socket cred implementation for this platform

#else

unsigned IsClientAuthorized(int socket, const char* action, const char* reader)
Copy link

Choose a reason for hiding this comment

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

Given the above, #error polkit is enabled, but no socket cred implementation for this platform, I'm not sure if this will matter or not. But I wonder if this function still has an effect, if we can log some kind of message to stderr stating that the allow-all IsClientAuthorized() function is being used.

May be outside the scope of this PR, but wanted to suggest it as a way to make it more obvious if others run into this.

Copy link
Contributor Author

@kevans91 kevans91 Sep 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The allow-all one is what you should get when you build with -Dpolkit=false to avoid sprinkling more HAVE_POLKIT around and allow other authorization mechanisms to be plugged in instead. Its use is actually expected rather than an error in the default configuration, and that seems more or less fine.

Copy link

Choose a reason for hiding this comment

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

Ah, gotcha. Thank you for explaining!

Expand Down