USBguard bug

[Steve Grubb]

On Tuesday, February 4, 2020 3:10:14 AM EST Burn Alting wrote:
> On Mon, 2020-02-03 at 11:35 -0500, Steve Grubb wrote:
> > Hello,
> > 
> > On Friday, January 31, 2020 4:58:18 PM EST Burn Alting wrote:
> > > Currently when the USB management framework, usbguard (
> > > https://github.com/USBGuard/usbguard),  is building it's key-value
> > > pairsprior to calling audit_log_user_message() with a
> > > AUDIT_USER_DEVICE type,it looks at each value  and decides to hex
> > > encode the value if anycharacter  in the value matches the expression
> > > (str[i] == '"' || str[i] <0x21 || str[i] == 0x7F).> 
> > It should be calling audit_value_needs_encoding().
> > 
> > > This can be found in
> > > https://github.com/USBGuard/usbguard/blob/master/src/Daemon/LinuxAuditB
> > > ack
> > > end.cpp where it makes the call
> > > 
> > > 	audit_log_user_message(_audit_fd, AUDIT_USER_DEVICE,
> > > 
> > > message.c_str(),      /*hostname=*/nullptr, /*addr=*/nullptr,
> > > /*tty=*/nullptr, result);
> > > As a result, one sees audit events such as
> >  
> >  <snip>
> >  
> > > I have a number of questions- What is the best recommendation I can
> > > make in a bug report I'd like toraise so that the auparse library can
> > > reliably interpret all their key'svalues?
> > 
> > If its a field that is knowingly going to be user controlled, then it has
> > to follow the convention shown here:
> > https://github.com/linux-audit/audit-userspace/blob/master/lib/
> > audit_logging.c#L196
> > Notably, the "else" branch includes double quotes.
> 
> I believe their code does that. I should have been a little clearer ... if
> they have a msg value with multiple key value pairs, some escaped with
> double quotes and other hex encoded, how do I get ausearch to reliably
> decode the hex encoded value?

It should decode hex-encoded fields.

> Do we need to add usbguard specific keys to
> auparse/typetab.h?

Possibly. They may have did their own thing without coordination. Wouldn't be 
the first time nor the last.

> > > - Should I also request they actually provide hostname and addrvalues
> > > to audit_log_user_message()?
> > 
> > This should be covered by auditd.conf, name_format.
> > 
> > > - If one want them to identify the user who participates in the
> > > activitywhat is the best recommendation to make in terms of keys in
> > > the message?
> > 
> > There is no way to associate a user to a device being plugged in. What if
> > no one is logged in? For example a "janitor" walks by a system at night
> > and plugs in a usb cactus or evil crow. And then sometimes a system
> > permanently has a usb device connected and the event is seen during boot
> > before people log in.
>
> Agreed, but the USBguard daemon accepts commands from authorised users and
> acts on those commands. For example, blocking or unblocking access for a
> device just inserted. What key should be given in their msg string given
> the initiating user is not root (or unset). At the moment, they don't log
> this detail but I will ask them to, so want to advise the key to use.

sauid is used for second-hand information. It is not considered trustworthy 
since the kernel isn't the source of the identity. We need their subject 
label as well.

And if you are talking to them, I do not believe it is proper to log the 
actual rule that they are triggering on. This causes a lot of hex-encoded 
text that is meaningless.

-Steve