mac_do: add a new MAC/do policy and mdo(1) utility

[bapt]

Summary

This policy enables a user to become another user without having to be
root (hence no setuid binary). it is configured via rules using sysctl
security.mac.do.rules

For example:
security.mac.do.rules=uid=1001:80,gid=0:any

The above rule means the user identifier by the uid 1001 is able to
become user 80
Any user of the group 0 are allowed to become any user on the system.

The mdo(1) utility expects the MAC/do policy to be installed and its
rules defined.

[ltning-freebsd_anduin.net]

How does this work for jailed processes/users? I may have missed out on something (a lot) since last I prodded the MAC framework, but IIRC the support for jail-specific rules is limited, at best?

The idea is good, we would quickly become users of such a feature, but only if it can be leveraged in a jailed environment..

[delphij]

The idea is interesting (I have never thought about a MAC policy to accomplish this goal) and I like its simplicity, please see comment inline for some minor nits.

I'm a little bit concerned about how this would interact with jails because it seems that the policy would also work for jails, but there is no way to specify different policies for them. It seems that it's not trivial to implement per-jail policy with the current MAC framework, but perhaps this could be implemented as blocking the request if inside jail by default with a knob to enable it?

share/man/man4/mac_do.4

12

sys/security/mac_do/mac_do.c

153–160

This doesn't seem right to me. In line 153, an unprotected read of rule_string would be performed, based on the context I believe what you should have done is something like:

	new_string = malloc(MAC_RULE_STRING_LEN, M_DO,
	    M_WAITOK|M_ZERO);
	mtx_lock(&rule_mtx);
	strcpy(new_string, rule_string);
	mtx_unlock(&rule_mtx);

        error = sysctl_handle_string(oidp, new_string, MAC_RULE_STRING_LEN, req);
	if (req->newptr == NULL)
		return (error));

and the error = sysctl_handle_string below should be gone.

usr.bin/mdo/mdo.1

4

usr.bin/mdo/mdo.c

5

[bapt]

Add jail support with jail parameter mdo.rules
Fix manpage

[bapt]

protext read

[bapt]

manpage fixes

[bapt]

fix copyright

[bapt]

copyright again

[bapt]

per jail support has been added

[lwhsu]

sys/security/mac_do/mac_do.c

4

[bapt]

another copyright

[des]

share/man/man4/mac_do.4

2

4

We normally place the copyright first and the tag below, cf. https://docs.freebsd.org/en/articles/license-guide/

20

(no paragraph break needed before a new section)

24

25

32

47

48

51

54

55

60

61

Please add an EXAMPLES section.

sys/security/mac_do/mac_do.c

2

5

We normally place the copyright first and the tag below, cf. https://docs.freebsd.org/en/articles/license-guide/

23

needs sorting

194

209

243

254

Here you unlock ppr while pr is still locked. Technically there's no rule against that but it seems odd.

259

In every case where you call this with non-null lrp, you unlock pr immediately on return, so why not just unlock it here?

307

354

403

Isn't this redundant? It's global, it will be initialized to all-zeroes by the loader.

536

usr.bin/mdo/mdo.1

2

See above.

usr.bin/mdo/mdo.c

2

See above.

58

62

64

67

[bapt]

Address all of des@ points

[bapt]

I think I have addressed all the @des points

[des]

share/man/man4/mac_do.4

24

You dropped “other” before the second “users”, was that intentional?

61

missing “and”

69

78

79

80

81

sys/security/mac_do/mac_do.c

235

I find these names confusing. Do rules and nrules actually point to several rules, or just one (each)? And I understand that nrules means “new rules” but I keep reading it as “number of rules” so perhaps it would be better not to abbreviate “new”.

265

265

343

391

According to style(9), pr should go below methods, but I won't insist if you prefer it this way around.

392

This could be static. I'm not sure it matters. (I would also suggest adding const to the osd_register() prototype, but that's not your fault.)

[bapt]

sys/security/mac_do/mac_do.c

235

rules points to several rules 0-N rules in a tailq

[bapt]

Address the new set of comments from @des