Add support for syslog logging #176
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #176 +/- ##
==========================================
- Coverage 90.89% 90.37% -0.53%
==========================================
Files 31 31
Lines 6457 6627 +170
==========================================
+ Hits 5869 5989 +120
- Misses 588 638 +50 ☔ View full report in Codecov by Sentry. |
sosthene-nitrokey
force-pushed
the
syslog
branch
from
7c199c6 to
4368415
Compare
I guess that was not intended. The idea is probably to make it writable only to the owner. It sets „read_only“ to false, whatever that means. 🤷♂️ |
|
This methods sets the write flag for all users, this is why there's a clippy flag. I would rather do it it a more precise way to avoid the security issue, but I also feel like this isn't something that should be done silently like that. |
pkcs11/src/config/logging.rs
Outdated
| // On error, first try logging to syslog | ||
| if syslog::init_unix(syslog::Facility::LOG_USER, log::LevelFilter::Info).is_ok() { | ||
| return; | ||
| }; | ||
| // Otherwise try to log to stderr | ||
| env_logger::try_init().ok(); | ||
| return; |
There was a problem hiding this comment.
Maybe it would make sense to log both to stderr and syslog. In some cases, messages on stderr are more intutive. Do you know if pkcs11-tool passes through stderr messages per default?
There was a problem hiding this comment.
It does pass through stderr. Ok, i'll adapt it to log to both by default.
|
I’m also fine with dropping 3a635b4, especially as we now have improved diagnostics for issues with the log file. |
|
Ok, I reverted 3a635b4, and made multiple loggers possible, defaulting to both |
sosthene-nitrokey
force-pushed
the
syslog
branch
2 times, most recently
from
46edf70 to
f8957dd
Compare
Log to stderr correctly when no file is specified Correctly set the log_level from RUST_LOG when it's not default
sosthene-nitrokey
force-pushed
the
syslog
branch
from
f8957dd to
f8dcb85
Compare
In #176 I wanted to allow RUST_LOG overriding the config file, but it also made the config file be always ignored when file logging was enabled
In #176 I wanted to allow RUST_LOG overriding the config file, but it also made the config file be always ignored when file logging was enabled
This patch adds support for syslog logging.
On initialization errors, it will also now by default log to syslog, instead of logging to stderr. Ideally it would do both, but there doesn't appear to be an easy way to support multiple loggers at the same time.
I also want to revert 3a635b4, this seems like a security concern as it makes the log file writeable by anyone. What was the justification for this change? I also want to not panic if the file can't be opened as is currently the case.
Fix #153