On St, 2014-09-03 at 17:02 +0400, Dmitry V. Levin wrote:
On Wed, Sep 03, 2014 at 08:45:20AM -0400, Richard Guy Briggs wrote:
> On 14/09/03, Dmitry V. Levin wrote:
> > On Wed, Sep 03, 2014 at 10:30:26AM +0200, Tomas Mraz wrote:
> > > On St, 2014-09-03 at 07:52 +0400, Dmitry V. Levin wrote:
> > > > On Thu, Jul 24, 2014 at 05:29:50PM +0200, Tomas Mraz wrote:
> > [...]
> > > > > @@ -134,9 +188,18 @@ _pam_auditlog(pam_handle_t *pamh, int
action, int retval, int flags)
> > > > > retval = PAM_SYSTEM_ERR;
> > > > > }
> > > > >
> > > > > - if (_pam_audit_writelog(pamh, audit_fd, type, message,
retval) < 0)
> > > > > + buf = _pam_list_grantors(h, message, retval);
> > > > > + if (buf == NULL) {
> > > > > + /* allocation failure */
> > > > > + pam_syslog(pamh, LOG_CRIT, "_pam_list_grantors()
failed: %m");
> > > > > + retval = PAM_SYSTEM_ERR;
> > > > > + }
> > > >
> > > > Since there are no grantors when retval != PAM_SUCCESS, we can make
> > > > _pam_list_grantors simpler and save on extra asprintf/free by
calling
> > > > _pam_list_grantors only if retval == PAM_SUCCESS.
> > > >
> > > > No need to print "grantor=?" when there are no grantors.
> > >
> > > This was explicitly requested to make the audit record more orthogonal.
> > > So I will keep it as is.
> >
> > Still there is a slight chance of _pam_list_grantors returning NULL, so
> > there would be two kinds of messages, with and without mentioning
> > grantors, anyway. For me "grantor=?" in the case when there are no
> > grantors just looks confusing.
>
> In that case, Steve Grubb has requested something like "grantor=(none)"
> so that there are no disappearing fields in audit records.
No, mod_name can be almost anything, including "?", "(none)", and
even "".
It doesn't contain spaces, though, so "grantor= " would be less confusing,
but how you are going to distinguish "grantor=" (grantor is a module
called ".so") from "grantor= " (no grantors) is beyond my
imagination.
Well at least the pam module name cannot be /.so but grantor=/ could be
confusing as well. I'll stick with the original grantor=? as supporting
module named ?.so is something we do not really have to :).
So, I'll rather change the patch so there is no possibility of the
message not having the grantor field than remove the grantor=? case
completely.
The attached patch should contain this change and all the other minor
corrections.
--
Tomas Mraz
No matter how far down the wrong road you've gone, turn back.
Turkish proverb
(You'll never know whether the road is wrong though.)