pledge(2) talk(1)/talkd(8)

Discussion:

Ricardo Mestre

2016-02-01 07:59:21 UTC

Hi tech@,

Please find below a pledge(2) for talk(1)/talkd(8)

talkd(8):

rpath: fopen(3) _PATH_UTMP in read mode
wpath/cpath: fopen(3) full_tty in write mode (w), which implies O_CREAT
inet/dns: the intervening hosts in the conversation may remote, or not, but
since we will never know beforehand then it'll always need inet and dns to
resolve the hostnames

talk(1):

At the beginning the largest pledge is the following:

rpath: read /etc/hosts (gethostbyname(3)) and /etc/services (getservbyname(3))
inet/dns: talk may need to connect to a remote host and resolve it
getpw: if getlogin(2) fails then it needs getpwuid(3) as a fallback
tty: this is a typical tty application, so it'll always need this annotation

Then just before the application main loop check if the talk is with local user
so it only needs "stdio tty", if it's remote then it needs "stdio inet tty".

Comments?

Index: libexec/talkd/talkd.c
===================================================================
RCS file: /cvs/src/libexec/talkd/talkd.c,v
retrieving revision 1.24
diff -u -p -u -r1.24 talkd.c
--- libexec/talkd/talkd.c 1 Feb 2016 07:25:51 -0000 1.24
+++ libexec/talkd/talkd.c 1 Feb 2016 07:48:42 -0000
@@ -81,6 +81,11 @@ main(int argc, char *argv[])
signal(SIGALRM, timeout);
alarm(TIMEOUT);

+ if (pledge("stdio rpath wpath cpath inet dns", NULL) == -1) {
+ syslog(LOG_ERR, "pledge: %m");
+ _exit(1);
+ }
+
for (;;) {
CTL_RESPONSE response;
socklen_t len = sizeof(response.addr);
Index: usr.bin/talk/talk.c
===================================================================
RCS file: /cvs/src/usr.bin/talk/talk.c,v
retrieving revision 1.10
diff -u -p -u -r1.10 talk.c
--- usr.bin/talk/talk.c 1 Feb 2016 07:29:25 -0000 1.10
+++ usr.bin/talk/talk.c 1 Feb 2016 07:48:48 -0000
@@ -35,6 +35,7 @@
#include <unistd.h>

#include "talk.h"
+#include "talk_ctl.h"

/*
* talk: A visual form of write. Using sockets, a two way
@@ -53,6 +54,9 @@
int
main(int argc, char *argv[])
{
+ if (pledge("stdio rpath inet dns getpw tty", NULL) == -1)
+ err(1, "pledge");
+
get_names(argc, argv);
init_display();
open_ctl();
@@ -62,6 +66,15 @@ main(int argc, char *argv[])
invite_remote();
end_msgs();
set_edit_chars();
+
+ if (his_machine_addr.s_addr == my_machine_addr.s_addr) {
+ if (pledge("stdio tty", NULL) == -1)
+ err(1, "pledge");
+ } else {
+ if (pledge("stdio inet tty", NULL) == -1)
+ err(1, "pledge");
+ }
+
talk();
return (0);
}

Sebastien Marie

2016-02-01 08:37:39 UTC

Permalink

Post by Ricardo Mestre
Please find below a pledge(2) for talk(1)/talkd(8)

[...]

Post by Ricardo Mestre
rpath: read /etc/hosts (gethostbyname(3)) and /etc/services (getservbyname(3))

"dns" should be enough for reading these files: there are whitelisted
for reading when the process has PLEDGE_DNS promise.

Post by Ricardo Mestre
inet/dns: talk may need to connect to a remote host and resolve it
getpw: if getlogin(2) fails then it needs getpwuid(3) as a fallback
tty: this is a typical tty application, so it'll always need this annotation
Then just before the application main loop check if the talk is with local user
so it only needs "stdio tty", if it's remote then it needs "stdio inet tty".
Comments?
Index: libexec/talkd/talkd.c
===================================================================
RCS file: /cvs/src/libexec/talkd/talkd.c,v
retrieving revision 1.24
diff -u -p -u -r1.24 talkd.c
--- libexec/talkd/talkd.c 1 Feb 2016 07:25:51 -0000 1.24
+++ libexec/talkd/talkd.c 1 Feb 2016 07:48:42 -0000
@@ -81,6 +81,11 @@ main(int argc, char *argv[])
signal(SIGALRM, timeout);
alarm(TIMEOUT);
+ if (pledge("stdio rpath wpath cpath inet dns", NULL) == -1) {
+ syslog(LOG_ERR, "pledge: %m");
+ _exit(1);
+ }
+
for (;;) {
CTL_RESPONSE response;
socklen_t len = sizeof(response.addr);
Index: usr.bin/talk/talk.c
===================================================================
RCS file: /cvs/src/usr.bin/talk/talk.c,v
retrieving revision 1.10
diff -u -p -u -r1.10 talk.c
--- usr.bin/talk/talk.c 1 Feb 2016 07:29:25 -0000 1.10
+++ usr.bin/talk/talk.c 1 Feb 2016 07:48:48 -0000
@@ -35,6 +35,7 @@
#include <unistd.h>
#include "talk.h"
+#include "talk_ctl.h"
/*
* talk: A visual form of write. Using sockets, a two way
@@ -53,6 +54,9 @@
int
main(int argc, char *argv[])
{
+ if (pledge("stdio rpath inet dns getpw tty", NULL) == -1)
+ err(1, "pledge");
+
get_names(argc, argv);
init_display();
open_ctl();
@@ -62,6 +66,15 @@ main(int argc, char *argv[])
invite_remote();
end_msgs();
set_edit_chars();
+
+ if (his_machine_addr.s_addr == my_machine_addr.s_addr) {
+ if (pledge("stdio tty", NULL) == -1)
+ err(1, "pledge");
+ } else {
+ if (pledge("stdio inet tty", NULL) == -1)
+ err(1, "pledge");
+ }
+
talk();
return (0);
}

--
Sebastien Marie

Ricardo Mestre

2016-02-01 09:50:46 UTC

Permalink

Hi Sebastien,

Thanks for noticing, I keep forgetting that, sorry :\

But in this case I also wanted to add the real reason (which I also forgot)
that it needs to read ~/.terminfo through initscr(), so it still needs rpath.

Post by Sebastien Marie

Post by Ricardo Mestre
Please find below a pledge(2) for talk(1)/talkd(8)

[...]

Post by Ricardo Mestre
rpath: read /etc/hosts (gethostbyname(3)) and /etc/services (getservbyname(3))

"dns" should be enough for reading these files: there are whitelisted
for reading when the process has PLEDGE_DNS promise.

--
Sebastien Marie

Jérémie Courrèges-Anglas

2016-02-01 18:33:04 UTC

Permalink

Post by Ricardo Mestre
Please find below a pledge(2) for talk(1)/talkd(8)
rpath: fopen(3) _PATH_UTMP in read mode
wpath/cpath: fopen(3) full_tty in write mode (w), which implies O_CREAT
inet/dns: the intervening hosts in the conversation may remote, or not, but
since we will never know beforehand then it'll always need inet and dns to
resolve the hostnames
rpath: read /etc/hosts (gethostbyname(3)) and /etc/services (getservbyname(3))
inet/dns: talk may need to connect to a remote host and resolve it
getpw: if getlogin(2) fails then it needs getpwuid(3) as a fallback
tty: this is a typical tty application, so it'll always need this annotation
Then just before the application main loop check if the talk is with local user
so it only needs "stdio tty", if it's remote then it needs "stdio inet tty".
Comments?

It looks a lot like the diff I wrote yesterday evening. 8)

Post by Ricardo Mestre
Index: libexec/talkd/talkd.c
===================================================================
RCS file: /cvs/src/libexec/talkd/talkd.c,v
retrieving revision 1.24
diff -u -p -u -r1.24 talkd.c
--- libexec/talkd/talkd.c 1 Feb 2016 07:25:51 -0000 1.24
+++ libexec/talkd/talkd.c 1 Feb 2016 07:48:42 -0000
@@ -81,6 +81,11 @@ main(int argc, char *argv[])
signal(SIGALRM, timeout);
alarm(TIMEOUT);
+ if (pledge("stdio rpath wpath cpath inet dns", NULL) == -1) {
+ syslog(LOG_ERR, "pledge: %m");
+ _exit(1);
+ }
+
for (;;) {
CTL_RESPONSE response;
socklen_t len = sizeof(response.addr);

Post by Ricardo Mestre
Index: usr.bin/talk/talk.c
===================================================================
RCS file: /cvs/src/usr.bin/talk/talk.c,v
retrieving revision 1.10
diff -u -p -u -r1.10 talk.c
--- usr.bin/talk/talk.c 1 Feb 2016 07:29:25 -0000 1.10
+++ usr.bin/talk/talk.c 1 Feb 2016 07:48:48 -0000
@@ -35,6 +35,7 @@
#include <unistd.h>
#include "talk.h"
+#include "talk_ctl.h"
/*
* talk: A visual form of write. Using sockets, a two way
@@ -53,6 +54,9 @@
int
main(int argc, char *argv[])
{
+ if (pledge("stdio rpath inet dns getpw tty", NULL) == -1)
+ err(1, "pledge");
+

Post by Ricardo Mestre
get_names(argc, argv);
init_display();
open_ctl();
@@ -62,6 +66,15 @@ main(int argc, char *argv[])
invite_remote();
end_msgs();
set_edit_chars();
+
+ if (his_machine_addr.s_addr == my_machine_addr.s_addr) {
+ if (pledge("stdio tty", NULL) == -1)
+ err(1, "pledge");
+ } else {
+ if (pledge("stdio inet tty", NULL) == -1)
+ err(1, "pledge");
+ }
+

While it seems to work at first, I don't understand the special case.

Post by Ricardo Mestre
talk();
return (0);
}

--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE

Ricardo Mestre

2016-02-01 18:46:56 UTC

Permalink

Hi Jérémie,

The hunk you are talking about I used because I didn't have a way to
test it between 2 different hosts, so better safe than sorry I used that
condition, but if you can test it and if it works with just stdio tty
then the condition can go away.

Post by JÃ©rÃ©mie CourrÃ¨ges-Anglas

It looks a lot like the diff I wrote yesterday evening. 8)

While it seems to work at first, I don't understand the special case.

Post by Ricardo Mestre
talk();
return (0);
}

Jérémie Courrèges-Anglas

2016-02-01 19:00:36 UTC

Permalink

Post by Ricardo Mestre
Hi Jérémie,
The hunk you are talking about I used because I didn't have a way to
test it between 2 different hosts, so better safe than sorry I used that
condition, but if you can test it and if it works with just stdio tty
then the condition can go away.

I was rather wondering why "inet" can be dropped in the localhost case.

[...]

Post by Ricardo Mestre

While it seems to work at first, I don't understand the special case.

Post by Ricardo Mestre
talk();
return (0);
}

--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE

Ricardo Mestre

2016-02-01 19:39:11 UTC

Permalink

If it's local it doesn't need it, but if it's remote I'm not sure. Most
likely it doesn't need as well (talk() only read(2)'s and write(1)'s on
the socket), but I can't test this between 2 different hosts to ensure that.

I was rather wondering why "inet" can be dropped in the localhost case.
[...]

Post by Ricardo Mestre

While it seems to work at first, I don't understand the special case.

Post by Ricardo Mestre
talk();
return (0);
}