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

Avoid calling openlog when using GLOME as a library #173

Merged
merged 1 commit into from
Jan 25, 2024

Conversation

lukegb
Copy link
Collaborator

@lukegb lukegb commented Jan 25, 2024

This causes log lines to always use glome-login as the ident, which isn't necessarily desirable.

We can just not call openlog: if we don't call it, syslog will initialise with some default values (i.e. not including the PID), so this won't break any users of GLOME-as-a-library who don't update to call openlog. To avoid this then logging to the wrong facility we update our own calls to syslog to explicitly set the facility.

PAM doesn't need a facility; it appears to just use LOG_AUTHPRIV internally.

@lukegb lukegb requested a review from pkern January 25, 2024 16:58
login/main.c Outdated Show resolved Hide resolved
This causes log lines to always use `glome-login` as the ident,
which isn't necessarily desirable.

We can just not call openlog: if we don't call it, syslog will
initialise with some default values (i.e. not including the PID).
To avoid this then logging to the wrong facility we update our
own calls to syslog to explicitly set the facility.

PAM doesn't need a facility; it appears to just use LOG_AUTHPRIV
internally.
Copy link
Member

@pkern pkern left a comment

Choose a reason for hiding this comment

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

LGTM

@pkern pkern merged commit 951687a into google:master Jan 25, 2024
7 checks passed
@lukegb lukegb deleted the syslogaint branch January 25, 2024 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants