[PATCH] OpenBSD: pledge minimum promises from the start

While the previous patch worked fine with `-o' for printing server
certificates, I did not take it into proper consideration;  here is
another commit fixing this and I am sending it separately for the sake
of review.

Mind simply amending/squashing this into the previous one while merging?

-- >8 --

New version properly taking `-o' into account;  the previous worked
just fine but it did pledge too much.  This patch should also be clearer.

-- >8 --

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

(In the special `-o' case only "stdio rpath inet dns" is needed.)

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.
---
 chat.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/chat.c b/chat.c
index b3990f3..20e0d89 100644
--- a/chat.c
+++ b/chat.c
@@ -295,7 +295,17 @@ int main(int argc, char *argv[]) {
 
 #ifdef __OpenBSD__
 	if (self.restricted) unveilAll(trust, cert, priv);
-	int error = pledge("stdio rpath wpath cpath inet dns tty proc exec", NULL);
+	char promises[64] = "stdio", *promisesFinal;
+	struct Cat cat = { promises, sizeof(promises), strlen(promises) };
+	if (!printCert) {
+		catf(&cat, " tty");
+		if (save || logEnable) catf(&cat, " rpath wpath cpath");
+		if (!self.restricted) catf(&cat, " proc exec");
+		promisesFinal = strdup(promises);
+		if (!promisesFinal) err(EX_OSERR, "strdup");
+	}
+	catf(&cat, " rpath inet dns");
+	int error = pledge(promises, NULL);
 	if (error) err(EX_OSERR, "pledge");
 #endif
 
@@ -323,10 +333,6 @@ int main(int argc, char *argv[]) {
 	uiDraw();
 	
 	int irc = ircConnect(bind, host, port);
-#ifdef __OpenBSD__
-	error = pledge("stdio rpath wpath cpath tty proc exec", NULL);
-	if (error) err(EX_OSERR, "pledge");
-#endif
 
 	if (pass) ircFormat("PASS :%s\r\n", pass);
 	if (sasl) ircFormat("CAP REQ :sasl\r\n");
@@ -357,12 +363,9 @@ int main(int argc, char *argv[]) {
 	}
 
 #ifdef __OpenBSD__
-	char promises[64] = "stdio tty";
-	struct Cat cat = { promises, sizeof(promises), strlen(promises) };
-	if (save || logEnable) catf(&cat, " rpath wpath cpath");
-	if (!self.restricted) catf(&cat, " proc exec");
-	error = pledge(promises, NULL);
+	error = pledge(promisesFinal, NULL);
 	if (error) err(EX_OSERR, "pledge");
+	free(promisesFinal);
 #endif
 
 	struct pollfd fds[] = {
-- 
2.32.0

On Mon, Jun 7, 2021, at 09:53, Klemens Nanni wrote:
> 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"

What about instead of duplicating the string, putting "rpath inet
dns" first and passing a pointer to the remainder of the string for
the final promises?

On Mon, Jun 07, 2021 at 11:07:17AM -0400, june wrote:
> On Mon, Jun 7, 2021, at 09:53, Klemens Nanni wrote:
> > 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"
> 
> What about instead of duplicating the string, putting "rpath inet
> dns" first and passing a pointer to the remainder of the string for
> the final promises?
That seems even more hackish and somewhat defeats the point of having
such append-functionality (struct Cat), doesn't it?

I'm not a fan of such string composition, but it still reads easy enough
which I wouldn't say about additional pointers in it.