I noticed that if offline auth failed for any reason including mistyped password, we would always print System Error. This makes auditing the logs hard as it sounds like an internal error occured.
I don't like the header with a single inline function myself, but I didn't want to clutter util.h with that function either. And we don't have any shared header for that purpose at the moment..suggestions are welcome.
On Thu, 2012-11-08 at 20:09 +0100, Jakub Hrozek wrote:
I noticed that if offline auth failed for any reason including mistyped password, we would always print System Error. This makes auditing the logs hard as it sounds like an internal error occured.
I don't like the header with a single inline function myself, but I didn't want to clutter util.h with that function either. And we don't have any shared header for that purpose at the moment..suggestions are welcome.
Any reason why you want an inline here and not use a normal function in util.c ?
On this topic I have been thinking fro quite a while that using pam status internally is quite confusing.
We should probably finally define our own internal error range and start adding errors there, then have error translators like this one used at the edges only.
Should we open a ticket ?
Simo.
On Thu 08 Nov 2012 02:19:57 PM EST, Simo Sorce wrote:
On Thu, 2012-11-08 at 20:09 +0100, Jakub Hrozek wrote:
I noticed that if offline auth failed for any reason including mistyped password, we would always print System Error. This makes auditing the logs hard as it sounds like an internal error occured.
I don't like the header with a single inline function myself, but I didn't want to clutter util.h with that function either. And we don't have any shared header for that purpose at the moment..suggestions are welcome.
Any reason why you want an inline here and not use a normal function in util.c ?
On this topic I have been thinking fro quite a while that using pam status internally is quite confusing.
We should probably finally define our own internal error range and start adding errors there, then have error translators like this one used at the edges only.
Should we open a ticket ?
It's been there and deferred for a long time: https://fedorahosted.org/sssd/ticket/453
On Thu, Nov 08, 2012 at 02:56:02PM -0500, Stephen Gallagher wrote:
On Thu 08 Nov 2012 02:19:57 PM EST, Simo Sorce wrote:
On Thu, 2012-11-08 at 20:09 +0100, Jakub Hrozek wrote:
I noticed that if offline auth failed for any reason including mistyped password, we would always print System Error. This makes auditing the logs hard as it sounds like an internal error occured.
I don't like the header with a single inline function myself, but I didn't want to clutter util.h with that function either. And we don't have any shared header for that purpose at the moment..suggestions are welcome.
Any reason why you want an inline here and not use a normal function in util.c ?
On this topic I have been thinking fro quite a while that using pam status internally is quite confusing.
We should probably finally define our own internal error range and start adding errors there, then have error translators like this one used at the edges only.
Should we open a ticket ?
It's been there and deferred for a long time: https://fedorahosted.org/sssd/ticket/453
I've bumped it up to NEEDS_TRIAGE.
On Thu, Nov 08, 2012 at 02:19:57PM -0500, Simo Sorce wrote:
On Thu, 2012-11-08 at 20:09 +0100, Jakub Hrozek wrote:
I noticed that if offline auth failed for any reason including mistyped password, we would always print System Error. This makes auditing the logs hard as it sounds like an internal error occured.
I don't like the header with a single inline function myself, but I didn't want to clutter util.h with that function either. And we don't have any shared header for that purpose at the moment..suggestions are welcome.
Any reason why you want an inline here and not use a normal function in util.c ?
It's a simple switch, pretty much just a setter. There's no logic in the function, not even a local variable.
On Fri, 2012-11-09 at 09:00 +0100, Jakub Hrozek wrote:
On Thu, Nov 08, 2012 at 02:19:57PM -0500, Simo Sorce wrote:
On Thu, 2012-11-08 at 20:09 +0100, Jakub Hrozek wrote:
I noticed that if offline auth failed for any reason including mistyped password, we would always print System Error. This makes auditing the logs hard as it sounds like an internal error occured.
I don't like the header with a single inline function myself, but I didn't want to clutter util.h with that function either. And we don't have any shared header for that purpose at the moment..suggestions are welcome.
Any reason why you want an inline here and not use a normal function in util.c ?
It's a simple switch, pretty much just a setter. There's no logic in the function, not even a local variable.
Sure, but as you said it is a bit ugly to have inlines in headers, however I have nothing against if this is just a temporary thing until we properly address error returns, so you get my ack either way.
Simo.
On Fri, Nov 09, 2012 at 09:14:16AM -0500, Simo Sorce wrote:
On Fri, 2012-11-09 at 09:00 +0100, Jakub Hrozek wrote:
On Thu, Nov 08, 2012 at 02:19:57PM -0500, Simo Sorce wrote:
On Thu, 2012-11-08 at 20:09 +0100, Jakub Hrozek wrote:
I noticed that if offline auth failed for any reason including mistyped password, we would always print System Error. This makes auditing the logs hard as it sounds like an internal error occured.
I don't like the header with a single inline function myself, but I didn't want to clutter util.h with that function either. And we don't have any shared header for that purpose at the moment..suggestions are welcome.
Any reason why you want an inline here and not use a normal function in util.c ?
It's a simple switch, pretty much just a setter. There's no logic in the function, not even a local variable.
Sure, but as you said it is a bit ugly to have inlines in headers, however I have nothing against if this is just a temporary thing until we properly address error returns,
Yes, I noted that in a comment of https://fedorahosted.org/sssd/ticket/453
so you get my ack either way.
Pushed to master and sssd-1-9
On Mon, Nov 12, 2012 at 11:36:14AM +0100, Jakub Hrozek wrote:
On Fri, Nov 09, 2012 at 09:14:16AM -0500, Simo Sorce wrote:
On Fri, 2012-11-09 at 09:00 +0100, Jakub Hrozek wrote:
On Thu, Nov 08, 2012 at 02:19:57PM -0500, Simo Sorce wrote:
On Thu, 2012-11-08 at 20:09 +0100, Jakub Hrozek wrote:
I noticed that if offline auth failed for any reason including mistyped password, we would always print System Error. This makes auditing the logs hard as it sounds like an internal error occured.
I don't like the header with a single inline function myself, but I didn't want to clutter util.h with that function either. And we don't have any shared header for that purpose at the moment..suggestions are welcome.
Any reason why you want an inline here and not use a normal function in util.c ?
It's a simple switch, pretty much just a setter. There's no logic in the function, not even a local variable.
Sure, but as you said it is a bit ugly to have inlines in headers, however I have nothing against if this is just a temporary thing until we properly address error returns,
Yes, I noted that in a comment of https://fedorahosted.org/sssd/ticket/453
so you get my ack either way.
Pushed to master and sssd-1-9
Also pushed to sssd-1-8
This is good news for me, I am trying a ticket and this help me :)
Date: Thu, 8 Nov 2012 20:09:22 +0100 From: jhrozek@redhat.com To: sssd-devel@lists.fedorahosted.org Subject: [SSSD] [PATCH] Do not always return PAM_SYSTEM_ERR when offline krb5 fails
I noticed that if offline auth failed for any reason including mistyped password, we would always print System Error. This makes auditing the logs hard as it sounds like an internal error occured.
I don't like the header with a single inline function myself, but I didn't want to clutter util.h with that function either. And we don't have any shared header for that purpose at the moment..suggestions are welcome.
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
sssd-devel@lists.fedorahosted.org
-
Ariel Barria -
Jakub Hrozek -
Simo Sorce -
Stephen Gallagher