Bug 256233 - security/doas: target user's login class gets ignored
Summary: security/doas: target user's login class gets ignored
Status: New
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-ports-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-29 02:36 UTC by bugs.freebsd
Modified: 2021-06-06 23:19 UTC (History)
2 users (show)

See Also:
jsmith: maintainer-feedback+


Attachments
login.conf used for testing (7.19 KB, text/plain)
2021-06-05 01:49 UTC, bugs.freebsd
no flags Details
doas.conf test file (27 bytes, text/plain)
2021-06-05 01:51 UTC, bugs.freebsd
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description bugs.freebsd 2021-05-29 02:36:30 UTC
## Environment

The bug was found on FreeBSD 12.2-RELEASE GENERIC amd64 with doas-6.3p4 installed. But it will most likely manifest itself on all FreeBDS versions and architectures.

The system has 2 regular user accounts named alice and bob. Both accounts belong to a non-default login class, and alice is allowed to use doas to execute commands as bob.


## Problem description

When running a command with doas, the target user's login class is ignored. The capabilities from the default login class are applied instead of the ones defined in the target user's class.

For example:
$ doas -u bob ulimit -a
Shows limits as defined for the default login class instead of those defined for bob's login class.
$ doas -u bob locale
Prints the locale from the default login class instead of the one defined in bob's actual login class.

Expected behaviour:
$ doas -u bob ulimit -a
Show the limits that are defined in /etc/login.conf for bob's login class.
A quick test seems to affirm that this is what happens on OpenBSD.
$ doas -u bob locale
Show the locale defined in /etc/login.conf for bob's login class.


## How to reproduce

Add login classes 'alice' and 'bob' to login.conf. Each off the classes has a different language and memory limit:
# diff -u login.conf.orig login.conf
--- login.conf.orig     2021-05-29 01:45:22.537277000 +0200
+++ login.conf  2021-05-29 02:59:28.979606000 +0200
@@ -49,6 +49,16 @@
        :ignoretime@:\
        :umask=022:
 
+alice:\
+       :lang=en_CA.UTF-8:\
+       :memoryuse=8G:\
+       :tc=default:
+bob:\
+       :lang=en_US.UTF-8:\
+       :memoryuse=4G:\
+       :tc=default:

Create users alice and bob, each belonging to their respective login class.

A single line is added to doas.conf:
permit nopass alice as bob


Log in as user alice, and check that her memory limit and language are applied:
alice@doas-bug:~ $ ulimit -m
8388608
alice@doas-bug:~ $ locale | grep LANG
LANG=en_CA.UTF-8

Now run the same commands with doas, and observe that the limit and language are taken from the default login class:
alice@doas-bug:~ $ doas -u bob ulimit -m
unlimited
alice@doas-bug:~ $ doas -u bob locale | grep LANG
LANG=

The expected result is that limits from class 'bob' are be applied (as happens on OpenBSD).
Logging in as bob shows the expected limits:
bob@doas-bug:~ $ ulimit -m
4194304
bob@doas-bug:~ $ locale | grep LANG
LANG=en_US.UTF-8
Comment 1 Jesse Smith 2021-05-29 15:15:49 UTC
I've looked into this a little bit and confirmed the behaviour described in the issue report is reproducible on my FreeBSD machines. The login class of the target user is indeed ignored when using "doas -u username". Thank you for the detailed problem description.

While looking into this I gave some thought as to how doas is working, particularly in comparison to other tools. For instance, when I run the "su" command to run a shell or command as another user, it behaves the same way as doas.

As an example, if I run "su alice" and then run "ulimit -a" as Alice her login class limits are ignored. However, if I run "su - alice" it triggers an effective wipe/fresh login action and when I run "ulimit -a" as Alice then her login class is in effect.

In short, at least where the "su" utility is concerned, login classes only seem to matter when we're forcing a fresh login, not when we're simply switching accounts/permissions to another user. The default behaviour of su on my FreeBSD machine is to ignore the login class (and limits) of the user.

Which makes me wonder which behaviour makes more sense for doas? Should it act like the default "su" behaviour and simply switch user permissions or should it act as though it is performing a fresh login for the new user?

While considering this I noticed that running "doas -S -u alice", which is supposed to simulate a fresh login, also doesn't respect the user class and /etc/login.conf settings. Which makes me think the best way to approach this would be to:

1. Leave the default behaviour as it is, to match su.
2. Fix "doas -S" to properly simulate a full login, similar to running "su -" which would appear to best match the upstream doas manual page.

I'm open to thoughts and feedback on this idea.
Comment 2 bugs.freebsd 2021-05-30 01:57:10 UTC
Thanks for the quick reply. I've done a little bit of extra testing, and came up with some points and opinions.

Regarding su, doas actually doesn't behave in the same way as su in my tests. While su also ignores the target user's login class, it does keep the capabilities of the caller intact. Conversely, doas applies capabilities from the "default" class.
To demonstrate that su just leaves the caller's limits intact:
alice@doas-bug:~ $ su bob -c 'ulimit -m'
Password:
8388608
alice@doas-bug:~ $ su bob -c 'locale | grep LANG'
Password:
LANG=en_CA.UTF-8

## On OpenBSD

To compare what happens on OpenBSD, I have done a clean install of OpenBSD 6.9 (in a VM) with similar limits configured for alice and bob. This yields some interesting results:
alice@openbsd $ ulimit -m
8388608
alice@openbsd $ su bob -c 'ulimit -m'
4194304
alice@openbsd $ doas -u bob ulimit -m
doas: ulimit: command not found
alice@openbsd $ doas -u bob sh
bob@openbsd $ ulimit -m
4194304

1. In contrast to FreeBSD, su also applies limits from the target user's login class. This difference in behaviour came somewhat as a surprise to me.
2. Interestingly, OpenBSD doesn't let you run "doas ulimit" (another surprise). But "doas -u bob sh" seems to indicate that limits from bob's login class are applied.
3. On OpenBSD, behaviour between doas and su is consistent.

Note: apparently OpenBSD doesn't let you configure "lang" in /etc/login.conf, so it has been omitted here.

## My opinion

All things considered, this is my take on the issue:
* By default, doas should either apply the target user's login class (as is the case on OpenBSD) or keep the capabilities of the caller (like su on FreeBSD). In the current situation doas always applies the "default" login class, which I'd consider incorrect in any case.
* For me personally, I'd prefer the same behaviour as on OpenBSD (apply target user's login class). A clear downside of this approach is that it would make doas inconsistent with su. But in my opinion, OpenBSD's su behaviour also makes more sense (apply target user's limits) compared to FreeBSD.
* I do not consider it likely that the behaviour of su on FreeBSD will be changed anytime soon, so changing doas to just keep the callers capabilities seems like a very reasonable thing to do. This way, doas and su work consistently. Depending on who you'd ask, this could be intended behaviour or still not correct.
* As for "doas -S", I think it should indeed be identical to "su -l".

This whole issue does make me wonder about the proper/intended behaviour of su.
Comment 3 Jesse Smith 2021-05-30 15:37:41 UTC
Thank you for looking into this further.

I did some looking about in the code and manual pages and discovered the issue. FreeBSD has a "class" field in the password structure which doesn't exist in other supported platforms. Since it's a FreeBSD-ism it wasn't a field which was set/checked in the code. Which meant when class resources were being set in setusercontext() the class field would be blank and the system would just set the defaults.

This has been changed upstream. When getting the password data doas will now check if it is running on FreeBSD and, if so, copy the class field and use it when applying login rules/limits.

I've tested this on FreeBSD 12.2 and confirmed restrictions like max memory usage are being applied. The fix is now in the GitHub repo: https://github.com/slicer69/doas

This was a small fix, just two lines in two files (doas.c and env.c). If you could give the fixed code a test run and confirm it's using the proper limits from the target that would be very helpful. Assuming it works and I don't run into any problems on my other test systems, I'll publish a new version with the fix.
Comment 4 bugs.freebsd 2021-05-31 03:15:30 UTC
I've tried your fix and can confirm that resource limits are now being applied according to the target user's login class. However, it still sets the locale ("lang") from the default login class (also when using "doas -u bob -S").

To get a few more data points, I've experimented a bit with login class settings other than the resource limits. Here are some results:
:lang:      Gets reset to the default class.
:welcome:   Shows the default motd when using "doas -S", not the one specified.
:umask:     The umask from the target users login class is honoured (the expected result).
:setenv: and :path: are ignored as far as I can tell. But this is expected since environment variables are explicitly handled by doas.

This suggests there might be other login class capabilities which are reset to the default class. I haven't tried out modifying things like mail, shell, timezone etc., but I guess they are mostly irrelevant (except maybe when using "doas -S").
Comment 5 Jesse Smith 2021-05-31 14:07:51 UTC
Since lang and welcome are both environment variables I'd expect them to be ignored (when using doas's keepenv flag) or wiped out if keepenv isn't specified in doas.conf. The calling user isn't likely to want to have their language setting suddenly change to the target user's when they run a command.

You are correct that setusercontext() can be used to set some environment variables to the target user's settings. According to the manual page lang, charset, timezone, and term are supported. (Looks like welcome/motd is not.) But I'm not sure it's a good idea to risk doing that as it means switching the language settings could make a mess of the terminal and input of the calling user.

Is there a reason why I'd want my language settings and/or timezone to change when running doas? It seems like it would cause more problems than offer solutions. But I'm open to the idea.
Comment 6 bugs.freebsd 2021-06-01 01:19:28 UTC
Calling doas with the keepenv flag should indeed keep the language settings from the caller intact. In the same vein, when a locale is explicitly provided though the setenv flag, it should be used.

We also seem to agree that the caller doesn't want 'unexpected' language changes.
But this is where it gets murky: if no language is passed to doas (by keepenv or setenv), the language is going to change nonetheless. Without keepenv, I see two possible outcomes:
1. The environment is cleared, and the command is executed using the "C" locale. This happens in the current situation (*). I'm not sure if this is what most people would expect.
2. The language from the target user's login class gets used. This is what initially made more sense to me, because I wasn't expecting the C locale.

(*) Initially, I thought it used the language defined in the default login class. In reality, it always seems to apply the C locale.


Now that I have given all this a bit more thought, this whole conundrum stems from the fact that specifying lang in login.conf is just a way to set a regular environment variable. Conversely, doas clears virtually all environment variables by design. But some essential environment variables will allways be set (regardless of keepenv and setenv), such as DOAS_USER, HOME, TERM, PATH and SHELL.
Now the big question is: should LANG also be in that list (i.e. considered essential)?

The way it is now, you'd have to explicitly specify "setenv { LANG }" to keep the caller's language or use "setenv { LANG=en_US.UTF-8 }" to match bob's language. When nothing is specified, you'll get the C locale.
Consider if the same thing happened with HOME or SHELL. My guess is that most people would argue those shouldn't be reset to the system default, but instead use the target user's shell.
Should or shouldn't LANG be handled in the same way as SHELL, TERM or HOME? I'm not saying it should, but it would definitely make sense to me. 


Another thing I noticed is that the default PATH is hardcoded in doas (on FreeBSD). I've checked how this is handled on OpenBSD, and there "doas -u bob printenv" yields the PATH as defined for bob's login class in /etc/login.conf. Do you know the reason why is's hardcoded on FreeBSD?

When I look at the behaviour of the original doas on OpenBSD, it appears that the environment from the target login class is always applied (with the caveat that OpenBSD lets you specify less options in login.conf compared to FreeBSD, "lang" being one of the missing ones). My guess is that the original design of doas was to respect environment variables defined in login classes, except where it really doesn't make sense. Maybe is's a good idea to ask upstream about this?
Comment 7 Jesse Smith 2021-06-01 14:20:33 UTC
I think preserving LANG probably makes sense, keeping the original caller's language settings. This probably got overlooked before since (as you pointed out) OpenBSD doesn't define LANG in login.conf and I never have LANG set on my FreeBSD machine. So it wasn't addressed originally in the code and it wouldn't have come up when I was porting.

I'll look at preserving LANG, like HOME and and SHELL, as a special variable. Off the top of my head, I don't think LANG can be used to do much harm. Technically, I suppose, there is a way to mess with something in another person's home directory using LANG, but if we have doas access to someone else's HOME then there are easier ways to cause mayhem.

The PATH is hard coded on all platforms of the port at compile time. I think originally OpenBSD's version hard coded the PATH right in a header file and it wasn't changeable at build time without patching the code. (I may be mistaken, but that is how I think it was set up.) Since each platform might use a slightly different default PATH this was edited to allow easier build-time changes.
Comment 8 Jesse Smith 2021-06-01 17:32:36 UTC
I made a minor change to the doas code so that LANG is now treated as a special environment variable and copied over to the target user's environment. This is now working on both FreeBSD and Linux. The variable is skipped if it isn't set in the caller's environment.

Assuming no testing reveals a problem, I'll publish a new update to doas on GitHub later this week and update the FreeBSD doas port to match.
Comment 9 bugs.freebsd 2021-06-01 23:33:09 UTC
I think it would be better to default to using the target user's language (per login.conf), and only copy the language from the caller when it's explicitly specified in setenv (or when keepenv is used).

The fix also doesn't catch the "doas -u bob -S" usecase. Simulating a full login should always result in the target user's language, but it never does. It results in either the caller's language, or when that isn't defined it still defaults to the C locale.
Comment 10 Jesse Smith 2021-06-01 23:50:16 UTC
I agree that when "doas -S" is used it makes sense to apply the login settings for the target user account. This has been addressed now upstream.

However, why would the user want the target user's language settings when they are not performing a complete login? That seems like it would be confusing. What's the use-case for applying the target user's language settings to the original user when they run doas?
Comment 11 bugs.freebsd 2021-06-02 02:45:50 UTC
When doas defaults to the target user's language, you can easily preserve the caller's language if you want to by using setenv in doas.conf. That is the way it works with most other environment variables (like HOME, for example).
With the current patch, LANG is essentially working the other way around. All other environment variables which are set by doas get their value from the target user, unless specified otherwise in doas.conf (keepenv/setenv). I would expect LANG to work in the same manner.

To give a few use-cases where you want the target user's language:
* You regularly have to work with documents/data in different languages, and use separate accounts and doas to do so.
* You're developing (or troubleshooting) applications in other languages.
* You're using FreeBSD as a desktop, and run heavy and/or risky (security-wise) applications as a seperate user. You want most of your desktop to use en_US, but for some applications you prefer your native language (this might be a text editors/word processors, mail clients, maybe a web browser). While I'll admit I'm making this up (not the separate user accounts), I can imagine some cases where this might make sense.
* A variation of the previous scenario. You run some software under different accounts (as a policy), but default to using your native language. However, the localization of certain programs is really bad, and you prefer to use a locale based on English for those.
* Last but not least: in most cases the calling and target user will probably use the same language anyway. If they differ, it's probably for a reason.
Comment 12 Jesse Smith 2021-06-02 13:39:45 UTC
I've given this some thought and decided LANG will stick with the original user unless the -S flag is specified. I work on some servers with people who have different languages/locales than I do and both myself (or they) would be completely lost if "doas <command>" or "doas -s" resulted in switching to the other user's language. For people who really want to switch languages they can either login to that user account (via desktop switching, su -, or doas -S). Nothing prevents them from still forcing a language switch if they really need it.
Comment 13 bugs.freebsd 2021-06-03 03:20:32 UTC
The suggestion to use "doas -S" or "su -" won't really work in many situations. You can't run arbitrary commands (e.g. when calling doas from a script) and you can't pass or modify environment variables. Simulating a full login is only useful in fairly specific circumstances (predominantly interactive shell work).

I'm not quite convinced that it's confusing when the language gets switched. In a way, that was already happening before the fix (always switching to the C locale because no LANG was set). But I guess we will have to agree to disagree on this point.

When I think about it, I might even prefer the old behaviour (LANG gets cleared) over the new one (default to setting the caller's LANG). The reason is simply that no other environment variable defaults to the caller's value. All others either get cleared or set to the value appropriate for the target user.

I think you should consider to either revert to the old behaviour (just reset LANG), or use the language from the target user. I believe using the caller's language is just not very favorable from a consistency standpoint.
For what it's worth, doas on OpenBSD clears the LANG environment (but it also doesn't let you configure "lang" in login.conf).
Comment 14 Jesse Smith 2021-06-03 14:52:13 UTC
I think you make a good point about the LANG variable being an exception. Virtually every other environment variable is discarded or set to the target user. I've reverted this change.

Now if the user wants to switch languages when switching users they can set LANG in the doas.conf file. Otherwise doas will use the system default (normally) or use the target's language if simulating a full login with "doas -S".

I've made this change upstream and reverted the doas.conf manual page to match.
Comment 15 bugs.freebsd 2021-06-04 01:07:24 UTC
Ok, this seems like a good solution.

One nitpick: the latest revision clears the language when using "doas -u bob -S". Other than that, I haven't found any issues.
Comment 16 Jesse Smith 2021-06-04 01:18:40 UTC
Can you please confirm you're using the latest source code from earlier today? I've tested it on my machines and in my case using "doas -S" sets the target's language (if they have one) or wipes it to default if none is set for the target user. So it should only clear the language entirely if the target user doesn't have one specified.
Comment 17 bugs.freebsd 2021-06-04 02:25:44 UTC
This is using revision 065b375. I rebuilt and reinstalled again to make sure, but with the same result: "doas -u someuser -S" sets the C locale regardless of the one defined in the login class.
Comment 18 Jesse Smith 2021-06-04 14:47:22 UTC
I can't duplicate that behaviour. On my test machine with a clean build of the same commit it's giving me the LANG defined in login.conf if one is set up for that user or the system default if one is not defined.
Comment 19 Jesse Smith 2021-06-04 15:10:52 UTC
If this is persisting, could you please send me the relevant part of the login.conf file and your doas.conf file? I want to make sure what I'm looking at matches your setup.
Comment 20 bugs.freebsd 2021-06-05 01:49:59 UTC
Created attachment 225559 [details]
login.conf used for testing

This file adds 2 login classes (alice and bob) with different limits and language. In addition to the new classes, a language has been added to the "default" login class.
Comment 21 bugs.freebsd 2021-06-05 01:51:00 UTC
Created attachment 225560 [details]
doas.conf test file

doas configuration used for testing. It's about as simple as it gets.
Comment 22 bugs.freebsd 2021-06-05 01:51:32 UTC
Sure. I primarily tested this in a clean jail (but also on the host system itself). I attached login.conf and doas.conf from the test jail.

For completeness, I build the port from a fresh ports checkout, with only 2 simple changes to the Makefile:
PORTVERSION=    6.3p7
GH_TAGNAME=     065b375

Also did another quick test with the latest commit (e0fb8d8) in a fresh jail, with the same results.
Comment 23 Jesse Smith 2021-06-05 15:49:29 UTC
Thanks for sharing the attachments. Looks like the difference between our configurations was I had KEEPENV specified in my configuration file so environment variables were getting copied over, rather than reset.

I've looked at this some more and decided having one login class variable (language) in this case be an exception to how things work doesn't really fit with how doas works. LANG (and other variables) are meant to be set in doas.conf rather than pulled in from other places.

So what I've ended up doing is allowing login.c to set resource limits on FreeBSD to avoid punching a hole in a user's resource restrictions. However, I'm going to leave setting environment variables up to the configuration of doas.conf.
Comment 24 bugs.freebsd 2021-06-06 01:39:45 UTC
Thanks for looking into it. I'm fine when the language is ignored with "normal" doas usage (without the -S flag). But I would consider it a bug when "doas -S" ignores it. I would expect "doas -S" to provide me with exactly the same environment as if I had used "su -" (which includes setting the language).

Fortunately for me, I personally don't really care what happens with "doas -S" (I never use the -S flag). But if you keep it as it is now, I'd like to suggest to document this behaviour in the manual page. Maybe something like:
BUGS: doas -S ignores the language set in login.conf and uses the system default instead.
Comment 25 Jesse Smith 2021-06-06 11:48:16 UTC
I disagree, the environment variables are disregarded in login.conf the same way they are elsewhere. This behaviour isn't a bug, it's trimming out environment variables.

I' open to the idea of documenting that -S simulates a login without adopting target user variables, but this appears to be by design (upstream) not unintended behaviour or a bug. In other words, the behaviour seems consistent across platforms.
Comment 26 bugs.freebsd 2021-06-06 18:36:35 UTC
Ok, maybe it's considered intended behaviour and not a bug. But it would be nice if this is mentioned in the manual page somehow, because the current description of -S doesn't make this clear at all. I think many people would expect it to be similar to "su -", when in fact it isn't. Right now I'm really at a loss of what the -S is supposed to do, and in what ways it is intended to be different from -s.

So I did a few more tests hoping to wrap my head around it. Until now, I've mostly been testing without the keepenv setting. However, when keepenv is defined in doas.conf, things get weird. Observe the difference in behaviour without and with keepenv:

permit nopass alice as bob
  In this case, "doas -u bob -S" resets all environment variables, and you end up with a very minimal environment.

permit nopass keepenv alice as bob
  With keepenv defined, "doas -u bob -S" actually results in a shell with all environment variables set according to bob's (!!!) login class. This includes the language! It also includes variables from the "setenv" line in login.conf.

With keepenv, doas -S is exactly doing what I always expected it do when keepenv is NOT set! What's happening here?


Regarding original design and consistency between platforms, I'd like to mention that upstream (doas on OpenBSD) doesn't even have the -S flag at all. Maybe there's a good reason they never included it.
Comment 27 Jesse Smith 2021-06-06 20:33:16 UTC
I believe I mentioned the other day that I was getting the target user's environment variables when KEEPENV is defined in the doas.conf file when performing "doas -S", so your test results are expected and match mine.

If you are curious about the idea behind the "-S" flag, it was added from this pull request: https://github.com/slicer69/doas/pull/36

The behaviour of -S is mentioned in the manual page now.
Comment 28 bugs.freebsd 2021-06-06 23:19:21 UTC
You indeed mentioned that in comment 23. I interpreted "copied over" as copied from the caller instead of the target. This is probably because until now I always assumed that keepenv was only meant to pass environment variables from the caller to the target (and have no real effect on doas -S).

In my opinion, this alternative meaning of keepenv (when using -S) remains a bit inconsistent. But the change in the manual page at least makes this explicit; it now makes clear what to expect when using -S. So as far as I'm considered this resolves the issue.

Thank you for looking into this (and for your maintainership in general).