qubes.Syslog #321
qubes.Syslog #321
Conversation
This is a qrexec service to receive logs. Fixes QubesOS/qubes-issues#830
|
KISS ftw! |
| if cat[0] == "C": | ||
| ret += "_" | ||
| else: | ||
| ret += ch |
There was a problem hiding this comment.
I'd be more strict about it: just printable ASCII only. Rendering unicode is a complex thing, especially as you can combine various characters etc. Lets not expose whatever you use for viewing the logs...
There was a problem hiding this comment.
I don't like to exclude all non-American non-European logs.
What a receiving application should do: Don't assume anything about the encoding, consider it a byte stream and store it somewhere. Whatever viewer then will attempt to render that byte stream (probably less, vim or whatever pager journalctl uses). rsyslog seems to do it that way.
RFC5424 even clearly states:
"The character set used in MSG SHOULD be UNICODE, encoded using UTF-8
as specified in [RFC3629]. If the syslog application cannot encode
the MSG in Unicode, it MAY use any other encoding." RFC3164 doesn't have such encoding statements which probably resulted in the aforementioned rsyslog finding ("I have now simply taken the message as is and stuffed it into the MSG part").
I also think that users can understand the implications of working with potentially untrustworthy logs.
Btw if such a bug via non-ASCII characters existed, all Linux distributions would be affected by privilege escalation whenever a root user would view system logs via e.g. journalctl -b0. A user would just have to do something like logger "exploit here"... Not impossible, but rather unlikely from my point of view.
There was a problem hiding this comment.
I would say that for a first version and until we are sure there is no security implication or side effect, I would suggest to restrict to what @marmarek said.
There was a problem hiding this comment.
I understand this limitation would affect some people more than others, but also, the main focus of Qubes is to provide reasonably secure system, and rendering unicode does have significant attack surface. This is not purely theoretical, see CVE-2019-18397 for example or almost yearly incident of an unicode wifi name or message crashing iPhones.
Excluding control characters reduces that risk significantly, but IMO not enough to have it allowed by default. That's also why we are not allowing unicode characters in window titles by default.
I also think that users can understand the implications of working with potentially untrustworthy logs.
No, I don't think so. Many (most?) can't understand that doing "curl | bash" is a horrible idea, or pasting something copied from a website directly to a shell (even if the command looks harmless: https://thejh.net/misc/website-terminal-copy-paste). So I definitely wouldn't expect people to understand that reading logs (which is seemingly much safer operation) may be risky if they come from untrusted place. When reading logs in a logvm, it may even not be obvious that they do come from untrusted place in the first place.
Btw if such a bug via non-ASCII characters existed, all Linux distributions would be affected by privilege escalation whenever a root user would view system logs via e.g. journalctl -b0. A user would just have to do something like logger "exploit here"... Not impossible, but rather unlikely from my point of view.
Here you go: https://seclists.org/fulldisclosure/2021/May/51
This one relies on control characters that you excluded, but just a proof that printing untrusted data in a terminal is still (in 2021) not safe yet.
There was a problem hiding this comment.
OK, that proves you right.
I made it ASCII by default now, but provided an option for unicode support.
If that approach is acceptable, I'd adjust my other PR accordingly.
Btw wrt security considerations:
The logs are written via the libc syslog routine, which then writes to the /dev/log unix socket, which usually journald listens to. rsyslog - if installed & configured - then obtains the logs via its imjournal module from journald and does whatever it's configured to do with them. Some distros (e.g. debian) don't use imjournal, but imuxsock instead.
Codecov Report
@@ Coverage Diff @@
## master #321 +/- ##
=======================================
Coverage 73.41% 73.41%
=======================================
Files 3 3
Lines 553 553
=======================================
Hits 406 406
Misses 147 147 Continue to review full report at Codecov.
|
Better not rely on the OS encoding to include ASCII as subset.
| `True`. This infers security risks with unicode parsing bugs in log | ||
| viewers. |
There was a problem hiding this comment.
Another option would be to losslessly escape them, so that no information is lost.
There was a problem hiding this comment.
Yes, a sender may e.g. encode all logs in base64 and then decode it in his super trusted log viewer. ^^
Until such a log viewer exists however I'll assume that automatic escaping/unescaping only provides more bug opportunities in realistic viewers.
Btw a Qubes OS-like solution to this viewer distrust would be to accept unicode and then allow viewing the logs only in a dispVM.
There was a problem hiding this comment.
Btw a Qubes OS-like solution to this viewer distrust would be to accept unicode and then allow viewing the logs only in a dispVM.
I love this solution!
| syslog.openlog(ident=f"qubes.Syslog|{vm}[{pid}]", facility=syslog.LOG_USER) | ||
|
|
||
| input_stream = io.TextIOWrapper(sys.stdin.buffer, encoding="utf-8") | ||
| for untrusted_line in input_stream: |
There was a problem hiding this comment.
This allows a trivial DoS by sending a very very very long line.
There was a problem hiding this comment.
Yup. I didn't bother about DoS as there's many opportunities in Qubes to do something like that (e.g. use the CPU a lot, I/O a lot, send tons of useless logs making journald rotate out the relevant ones, ...). Since it's easy to fix, I fixed it though.
|
Hey, I just wanted to say that this is very exciting and thank you for doing this! |
|
@marmarek Does this PR still have a chance to go upstream or should I close it down? |
|
Just found this. Too bad it is dead :-/ |
This is a qrexec service to receive logs.
Fixes QubesOS/qubes-issues#830
All the doc is in the file header.
The packaging should work. I don't see the point of providing a default RPC policy for this qrexec service as it requires user intervention anyway.