Page MenuHomeFreeBSD

/etc/rc add trace debug and verify
ClosedPublic

Authored by sjg on Jan 30 2024, 7:27 PM.
Tags
Referenced Files
Unknown Object (File)
Sun, Apr 28, 5:02 PM
Unknown Object (File)
Sun, Apr 28, 4:41 PM
Unknown Object (File)
Sun, Apr 28, 1:00 PM
Unknown Object (File)
Sun, Apr 28, 10:55 AM
Unknown Object (File)
Fri, Apr 26, 11:53 PM
Unknown Object (File)
Fri, Apr 26, 10:37 PM
Unknown Object (File)
Fri, Apr 26, 10:17 PM
Unknown Object (File)
Fri, Apr 26, 10:17 PM
Subscribers

Details

Summary

Debugging boot issues can be helped by
logging each rc.d script as it is run
and being able to selectively enable/disable set -x
debug.sh provides an elaborate framework for debugging shell scripts.

For secure systems, we want to be paranoid about what we read
during boot.

dot() simply reads (.) arg file if it exists
vdot() if mac_veriexec is active, ignore unverified files
otherwise behaves much the same as dot()
safe_dot() in safe_eval.sh allows reading an untrusted file;
limiting the input to simple variable assignments.

In load_rc_config allow caller to provide an option to indicate how to
handle its arg:
-v use vdot()
-s use sdot() which will try to use vdot() and fallback to safe_dot()
The default is to read using dot()

rc_run_scripts()
encapsulate the running of rc.d scripts
so that we can easily call it more than twice.

We vdot rc.subr.local to pick up extensions (like
run_rc_scripts_final) and overrides.

We also allow rc.subr.local or rc.conf to set rc_config_xtra
eg (rc_config_xtra=XXX for historic compatibility)

rc use set -o verify around the reading in of rc.subr
This has no effect if mac_veriexec is not active, but if it is; ensures
rc.subr has not been tampered with.

Sponsored by: Juniper Networks, Inc.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sjg requested review of this revision.Jan 30 2024, 7:27 PM
sjg created this revision.

Note: If the format of rc_log is deemed undesirable we could move it (and rc_trace) to rc.subr.local and just put a place holder in rc.subr

Check for -f as well as -s

The comments for rc.subr also apply to the new files.
What about a man page update how to turn on the debug feature? To my current understanding DEBUG_SH can be set interactively in the loader, and administratively in loader.conf... maybe also in rc.conf? So more than one man page to touch at least with a cross-ref.

libexec/rc/rc.subr
97–99

style: rc.subr uses "[ ... ] && " instead of "test ... &&".
Other places below are similar.

115

style: rc.subr uses "if [ ... ]" instead of "if test ...".

141–142

style (entire patch): rc.subr uses "for ...; do". The same for "while ... ; do".

176–177

something missing (return?) after && or line continuation (= missing indent) and line continuation character missing.

1627–1628

for better readability: actions in case statemements should be line seperated, not semicolon seperated, and the end of each case should be on its own line too, e.g.
---snip---
--arg)

_arg="$s"
shift 2
;;

---snip---

1635–1637

Same as above, and additionally missing indent for the case.

2493

Shouldn't this be "$_dot" instad of "vdot"?

Will have a go at the style issues.

As for man page updates - happy to once the patch is about ready.
debug.sh has its documentation within it (I have a tool that turns that comment into useable troff ;-)
Yes DEBUG_SH can be set pretty much anywhere, and can be augmented along the way.

libexec/rc/rc.subr
176–177

There is no need to escape newline after && etc, the shell knows that more is needed, thus the sed command is only done if $cf is non-empty

2493

No, $_dot is only used in load_rc_config so caller can control which of dot, sdot or vdot should be used.
Here we want vdot - we want to be sure rc.subr.local has not been tampered with.

libexec/rc/rc.subr
176–177

So only a style remark: indent missing for the continuation. And just noticing, I think we have switched to use $( ...) for subshells.

2493

In that case I suggest to add a comment to this effect.

Fix some style issues in rc.subr

sjg marked 5 inline comments as done.Jan 31 2024, 8:09 AM
sjg added inline comments.
libexec/rc/rc.subr
1635–1637

Sorry? where is indent missing?

2493

Sure, and thanks for the feeback btw

libexec/rc/rc.subr
1635–1637

The case statement below with only one case. But as you don't indent each case but have it on the same height than the case keyword, you can forget my comment (personally I don't care which of both stales are used, and I failed to check which style is used in rc.subr before now...). You can mark this as done.

Make a start on man pages

sjg marked 4 inline comments as done.

Further man page tweaks

libexec/rc/rc.subr
2494

Would this be better named local.rc.subr ?

Add man page for debug.sh

Tweak debug.sh.8, use .Fn when refering to functions called.

rc.subr add comment explaining the no-op Debug{On,Off} and safe_dot
at the end - in case the real ones could not be found.

libexec/rc/debug.sh
4

Maybe this should be debug.subr?

76

Did you mean to include this?

85

Copyright placement is non-standard.
Also, the license isn't any of the standard ones. Maybe ISC license would be closest?

libexec/rc/rc.subr
178

This may just be my preference, but , is a very visually indistinct mark. I prefer / typically for this.

1573

why is local hard coded to be trusted?

1694

not a problem, per-se, but some routines have long args, others don't. any reason?

1737

How do you prevent un-trusted files from bringing in other files with a bare dot?

libexec/rc/debug.sh
4

It is a standalone facility for debugging complicated shell scripts, is existed in much this form for about 30 years, I'd rather leave it alone.

I can leave it out of this patch - the no-op Debug{On,Off} at the end of rc.subr ensure no harm.

But I figured others could make use of it.

85

True, as noted this thing has been around like this for a long time,
I've no objection to adding an

SPDX-License-Identifier: BSD-2-Clause

which is morally equivalent.

libexec/rc/rc.subr
178

Sure

1573

Excellent question - part of the discussion I wanted to have on this ;-)
I'd prefer to apply the same verify requirement.

1694

The long args to run_rc_scripts are an aid to understanding,
these can also be made long (--verify and --safe ?)

1737

You cannot, that's the whole point of allowing -v etc.
Eg. I have an rc.d/audit which uses -v to ensure only a trusted config can be used. We turn on -o verify to help prevent a trusted file accidentally reading in an untrusted one.

There are of course limits - if you allow any untrusted files you accept a risk.
I guess it would makes sense to allow rc.conf et al to set the default _dot for load_rc_config so the truely paranoid can set it to _vdot or _sdot

sjg marked 4 inline comments as done.Feb 6 2024, 11:56 PM
sjg added inline comments.
libexec/rc/rc.subr
1737

If you have a better name suggestion that load_rc_config_reader for controlling which of {dot,sdot,vdot} should be used by default - I should validate it is one of those...

Validate load_rc_config_reader

Use local.rc.subr rather than rc.subr.local

Add SPDX BSD-2-Clause to safe_eval.sh

This revision is now accepted and ready to land.Feb 9 2024, 5:00 PM
This revision was automatically updated to reflect the committed changes.