[libvirt] [PATCH] audit: Log only an info message if audit_level < 2 and audit is not supported
Replace the error message during startup of libvirtd with an info message if audit_level < 2 and audit is not supported by the kernel. Audit is not supported by the current kernel if the kernel does not have audit compiled in or if audit is disabled (e.g. by the kernel cmdline). Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- daemon/libvirtd.c | 2 +- src/util/viraudit.c | 17 +++++++++++++++-- src/util/viraudit.h | 2 +- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 589b32192e3d..6bbff0d45684 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1418,7 +1418,7 @@ int main(int argc, char **argv) { if (config->audit_level) { VIR_DEBUG("Attempting to configure auditing subsystem"); - if (virAuditOpen() < 0) { + if (virAuditOpen(config->audit_level) < 0) { if (config->audit_level > 1) { ret = VIR_DAEMON_ERR_AUDIT; goto cleanup; diff --git a/src/util/viraudit.c b/src/util/viraudit.c index 17e58b3a9574..9b755e384f24 100644 --- a/src/util/viraudit.c +++ b/src/util/viraudit.c @@ -55,11 +55,24 @@ static int auditfd = -1; #endif static bool auditlog; -int virAuditOpen(void) +int virAuditOpen(unsigned int audit_level) { #if WITH_AUDIT if ((auditfd = audit_open()) < 0) { - virReportSystemError(errno, "%s", _("Unable to initialize audit layer")); + /* You get these error codes only when the kernel does not + * have audit compiled in or it's disabled (e.g. by the kernel + * cmdline) */ + if (errno == EINVAL || errno == EPROTONOSUPPORT || + errno == EAFNOSUPPORT) { + const char msg[] = "Audit is not supported by the kernel"; + if (audit_level < 2) + VIR_INFO("%s", _(msg)); + else + virReportError(VIR_FROM_THIS, "%s", _(msg)); + } else { + virReportSystemError(errno, "%s", _("Unable to initialize audit layer")); + } + return -1; } diff --git a/src/util/viraudit.h b/src/util/viraudit.h index edaddf3c886f..e0471be1a85d 100644 --- a/src/util/viraudit.h +++ b/src/util/viraudit.h @@ -32,7 +32,7 @@ typedef enum { VIR_AUDIT_RECORD_RESOURCE, } virAuditRecordType; -int virAuditOpen(void); +int virAuditOpen(unsigned int audit_level); void virAuditLog(bool enabled); -- 2.13.4
On Mon, Nov 27, 2017 at 07:02 PM +0100, Marc Hartmayer <mhartmay@linux.vnet.ibm.com> wrote:
Replace the error message during startup of libvirtd with an info message if audit_level < 2 and audit is not supported by the kernel. Audit is not supported by the current kernel if the kernel does not have audit compiled in or if audit is disabled (e.g. by the kernel cmdline).
Maybe we should also take get_auditfail_action(...) [1] into consideration to determine what action should be taken when the audit subsystem is unavailable? This function is available starting with audit 1.2.4. [1] https://linux.die.net/man/3/get_auditfail_action […snip] -- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Mon, Nov 27, 2017 at 07:02 PM +0100, Marc Hartmayer <mhartmay@linux.vnet.ibm.com> wrote:
Replace the error message during startup of libvirtd with an info message if audit_level < 2 and audit is not supported by the kernel. Audit is not supported by the current kernel if the kernel does not have audit compiled in or if audit is disabled (e.g. by the kernel cmdline).
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- daemon/libvirtd.c | 2 +- src/util/viraudit.c | 17 +++++++++++++++-- src/util/viraudit.h | 2 +- 3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 589b32192e3d..6bbff0d45684 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1418,7 +1418,7 @@ int main(int argc, char **argv) {
if (config->audit_level) { VIR_DEBUG("Attempting to configure auditing subsystem"); - if (virAuditOpen() < 0) { + if (virAuditOpen(config->audit_level) < 0) { if (config->audit_level > 1) { ret = VIR_DAEMON_ERR_AUDIT; goto cleanup; diff --git a/src/util/viraudit.c b/src/util/viraudit.c index 17e58b3a9574..9b755e384f24 100644 --- a/src/util/viraudit.c +++ b/src/util/viraudit.c @@ -55,11 +55,24 @@ static int auditfd = -1; #endif static bool auditlog;
-int virAuditOpen(void) +int virAuditOpen(unsigned int audit_level) { #if WITH_AUDIT if ((auditfd = audit_open()) < 0) { - virReportSystemError(errno, "%s", _("Unable to initialize audit layer")); + /* You get these error codes only when the kernel does not + * have audit compiled in or it's disabled (e.g. by the kernel + * cmdline) */ + if (errno == EINVAL || errno == EPROTONOSUPPORT || + errno == EAFNOSUPPORT) { + const char msg[] = "Audit is not supported by the kernel"; + if (audit_level < 2) + VIR_INFO("%s", _(msg)); + else + virReportError(VIR_FROM_THIS, "%s", _(msg)); + } else { + virReportSystemError(errno, "%s", _("Unable to initialize audit layer")); + } + return -1; }
diff --git a/src/util/viraudit.h b/src/util/viraudit.h index edaddf3c886f..e0471be1a85d 100644 --- a/src/util/viraudit.h +++ b/src/util/viraudit.h @@ -32,7 +32,7 @@ typedef enum { VIR_AUDIT_RECORD_RESOURCE, } virAuditRecordType;
-int virAuditOpen(void); +int virAuditOpen(unsigned int audit_level);
void virAuditLog(bool enabled);
-- 2.13.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Polite ping :) -- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On 11/27/2017 07:02 PM, Marc Hartmayer wrote:
Replace the error message during startup of libvirtd with an info message if audit_level < 2 and audit is not supported by the kernel. Audit is not supported by the current kernel if the kernel does not have audit compiled in or if audit is disabled (e.g. by the kernel cmdline).
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- daemon/libvirtd.c | 2 +- src/util/viraudit.c | 17 +++++++++++++++-- src/util/viraudit.h | 2 +- 3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 589b32192e3d..6bbff0d45684 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1418,7 +1418,7 @@ int main(int argc, char **argv) {
if (config->audit_level) { VIR_DEBUG("Attempting to configure auditing subsystem"); - if (virAuditOpen() < 0) { + if (virAuditOpen(config->audit_level) < 0) { if (config->audit_level > 1) { ret = VIR_DAEMON_ERR_AUDIT; goto cleanup; diff --git a/src/util/viraudit.c b/src/util/viraudit.c index 17e58b3a9574..9b755e384f24 100644 --- a/src/util/viraudit.c +++ b/src/util/viraudit.c @@ -55,11 +55,24 @@ static int auditfd = -1; #endif static bool auditlog;
-int virAuditOpen(void) +int virAuditOpen(unsigned int audit_level)
@audit_level might be unused if building without AUDIT enabled.
{ #if WITH_AUDIT if ((auditfd = audit_open()) < 0) { - virReportSystemError(errno, "%s", _("Unable to initialize audit layer")); + /* You get these error codes only when the kernel does not + * have audit compiled in or it's disabled (e.g. by the kernel + * cmdline) */ + if (errno == EINVAL || errno == EPROTONOSUPPORT || + errno == EAFNOSUPPORT) { + const char msg[] = "Audit is not supported by the kernel"; + if (audit_level < 2) + VIR_INFO("%s", _(msg));
This is going to be terrible for translators. If anything, this needs to be: const char *msg = _("error message"); if () VIR_INFO("%s", msg); else virReportError(msg); However, I don't think that we need VIR_INFO to have translated messages at all, therefore we can go with just: if () VIR_INFO("Audit is not supported"); else virReportError(_("Audit is not supported"));
+ else + virReportError(VIR_FROM_THIS, "%s", _(msg)); + } else { + virReportSystemError(errno, "%s", _("Unable to initialize audit layer")); + } +
Otherwise looking good. In fact, we document the behaviour you're implementing. Wonder how we ended up there. Michal
On Wed, Dec 13, 2017 at 10:22 AM +0100, Michal Privoznik <mprivozn@redhat.com> wrote:
On 11/27/2017 07:02 PM, Marc Hartmayer wrote:
Replace the error message during startup of libvirtd with an info message if audit_level < 2 and audit is not supported by the kernel. Audit is not supported by the current kernel if the kernel does not have audit compiled in or if audit is disabled (e.g. by the kernel cmdline).
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- daemon/libvirtd.c | 2 +- src/util/viraudit.c | 17 +++++++++++++++-- src/util/viraudit.h | 2 +- 3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 589b32192e3d..6bbff0d45684 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1418,7 +1418,7 @@ int main(int argc, char **argv) {
if (config->audit_level) { VIR_DEBUG("Attempting to configure auditing subsystem"); - if (virAuditOpen() < 0) { + if (virAuditOpen(config->audit_level) < 0) { if (config->audit_level > 1) { ret = VIR_DAEMON_ERR_AUDIT; goto cleanup; diff --git a/src/util/viraudit.c b/src/util/viraudit.c index 17e58b3a9574..9b755e384f24 100644 --- a/src/util/viraudit.c +++ b/src/util/viraudit.c @@ -55,11 +55,24 @@ static int auditfd = -1; #endif static bool auditlog;
-int virAuditOpen(void) +int virAuditOpen(unsigned int audit_level)
@audit_level might be unused if building without AUDIT enabled.
Hmm, right.
{ #if WITH_AUDIT if ((auditfd = audit_open()) < 0) { - virReportSystemError(errno, "%s", _("Unable to initialize audit layer")); + /* You get these error codes only when the kernel does not + * have audit compiled in or it's disabled (e.g. by the kernel + * cmdline) */ + if (errno == EINVAL || errno == EPROTONOSUPPORT || + errno == EAFNOSUPPORT) { + const char msg[] = "Audit is not supported by the kernel"; + if (audit_level < 2) + VIR_INFO("%s", _(msg));
This is going to be terrible for translators. If anything, this needs to be:
const char *msg = _("error message"); if () VIR_INFO("%s", msg); else virReportError(msg);
However, I don't think that we need VIR_INFO to have translated messages at all, therefore we can go with just:
if () VIR_INFO("Audit is not supported"); else virReportError(_("Audit is not supported"));
I think this is fine - but I’m not sure we should omit „by the kernel“.
+ else + virReportError(VIR_FROM_THIS, "%s", _(msg)); + } else { + virReportSystemError(errno, "%s", _("Unable to initialize audit layer")); + } +
Otherwise looking good. In fact, we document the behaviour you're implementing. Wonder how we ended up there.
Thanks for the review. Shall I send a v2?
Michal
-- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On 12/13/2017 10:45 AM, Marc Hartmayer wrote:
On Wed, Dec 13, 2017 at 10:22 AM +0100, Michal Privoznik <mprivozn@redhat.com> wrote:
On 11/27/2017 07:02 PM, Marc Hartmayer wrote:
Replace the error message during startup of libvirtd with an info message if audit_level < 2 and audit is not supported by the kernel. Audit is not supported by the current kernel if the kernel does not have audit compiled in or if audit is disabled (e.g. by the kernel cmdline).
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- daemon/libvirtd.c | 2 +- src/util/viraudit.c | 17 +++++++++++++++-- src/util/viraudit.h | 2 +- 3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 589b32192e3d..6bbff0d45684 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1418,7 +1418,7 @@ int main(int argc, char **argv) {
if (config->audit_level) { VIR_DEBUG("Attempting to configure auditing subsystem"); - if (virAuditOpen() < 0) { + if (virAuditOpen(config->audit_level) < 0) { if (config->audit_level > 1) { ret = VIR_DAEMON_ERR_AUDIT; goto cleanup; diff --git a/src/util/viraudit.c b/src/util/viraudit.c index 17e58b3a9574..9b755e384f24 100644 --- a/src/util/viraudit.c +++ b/src/util/viraudit.c @@ -55,11 +55,24 @@ static int auditfd = -1; #endif static bool auditlog;
-int virAuditOpen(void) +int virAuditOpen(unsigned int audit_level)
@audit_level might be unused if building without AUDIT enabled.
Hmm, right.
{ #if WITH_AUDIT if ((auditfd = audit_open()) < 0) { - virReportSystemError(errno, "%s", _("Unable to initialize audit layer")); + /* You get these error codes only when the kernel does not + * have audit compiled in or it's disabled (e.g. by the kernel + * cmdline) */ + if (errno == EINVAL || errno == EPROTONOSUPPORT || + errno == EAFNOSUPPORT) { + const char msg[] = "Audit is not supported by the kernel"; + if (audit_level < 2) + VIR_INFO("%s", _(msg));
This is going to be terrible for translators. If anything, this needs to be:
const char *msg = _("error message"); if () VIR_INFO("%s", msg); else virReportError(msg);
However, I don't think that we need VIR_INFO to have translated messages at all, therefore we can go with just:
if () VIR_INFO("Audit is not supported"); else virReportError(_("Audit is not supported"));
I think this is fine - but I’m not sure we should omit „by the kernel“.
Oh, it's just me being lazy to write the full error message. I wasn't focusing on exact phrasing rather than the structure of code. Feel free to use whatever message you want. The one you already have is fine.
+ else + virReportError(VIR_FROM_THIS, "%s", _(msg)); + } else { + virReportSystemError(errno, "%s", _("Unable to initialize audit layer")); + } +
Otherwise looking good. In fact, we document the behaviour you're implementing. Wonder how we ended up there.
Thanks for the review. Shall I send a v2?
Yes please. Michal
participants (2)
-
Marc Hartmayer -
Michal Privoznik