Recently I was trying to convert some "echo a.b.c=1 > /etc/sysctl.conf" type statements to use sysrc instead. It appears if the variable contains one or more dots, sysrc will write the desired value, fail to read it, and it will always create a new entry if executed again. This is inconvenient for editing values in /etc/sysctl.conf using the -f parameter because such entries almost always have a dot in the variable. sysrc -f /etc/sysctl.conf Symptoms are present on FreeBSD 9 too. How-To-Repeat: root@build10:~ # grep a.b /etc/rc.conf root@build10:~ # sysrc a.b=yes a.b: -> root@build10:~ # grep a.b /etc/rc.conf a.b="yes" root@build10:~ # sysrc a.b=yes a.b: -> root@build10:~ # grep a.b /etc/rc.conf a.b="yes" a.b="yes"
Responsible Changed From-To: freebsd-bugs->dteske Over to maintainer.
While I can agree that a tool *like* sysrc is needed for editing files like /etc/sysctl.conf (and /boot/loader.conf), sysrc is not that tool. The tool for that job is being developed and is named "sysconf". Here is the code which is written in C and is almost functional (can read but not yet write out changes): http://druidbsd.cvs.sf.net/viewvc/druidbsd/sysconf/ The sysconf tool will have a syntax that is nearly identical to sysrc. The break-down between functionality that requires two separate utilities is as-such: sysrc is written in shell and uses shell to parse shell (rc- files are actually shell). sysconf is written in C and uses C to parse text while keeping to the rules that are used by loader.4th to parse loader.conf (which happens to be slightly more strict but compatible with how sysctl.conf is parsed). The tool should fit the job. While the bigger picture is to wait for sysconf, would you be interested in a patch that actively aborts sysrc if you attempt to pass a variable that appears invalid? I'm 50% looking for a POLA violation to say we should definitely filter input, and 50% saying we shouldn't stop the user from doing something that appears incorrect because we could potentially prevent them from doing something clever. As it stands, I fully expect the error that comes from asking sh(1) to expand a variable that contains dots. -- Devin _____________ The information contained in this message is proprietary and/or confidential. If you are not the intended recipient, please: (i) delete the message and all copies; (ii) do not disclose, distribute or use the message in any manner; and (iii) notify the sender immediately. In addition, please be aware that any message addressed to our domain is subject to archiving and review by persons other than the intended recipient. Thank you.
On 03/19/2014 18:28, dteske@FreeBSD.org wrote: > While I can agree that a tool *like* sysrc is needed for > editing files like /etc/sysctl.conf (and /boot/loader.conf), > sysrc is not that tool. Fair enough. I'll admit I've used it to modify /etc/periodic.conf as well, but that is probably similar enough. > > The tool for that job is being developed and is named > "sysconf". Here is the code which is written in C and is > almost functional (can read but not yet write out changes): > > http://druidbsd.cvs.sf.net/viewvc/druidbsd/sysconf/ > > The sysconf tool will have a syntax that is nearly identical > to sysrc. The break-down between functionality that > requires two separate utilities is as-such: > > sysrc is written in shell and uses shell to parse shell (rc- > files are actually shell). > > sysconf is written in C and uses C to parse text while > keeping to the rules that are used by loader.4th to > parse loader.conf (which happens to be slightly more > strict but compatible with how sysctl.conf is parsed). > > The tool should fit the job. That sounds like it would probably be faster to execute too. > > While the bigger picture is to wait for sysconf, would you > be interested in a patch that actively aborts sysrc if you > attempt to pass a variable that appears invalid? > > I'm 50% looking for a POLA violation to say we should > definitely filter input, and 50% saying we shouldn't stop > the user from doing something that appears incorrect > because we could potentially prevent them from doing > something clever. > > As it stands, I fully expect the error that comes from > asking sh(1) to expand a variable that contains dots. > I feel torn, part of me is satisfied with the explanation of "it's meant for shell-compatible variables only" (maybe a more explicit mention of that in the manpage would help) but part of me is still bothered that it silently makes a modification to the specified file while visually indicating that it did not. I was probably further drawn into the allure because sysrc(8) states "This utility works similar to sysctl" :) sysrc(8) also makes claims of appending only once and "taking care not to allow the file to grow unwieldy should sysrc be called repeatedly" which is not holding true in this circumstance. Is it possible to withhold the file editing until sysrc has executed far enough along that it realizes it thinks it is trying to set a null variable and abort? Thanks for your attention.
For bugs matching the following criteria: Status: In Progress Changed: (is less than) 2014-06-01 Reset to default assignee and clear in-progress tags. Mail being skipped
(In reply to ebay from comment #3) When I went to add the code to block parameters matching invalid variable names, it simply wasn't possible due to documented support for parameter expansion. From the sysrc(8) manual: In addition to the above syntax, sysrc also supports inline sh(1) PARAMETER expansion for changing the way values are reported, shown below: sysrc 'hostname%%.*' returns $hostname up to (but not including) first `.'. sysrc 'network_interfaces%%[$IFS]*' returns first word of $network_interfaces. sysrc 'ntpdate_flags##*[$IFS]' returns last word of $ntpdate_flags (time server address). sysrc usbd_flags-"default" returns $usbd_flags or default if unset or NULL. sysrc cloned_interfaces+"alternate" returns alternate if $cloned_interfaces is set. I will have to build an exclusion list based on possible shell expansion parameter possibilities to get at the actual variable name for testing.
A commit references this bug: Author: dteske Date: Mon May 28 23:34:23 UTC 2018 New revision: 334303 URL: https://svnweb.freebsd.org/changeset/base/334303 Log: sysrc(8): Test variable names for invalid characters PR: bin/187461 Reported by: ebay@looksharp.net MFC after: 4 weeks X-MFC-to: stable/11 (after 11.2-R) Sponsored by: Smule, Inc. Changes: head/usr.sbin/sysrc/sysrc
Here's what sysrc does now: $ sysrc ifconfig.ix0=abc sysrc: ifconfig.ix0: name contains characters not allowed in shell All examples in the manual were tested to make sure no regressions were introduced. I'll keep this bug open for the next 4 weeks until I merge back to stable/11 (after which I'll close this bug)
This change broke my bootstrap scripts[1] but from what I read in this thread it was unavoidable. Is there a dedicated tool in the base for setting things like hint.hdaa.1.nid22.config="as=1 seq=15"? Apart from grep and echo of course :) [1]: https://github.com/0mp/dotfiles/blob/master/freebsd-yoga314/setup.root#L11-L28
(In reply to Mateusz Piotrowski from comment #8) Would it help if I made the check a warning instead of fatal?
(In reply to Devin Teske from comment #9) I am not sure if this is a good idea. Unless sysrc is officially supporting a.b.c.d-like variables I am not going to use it in such situations unfortunately. This undefined behavior might be broken anytime. Previously, I thought that sysrc is the tool to edit all those somewhat shell-looking configuration files on FreeBSD. Now that I know it's not I'll have to look for a substitute. Actually, maybe we could add a flag which turns on support for a.b.c.d-like variables? Or even more generally: a flag which adds additional characters to VALID_VARNAME_CHARS. (Unless we're migrating to UCL with all those config files soon... :) ) What do you think?
(In reply to Mateusz Piotrowski from comment #10) sysrc doesn't just edit files that look like shell, but files that are in-fact shell. This is to explain that if a file has: foo=abc bar=$foo sysrc knows that at the time the operating system boots, bar is in-fact abc, and if queried ("sysrc bar") displays "foo: abc". It was important for sysrc to embrace the fact that /etc/rc.conf and friends are shell rather than ignore it and hope nobody writes any shell code in them. This invariably, as you have noticed, makes sysrc a poor choice for editing any file that either is not in-fact shell or fails to adhere to shell restrictions (e.g., /etc/sysctl.conf). sysrc is written in shell and it operates on shell. By that reasoning, the right tool for sysctl.conf and friends should be written in C. I personally am not a fan of trying to parse every file with libucl nor am I a fan of converting existing configuration file formats to appease a parser. I am also not a fan of complex parsers such as yacc/bison. My solution to the problem is to leverage existing work that I already have in base: https://svnweb.freebsd.org/base/head/lib/libfigpar/ That's the first half of two requirements to solve the problem in C with BSD-licensed code. figpar above is the generalized configuration parser (man page below): https://www.freebsd.org/cgi/man.cgi?query=figpar I have already incorporated figpar into a tool called "sysconf" which can query values. The missing piece of the puzzle, the other half required, is me to write libfigput as the library to supply the generalized configuration writer for rewriting an existing config with new values. I prefer this approach because it is super light-weight and super portable. The figpar code was actually written by me on Debian Linux 2.2 (code named "Potato") and I ported it to FreeBSD after I became a FreeBSD developer; but I also had access to the [long-since shutdown] SourceForge compile farm back then and so I know figpar works on every platform I could get my hands on. That's the same aim that I have with figput and sysconf.
If OK, I am going to merge 334303 to stable/11, after which I will close this bug
(In reply to Devin Teske from comment #12) +1
A commit references this bug: Author: dteske Date: Wed Jun 20 06:24:03 UTC 2018 New revision: 335409 URL: https://svnweb.freebsd.org/changeset/base/335409 Log: MFC r334303: sysrc(8): Test variable names for invalid characters PR: bin/187461 Reported by: ebay@looksharp.net Sponsored by: Smule, Inc. Changes: _U stable/11/ stable/11/usr.sbin/sysrc/sysrc
Thank you so very much for your extended patience in awaiting resolution.
*** Bug 215984 has been marked as a duplicate of this bug. ***