USBguard bug

USBguard bug

[Burn Alting]

[-- Attachment #1.1: Type: text/plain, Size: 3225 bytes --]

All,

I need some advice.

Currently when the USB management framework, usbguard (
https://github.com/USBGuard/usbguard),  is building it's key-value pairs prior 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 any character  in the value matches
the expression (str[i] == '"' || str[i] < 0x21 || str[i] == 0x7F). This can be found
in https://github.com/USBGuard/usbguard/blob/master/src/Daemon/LinuxAuditBackend.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

type=USER_DEVICE msg=audit(1580255002.606:352190): pid=3115 uid=0 auid=4294967295
ses=4294967295 subj=system_u:system_r:unconfined_service_t:s0 msg='op="changed-
authorization-state-for" device="/devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.3"
target="allow"
device_rule=626C6F636B20696420303738313A353539312073657269616C2022344335333030303132
323034313231303533313322206E616D652022556C7472612055534220332E3022206861736820227953
6D433045594970734A575666474436414854774577712F624974344631466A78785856306C3552356B3D
2220706172656E742D6861736820226B763376322B726E713951765949332F48624A314556397664756A
5A30615643512F43474259496B4542303D22207669612D706F72742022312D312E332220776974682D69
6E746572666163652030383A30363A3530 exe="/usr/sbin/usbguard-daemon" hostname=? addr=?
terminal=? res=success'\x1dUID="root" AUID="unset"
where device_rule started as
	block id 0781:5591 serial "4C530001220412105313" name "Ultra USB 3.0" hash
"ySmC0EYIpsJWVfGD6AHTwEwq/bIt4F1FjxxXV0l5R5k=" parent-hash
"kv3v2+rnq9QvYI3/HbJ1EV9vdujZ0aVCQ/CGBYIkEB0=" via-port "1-1.3" with-interface
08:06:50
or

type=USER_DEVICE msg=audit(1580255002.605:352187): pid=3115 uid=0 auid=4294967295
ses=4294967295 subj=system_u:system_r:unconfined_service_t:s0 msg='op="discovered-
device" device="/devices/pci0000:00/0000:00:1d.0/usb2/2-1"
device_rule=616C6C6F7720696420383038373A303032342073657269616C202222206E616D65202222
206861736820225A78377630464D51456A53634B534146454E41696F624573314F47505042305957522B
79584443564530343D2220706172656E742D68617368202257484254784E61456F4D474E534E6333314B
70464E53416546463448624C4D51675342714F526C433653383D22207669612D706F72742022322D3122
20776974682D696E746572666163652030393A30303A3030 exe="/usr/sbin/usbguard-daemon"
hostname=? addr=? terminal=? res=success'\x1dUID="root" AUID="unset"
where device_rule started as
	allow id 8087:0024 serial "" name "" hash
"Zx7v0FMQEjScKSAFENAiobEs1OGPPB0YWR+yXDCVE04=" parent-hash
"WHBTxNaEoMGNSNc31KpFNSAeFF4HbLMQgSBqORlC6S8=" via-port "2-1" with-interface
09:00:00

I have a number of questions
- What is the best recommendation I can make in a bug report I'd like to raise so
that the auparse library can reliably interpret all their key's values?
- Should I also request they actually provide hostname and addr values to
audit_log_user_message()?
- If one want them to identify the user who participates in the activity what is the
best recommendation to make in terms of keys in the message?

Thanks in advance



[-- Attachment #1.2: Type: text/html, Size: 3801 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: USBguard bug

[Steve Grubb]

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 pairs
> prior 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 any
> character  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/LinuxAuditBack
> 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 to
> raise so that the auparse library can reliably interpret all their key's
> values?

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. 


> - Should I also request they actually provide hostname and addr
> values 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 activity
> what 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.

-Steve

Re: USBguard bug

[Burn Alting]

[-- Attachment #1.1: Type: text/plain, Size: 2760 bytes --]

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/LinuxAuditBack
> > 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?Do we
need to add usbguard specific keys to auparse/typetab.h?
> > - 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.
> -Steve
> 

[-- Attachment #1.2: Type: text/html, Size: 4728 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: 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