All opening happens before unveil/pledge and the file handle is kept
open read/write so it can be used without any pledge.
Simpler/less code and less chances to write other files (accidentially).
Opening the same file *path* twice is a TOCTOU, although not a critical
one: worst case we load from one file and save to another - the impact
depends on how and when catgirl is started the next anyway.
More importantly, keeping the file handle open at runtime allows us to
drop all filesystem related promises for `-s/save' on OpenBSD.
uiLoad() now opens "r+", meaning "Open for reading and writing." up
front so uiSave() can write to it. In the case of a nonexistent save
file, it now opens with "w" meaning "Open for writing. The file is
created if it does not exist.", i.e. the same write/create semantics as
"w" except uiLoad() no longer truncates. existing files.
uiSave() now truncates the save file to avoid appending in general.
After TLS cert/key files, the save file is the only file being read from;
do so before pleding and drop the "rpath" promise all together: log files
will only be created and written to.
Both ssl(8) as well as ncurses(3) related files are now read completely
by the time of ircConfig() and uiInitEarly() respectively, so read
access to the filesystem is no longer needed at all unless the "log" or
"save" options are used.
Previous tls_default_ca_cert_file(3) hoisting makes this possible: all
TLS related files are fully loaded into memory by ircConfig() such that
ircConnect() will not do any file I/O.
Call ircConfig() before pledge(2) in the `-o' "print cert" case so this
works out -- that order should have been preserved in the previous
a989e15 "OpenBSD: hoist -o/printCert code to simplify" but fixing it now
nicely demonstrates the achivement even more so.
catgirl needs:
- "stdio tty" at all times
- "rpath inet dns" once at startup for terminfo(5) and ssl(8)
- "proc exec" iff -R/restrict options is disabled
- "rpath wpath cpath" iff -s/save or -l/log options is enabled
Status quo: catgirl starts with the superset of all possible promises
"stdio rpath wpath cpath inet dns tty proc exec", drops offline with
"stdio rpath wpath cpath tty proc exec" and possibly drops to either of
"stdio rpath wpath cpath tty", "stdio tty proc exec" or "stdio tty"
depending on the options used.
Such step-by-step reduction is straight forward and easy to model along
the process runtime, but it comes with the drawback of starting with
too broad promises right from the beginning, i.e. `catgirl -R -h host'
is able to execute code and write to filesystems even though it must
never do so according the (un)used options.
Lay out required promises up front and pledge in two stages:
1. initial setup, i.e. fixed "stdio tty" plus temporary "rpath inet dns"
plus potential "rpath wpath cpath" plus potential "proc exec"
2. final rutime, i.e. fixed "stdio tty"
plus potential "rpath wpath cpath" plus potential "proc exec"
This way the above mentioned usage example can never execute or write
files, hence less potential for bugs and more accurate modelling of
catgirl's runtime -- dropping "inet dns" alone in between also becomes
obsolete with this approach.
initscr(3) in uiInitEarly() attempts more than /usr/share/terminfo/, see
`mandoc -O tag=TERMINFO ncurses`.
Even though non-default terminfo handling seems rare and it is unlikely
to have ever caused a problem for catgirl users on OpenBSD, the current
is still wrong by oversimplifying it.
Avoid the entire curses/unveil clash by setting up the screen before
unveiling.
Nothing but the TLS handshake is required, so skip all other setup.
On OpenBSD, unveil() handling needs fixing which will involve code
reshuffling -- this is the first related but standalone step.
Also pledge this one-off code path individually such with simpler and
tighter promises while here.
The (not perfectly obvious) way catgirl crafts directories gets triggered
by unveilAll() even if no passed option requires filesystem access:
$ env -i TERM=xterm ./catgirl -h irc.hackint.eu -R -n nobody
catgirl: HOME unset
Here unveil(2) is used due to the "restrict" option, but besides terminfo(5)
and certificates catgirl does not need any other files, yet it tries to init
the data path -- passing XDG_DATA_HOME=/var/empty makes above invocation work
showing how the then successful path setup is not required.
Fix this by not unveiling the unneeded data path in the first place.
Log files and state save/restore both require read/write access to
the filesystem, both during start and exit.
If neither features are used, catgirl may run with "stdio tty".
catgirl has no reconnect feature and generally must not do
anything but read/write from/to the connected socket which
does not require "inet" or "dns" promises.
Simplify logic, be more idiomatic and finalize by pledging after
all unveiling is done by omitting the "unveil" promise and thereby
not allowing further calls to it.
Restrict mode will focus on sandboxing, while kiosk will continue
to restrict IRC access through a public kiosk. Kiosk mode without
restrict mode allows execution of man 1 catgirl with /help, assuming
external sandboxing.
The /list and /part commands are also added to the list of disabled
commands in kiosk mode, since they are pointless without access to
/join.
This allows limiting the nick colors used to the 16-color terminal set
without modifying the TERM environment variable. Produces different
results from just using the default configuration in a 16-color
terminal, but what can you do?
Apparently IRCds have decided that the 15-parameter limit doesn't matter
anymore. 254 is the maximum number of single-byte parameters (following
a single-byte command) which fit in a 512-byte CR-LF-terminated line.
When everyone decides that the 512-byte line length limit doesn't matter
either, I will delete my software and people can use some JavaScript
garbage instead.
This makes struct Message 2080 bytes, but there's only ever one or two
of them around at once. Avoid passing it by value to handle.
Without this, I was having catgirl "time out" from pounce's POV, but
without catgirl noticing anything... I still don't understand this. Been
using this fix for a couple weeks though and it stopped happening, and
it's otherwise harmless, but yikes.
These are actually from RFC 1459, since that seems to be the more likely
lowest common denominator, so I should maybe it it to STANDARDS (along
with ircdocs' section on ISUPPORT). RFC 2812 has a lot of stuff that
isn't currently used.
Klemens Nanni
C. McEnroe
Klemens Nanni