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

GPO: Duplicated error message for unreadable GPO #983

Closed
wants to merge 1 commit into from

Conversation

elkoniu
Copy link
Contributor

@elkoniu elkoniu commented Feb 6, 2020

In case of unreadable GPO error message has been produced
twice - using sss_log() and DEBUG(). Message priority was
set to SSSDBG_CRIT_FAILURE which is not true in this case
and sidefect was message broadcast over users terminals.

Error message duplication has been removed and error
level changed to SSSDBG_MINOR_FAILURE.

Resolves:
https://pagure.io/SSSD/sssd/issue/4133

@pbrezina
Copy link
Member

Doesn't it always deny access if unreadable GPO is found?

@pbrezina
Copy link
Member

We already discussed this on IRC. TLDR this is a critical error unless it is specifically set to ignore it, because it denies access when GPO entry is potentially misconfigured/malformed. In my opinion we should keep logging it to syslog (sss_log()) but provide a correct priority level from SSS_LOG_* instead of incorrectly using SSSDBG_CRIT_FAILURE which translates to emergency syslog level and thus printing the messages also to terminal.

@mzidek-gh
Copy link
Contributor

Just a note that I also agree with @pbrezina that we should keep logging this in syslog. This is indication of potentially serious misconfiguration, so I do not mind if it is loud (but changing the debug message to MINOR from CRIT is OK).

sss_log() had wrong type set as log level.
The result was error message with very high
priority displayed on all terminals.

Resolves:
https://pagure.io/SSSD/sssd/issue/4133
@elkoniu
Copy link
Contributor Author

elkoniu commented Feb 12, 2020

Thanks for input - I changed log level to SSS_LOG_ERR and kept syslog logging as suggested.

@pbrezina
Copy link
Member

Thank you. Ack.

@pbrezina
Copy link
Member

This patch definitely makes sense so I'm going to push it tomorrow if there are no objections. But I'd like @mzidek-gh to answer questions in the ticket before closing the ticket.

@pbrezina
Copy link
Member

  • master
    • 9188aa1 - GPO: Duplicated error message for unreadable GPO

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

Successfully merging this pull request may close these issues.

None yet

3 participants