Bug 236204 - Allow comma-separated values for setenv in login.conf
Summary: Allow comma-separated values for setenv in login.conf
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: conf (show other bugs)
Version: 12.1-STABLE
Hardware: Any Any
: --- Affects Some People
Assignee: Sean Eric Fagan
URL: https://reviews.freebsd.org/D25368
Keywords: feature, needs-qa
Depends on:
Blocks:
 
Reported: 2019-03-04 12:11 UTC by Michael Osipov
Modified: 2021-07-04 07:23 UTC (History)
4 users (show)

See Also:
koobs: mfc-stable13?
koobs: mfc-stable12?


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Osipov 2019-03-04 12:11:33 UTC
I cannot set the following in login.conf

> :setenv=MAIL=/var/mail/$,BLOCKSIZE=K,\
> HTTP_PROXY=http\c//194.145.60.1\c9400,\
> HTTPS_PROXY=http\c//194.145.60.1\c9400,\
> FTP_PROXY=ftp\c//194.145.60.1\c9400,\
> NO_PROXY="localhost,.siemens.net,.siemens.com,.siemens.de":\

because comma seperates the vars themselves.

While fetch(1) and curl(1) also supports whitespace-delimited values, poudriere chokes on: https://github.com/freebsd/poudriere/issues/669

There must be some mechanism to escape commas just like semicolons.
Comment 1 Bryan Drewery freebsd_committer 2019-03-04 19:37:32 UTC
Poudriere is effectively fine here with this syntax (no quotes needed):
        :setenv=MAIL=/var/mail/$,BLOCKSIZE=K,NO_PROXY=localhost .siemens.net .siemens.com .siemens.de:\
Comment 2 Michael Osipov 2019-03-05 09:15:03 UTC
I cannot confirm this. Besides poudriere, it whould be possible in general to have commas in env values.
Comment 3 Michael Osipov 2020-06-16 08:43:10 UTC
I think I have traced down the cause: cap_mkdb uses cgetnext() whose manpage says:
>      String capability values may contain any character.  Non-printable ASCII
>      codes, new lines, and colons may be conveniently represented by the use
>      of escape sequences:
> 
>      ^X        ('X' & 037)          control-X
>      \b, \B    (ASCII 010)          backspace
>      \t, \T    (ASCII 011)          tab
>      \n, \N    (ASCII 012)          line feed (newline)
>      \f, \F    (ASCII 014)          form feed
>      \r, \R    (ASCII 015)          carriage return
>      \e, \E    (ASCII 027)          escape
>      \c, \C    (:)                  colon
>      \\        (\)                  back slash
>      \^        (^)                  caret
>      \nnn      (ASCII octal nnn)

While I have tried to do \054, it is still not working because the setenv magic is performed with login_getcaplist() in
> ./lib/libutil/login_class.c:    const char      **set_env = login_getcaplist(lc, "setenv", ",");

So also the \054 is converted to comma, the subsequent call adds its own sematics to split the setenv capability with comma. So it would require something like \O which is used in login_class only and
 if ((np = substvar(p, pwd, hlen, pch, nlen)) != NULL) {
     setenv(*set_env, np, 1);
     free(np);
 }

and np needs to be processed for \O to replace with a comma. If I understand ./lib/libc/gen/getcap.c correctly the blackslash is retained by "*mp++ = *(bp-1)" if the escape char is unknown.
Comment 4 Sean Eric Fagan freebsd_committer 2020-06-20 00:12:30 UTC
Ok, on HEAD, I've got a change that, for that login.conf entry, gives me:

	0: MAIL=/var/mail/$
	1: BLOCKSIZE=K
	2: 	HTTP_PROXY=http://194.145.60.1:9400
	3: 	HTTPS_PROXY=http://194.145.60.1:9400
	4: 	FTP_PROXY=ftp://194.145.60.1:9400
	5: 	NO_PROXY=localhost,.siemens.net,.siemens.com,.siemens.de

as the output.  Specifically, it treats a quoted string as a quoted string, and then removes the quotes.  I'll create a review for it.
Comment 5 Sean Eric Fagan freebsd_committer 2020-06-20 00:24:50 UTC
I created review D25368
Comment 6 Michael Osipov 2020-06-20 09:34:48 UTC
Sean, thanks for the patch. I will try that next week and let you know!
Comment 7 Michael Osipov 2020-06-22 15:03:26 UTC
I cannot unfortunately sign in into Phabricator:

Fehler 401: disabled_client
The OAuth client was disabled.

I have tested your patch. Set up a jail from stable/12 with Poudriere applied patch, recompiled:

> root@deblndw011x:/var/poudriere/jails/121-stable-amd64/usr/src
> # jexec 121-stable-amd64-default  env -i TERM=$TERM /usr/bin/login -fp root
> Last login: Mon Jun 22 14:51:48 on pts/2
> FreeBSD ?.?.?  (UNKNOWN)
> 
> root@121-stable-amd64-default:~ # less /etc/login.conf
> # login.conf - login class capabilities database.
> #
> ...
> default:\
>     :passwd_format=sha512:\
>     :copyright=/etc/COPYRIGHT:\
>     :welcome=/etc/motd:\
>     :setenv=BLOCKSIZE=K,UNAME_r=12.1-STABLE,UNAME_v=FreeBSD 12.1-STABLE 1201518,OSVERSION=1201518,\
> LSCOLORS=gxfxcxdxbxegedabagacad,\
> CLICOLOR=YES,\
> EDITOR=vim,\
> LESS=-x4 -R -eFK,\
> NCURSES_NO_UTF8_ACS=1,\
> HTTP_PROXY=http\c//194.145.60.1\c9400,\
> HTTPS_PROXY=http\c//194.145.60.1\c9400,\
> FTP_PROXY=http\c//194.145.60.1\c9400,\
> NO_PROXY="localhost,.siemens.net,.siemens.com,.siemens.de":\
>     :mail=/var/mail/$:\
>     :path=/sbin /bin /usr/sbin /usr/bin /usr/local/sbin /usr/local/bin ~/bin:\
>     :nologin=/var/run/nologin:\
>     :cputime=unlimited:\
>     :datasize=unlimited:\
>     :stacksize=unlimited:\
>     :memorylocked=64K:\
>     :memoryuse=unlimited:\
>     :filesize=unlimited:\
>     :coredumpsize=unlimited:\
>     :openfiles=unlimited:\
>     :maxproc=unlimited:\
>     :sbsize=unlimited:\
>     :vmemoryuse=unlimited:\
>     :swapuse=unlimited:\
>     :pseudoterminals=unlimited:\
>     :kqueues=unlimited:\
>     :umtxp=unlimited:\
>     :priority=0:\
>     :ignoretime@:\
>     :umask=022:
> 
> root@121-stable-amd64-default:~ # env
> USER=root
> LOGNAME=root
> HOME=/root
> SHELL=/bin/csh
> NO_PROXY=localhost,.siemens.net,.siemens.com,.siemens.de
> FTP_PROXY=http://194.145.60.1:9400
> HTTPS_PROXY=http://194.145.60.1:9400
> HTTP_PROXY=http://194.145.60.1:9400
> NCURSES_NO_UTF8_ACS=1
> LESS=-x4 -R -eFK
> EDITOR=vi
> CLICOLOR=YES
> LSCOLORS=gxfxcxdxbxegedabagacad
> OSVERSION=1201518
> UNAME_v=FreeBSD 12.1-STABLE 1201518
> UNAME_r=12.1-STABLE
> BLOCKSIZE=K
> MAIL=/var/mail/root
> PATH=/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin:/root/bin
> TERM=xterm
> HOSTTYPE=FreeBSD
> VENDOR=amd
> OSTYPE=FreeBSD
> MACHTYPE=x86_64
> SHLVL=1
> PWD=/root
> GROUP=wheel
> HOST=121-stable-amd64-default
> PAGER=less

Works for me. It needs a manpage update to tell that quotes will help for commas or maybe other characters affecting parsing.
Comment 8 Michael Osipov 2021-01-10 17:48:09 UTC
Can we merge this please?

My issue with Python Requests has been rejected: https://github.com/psf/requests/pull/5465 although I have provided sufficient justification.

koobs, can you help here?
Comment 9 Michael Osipov 2021-07-03 20:43:59 UTC
Can someone sponsor this?
Comment 10 Kubilay Kocak freebsd_committer freebsd_triage 2021-07-04 01:55:08 UTC
(In reply to Michael Osipov from comment #8)

Not sure how I can help here
Comment 11 Sean Eric Fagan freebsd_committer 2021-07-04 02:32:49 UTC
The PR needs to be updated (on me), reviewed (on others) and ideally tested.  I'm not quite sure how to make an automated test for it, so I'm open to suggestions.
Comment 12 Michael Osipov 2021-07-04 07:23:56 UTC
What I can offer is testing on 12-STABLE and 13-STABLE.

@koobs. I added you because this is Python relevant. Although libcurl accepts comma and space (not documented) my PR has been rejected and py-requests is too important.

At the end a core dev needs to at least review. I see no other reasonable way to set NO_PROXY in login.conf.