[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: logging from PAM modules



Solar Designer wrote:
> 
[]
> This (opening /dev/log from within libpam) is, in my opinion, a bad
> idea.  The reason for this is that we add yet another package
> (Linux-PAM) that needs to be compatible with the system's logging
> protocol.  Yes, it has remained constant for many years, but this is
> changing.  We already have datagram and stream sockets, and we will
> likely/hopefully have credential passing soon.

Uphh, compatibility is a pain. :(

> 
> I think the application should register a callback function with
> libpam.  That function will accept the module and service names and
> the format string plus any number of arguments as passed from the
> module.  There should also be a default function provided within
> libpam itself.  The call chain might look like:
> 
> (module) ->                                     /* in module */
> pam_log(format, ...) ->                         /* in libpam */
> appl_log(module, service, format, ...) ->       /* in application */
> syslog(LOG_AUTH, format, ...)                   /* in application */

Module:  pam_log("error %s", pam_strerror(...))
pam:
  sprintf(buf, format, ...)  <<-- the pain is here
  appl_log("%s[%d] %s: %s", service, getpid(), module, buf);
application:
  syslog(LOG_AUTH, format, ...)

Syslog call should include the whole line.  To make this whole line,
one need to process both format strings e.g. as shown (to prefix module's
message with the service/modulename).
The pain is that it is bad to have such a sprintf(buf...) call in each
place -- too many buffer overflows already (yes, I know about snprintf --
this should serve just for example) -- this s[n]printf should not be in
each application (even if very limited set of them should use the facility),
but should be in one place (in pam).

Ok, module name and service (why service needed -- application knows it
nicely) can also be passed as them can be useful to app, but message should
be formatted.  Maybe app_log can be with fixed number of args, like:

  app_log(service, module, message)

where message is again fully formatted string.

But this all are details.

Maybe this is an acceptable solution as you proposed, at least I don't
see drawbacks here.  Also, we _may_ omit openlog at all (and omit
closelog).   This _has_ some drawbacks (indent string will be NULL
or that from application, there is no way to change LOG_PID/LOG_CONS etc
flags).  I don't know how serious this can be.

Regards,
 Michael.





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index] []