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.