mac_do: Log credentials switching

[otis]

Summary

Log switching of UID and GID from within mac_do

[bapt]

sys/security/mac_do/mac_do.c

51

I think I should call it verbose or something like this rather than log

1987

I would prefer to see the command name or command path, like mdo(pid)

[olce]

Please see the inline comments, the printing is currently misplaced.

Also, please keep in mind that mac_do(4) can be configured per jail (well, only the rules for now). I'm not really sure about what/how we should log info when the request originates from a jail. I'm having the same dilemma with security.mac.do.print_parse_error. Perhaps @jamie could shed a light on that?

On a not-directly-related note, I should commit an updated man page for mac_do(4) soon(tm), sorry for this delay.

sys/security/mac_do/mac_do.c

51

Yes, log is too imprecise, for example, we already log for rules parse errors.

If what you want is to log all invocations, then perhaps log_invocations?

1983–1987

This is not the right place to do that because:

• There may be multiple rules, and it's not known in advance which one will authorize the transition, but currently the message will be repeated for all the preceding ones.
• It may be that no rule will authorize the transition, in which case this prints (as many times as there are rules) a message for a transition that doesn't happen in the end.

If you want to log only in case of success, you have to move this code under a new if (error == 0) test below (e.g., before if (error != EPERM)), making sure that we break out of the loop at the end of that branch.

If you want to log all invocations, even those that fail, then instead please move the printf() outside of the loop near the return and log the return value as well (you'll have to tweak the control flow to ensure there's only one exit point).

1984–1985

For consistency with the rest of the file, please capitalize the word after : in the message.

1984–1987

The printed message lacks real and saved UIDs and GIDs and all supplementary groups.

Also, as setcred() only operates on the parts of credentials based on the passed flags, you should gate printing with these flags, i.e., don't show a transition of (effective) UID iff SETCREDF_UID was passed (I think that testing if UIDs have changed wouldn't be enough, at we want to see requested changes even if in the end the UID is the same, e.g., to spot programming errors).

[otis]

• Print also process full path
• Tweak manpage a bit

[olce]

Could you please move the printing (see inline comment)? It is still misplaced.

sys/security/mac_do/mac_do.c

51

verbose is too imprecise as well. Could you use log_invocations, or log_requests (or something similar) instead?

[otis]

In D48129#1097601, @olce wrote:

Could you please move the printing (see inline comment)? It is still misplaced.

Yes, in next couple of whiles.

[olce]

In D48129#1097603, @otis wrote:

Yes, in next couple of whiles.

Great. Since our last messages appeared to cross, just wanted to be sure you saw the previous message. Thanks!

[otis]

It seems to me that there is another problem, or that I'm missing something obvious:

The line from manpage:

# sysctl 'security.mac.do.rules=uid=1000:80,gid=0:any'
security.mac.do.rules:
sysctl: security.mac.do.rules=uid=1000:80: Invalid argument

gives the following error:

kernel: MAC/do: Parse error at index 9: No valid type found.

[olce]

In D48129#1097617, @otis wrote:

It seems to me that there is another problem, or that I'm missing something obvious:

The line from manpage:

# sysctl 'security.mac.do.rules=uid=1000:80,gid=0:any'
security.mac.do.rules:
sysctl: security.mac.do.rules=uid=1000:80: Invalid argument

gives the following error:

kernel: MAC/do: Parse error at index 9: No valid type found.

The rules syntax has been substantially changed, this is why I wrote this above:

In D48129#1097525, @olce wrote:

On a not-directly-related note, I should commit an updated man page for mac_do(4) soon(tm), sorry for this delay.

I will upload an up-to-date manual page by the end of the week.

In the meantime you can find details in the code comments preceding parse_*() functions in mac_do.c.

Another, this time very small, change to it is going to happen soon (basically, changing one delimiter).

Currently, the example you gave converts to:
security.mac.do.rules=uid=1000:uid=80,gid=*,+gid=*;gid=0:any
in the new syntax.

[olce]

In the meantime, there's also D47616 which has lots of examples.

[olce]

Just for the record, the manual page was submitted in D48153 and has just been committed (bc201841d139).