RFE: audit_log_n_untrustedstring() should not log a string with spaces as hex-encoded

[nefigtut]

as of now audit_log_n_untrustedstring() logs a string with spaces as hex-encoded despite that it logs a string without spaces enclosed in quotes unconditionally. this happens because of the *p < 0x21 check in audit_string_contains_control().

i believe it is better to log a string with spaces just in quotes, for this the fix is as simple as comparing to 0x20.

example: a string without spaces (see "aaa" is in quotes anyway): type=AVC msg=audit(1585577505.586:120): avc: denied { relabelto } for pid=540 comm="chcon" name="aaa" dev="vda3" ino=33631320 ...

example: a string with spaces (see "a a" is hex-encoded): type=AVC msg=audit(1585577432.657:119): avc: denied { relabelfrom } for pid=538 comm="chcon" name=612061 dev="vda3" ino=33631336 ...

[nefigtut]

with the fix: a string with spaces (see "a a" in double-quotes):

type=AVC msg=audit(1585578889.168:123): avc: denied { relabelfrom } for pid=542 comm="chcon" name="a a" dev="vda3" ino=33631336 ...

[nefigtut]

@stevegrubb how do you think, if a userspace can handle this change? is it able to parse a string with a space in double-quotes?

[stevegrubb]

No, that violates the parsing rules. We actually have a specification for this. https://github.com/linux-audit/audit-documentation/wiki/SPEC-Writing-Good-Events

Each field should be logged normally when its dealing with trustworthy information like numbers. When the logging function knows that the next field contains something untrusted or it can trick naive parsing, then it has to go through the untrusted string function. The auditsc.c file has a lot of examples of how to do it right. The quotes is to signal that this field is untrusted but OK to parse. The hex encoded characters are to prevent misparsing.

[pcmoore]

More importantly, changing this behavior would be changing the record/field format and would potentially introduce compatibility problems between kernel and userspace.

Regardless, I don't think we are going to make this change anytime soon so I'm going to close this issue.