Re: unveil(2) getent(1)
On Mon, 24 Sep 2018 09:41:02 -0600, "Theo de Raadt" wrote: > > I wonder if we can do unveil(NULL, NULL) for getent databases without > > an explicit file. A quick test seems to work for dns. > > If the pledge lacks "unveil", you get the same effect. Am I correct in thinking that the veil is not activated for group, hosts and passwd? - todd
Re: unveil(2) getent(1)
On Mon, Sep 24, 2018 at 08:56:14PM +0100, Ricardo Mestre wrote: > I actually prefer to see it go away since it doesn't protect us much and the > real meat is actually on the pledge(2) inside the loop. Nevertheless this > still > should on a separate commit. OK kn
Re: unveil(2) getent(1)
I actually prefer to see it go away since it doesn't protect us much and the
real meat is actually on the pledge(2) inside the loop. Nevertheless this still
should on a separate commit.
OK?
Index: getent.c
===
RCS file: /cvs/src/usr.bin/getent/getent.c,v
retrieving revision 1.14
diff -u -p -u -r1.14 getent.c
--- getent.c1 Feb 2016 19:57:28 - 1.14
+++ getent.c24 Sep 2018 19:52:35 -
@@ -95,9 +95,6 @@ main(int argc, char *argv[])
{
struct getentdb *curdb;
- if (pledge("stdio dns rpath getpw", NULL) == -1)
- err(1, "pledge");
-
if (argc < 2)
usage();
for (curdb = databases; curdb->name != NULL; curdb++) {
On 20:41 Mon 24 Sep , Klemens Nanni wrote:
> On Mon, Sep 24, 2018 at 10:49:42AM +0100, Ricardo Mestre wrote:
> > Comments? OK? The initial pledge(2) is so short lived that I was tempted to
> > remove it, but I'm open to suggestions :)
> Is there any compelling reason to keep the initial superset pledge?
>
> Without it, the only code paths without pledge/unveil are either
> main()->usage()->exit() or main()->{all failing strcmp() loop}->return.
Re: unveil(2) getent(1)
On Mon, Sep 24, 2018 at 10:49:42AM +0100, Ricardo Mestre wrote:
> Comments? OK? The initial pledge(2) is so short lived that I was tempted to
> remove it, but I'm open to suggestions :)
Is there any compelling reason to keep the initial superset pledge?
Without it, the only code paths without pledge/unveil are either
main()->usage()->exit() or main()->{all failing strcmp() loop}->return.
Re: unveil(2) getent(1)
On Mon, Sep 24, 2018 at 09:33:50AM -0600, Todd C. Miller wrote: > I wonder if we can do unveil(NULL, NULL) for getent databases without > an explicit file. A quick test seems to work for dns. Same thought here at first, but we're pledging without "unveil" promise after unveiling files so no need to for that.
Re: unveil(2) getent(1)
Todd C. Miller wrote: > On Mon, 24 Sep 2018 09:21:17 -0600, "Theo de Raadt" wrote: > > > Only passwd, group, netid, and ethers are supported. Well, there > > is hosts (== networks in our case) but that translation happens > > seperately. > > That's what I thought from a grep of libc. So I don't see any > reason why Ricardo's diff can't go in. I agree. > I wonder if we can do unveil(NULL, NULL) for getent databases without > an explicit file. A quick test seems to work for dns. If the pledge lacks "unveil", you get the same effect.
Re: unveil(2) getent(1)
On Mon, 24 Sep 2018 09:21:17 -0600, "Theo de Raadt" wrote: > Only passwd, group, netid, and ethers are supported. Well, there > is hosts (== networks in our case) but that translation happens > seperately. That's what I thought from a grep of libc. So I don't see any reason why Ricardo's diff can't go in. I wonder if we can do unveil(NULL, NULL) for getent databases without an explicit file. A quick test seems to work for dns. - todd
Re: unveil(2) getent(1)
Todd C. Miller wrote: > On Mon, 24 Sep 2018 13:46:51 +0100, Ricardo Mestre wrote: > > > Oh boy, I took a brief look into Makefile.yp(8), let's forget about > > this since ALL of them can have YP maps (except for /etc/shells). > > Not all of those actually have plumbing in libc as far as I can > tell. I think we only support passwd, group and ethers via YP. > Someone more familiar with YP should probably chime in. When I wrote this code, I *intentionally* avoided coupling many databases to YP. Even in SunOS, it just created problems. Only passwd, group, netid, and ethers are supported. Well, there is hosts (== networks in our case) but that translation happens seperately. Really ANY kind of map could exist, but it doesn't mean things use it. Or should. Or will. Really, the situation is we absolutely won't ever, it helps nothing.
Re: unveil(2) getent(1)
On Mon, 24 Sep 2018 13:46:51 +0100, Ricardo Mestre wrote: > Oh boy, I took a brief look into Makefile.yp(8), let's forget about > this since ALL of them can have YP maps (except for /etc/shells). Not all of those actually have plumbing in libc as far as I can tell. I think we only support passwd, group and ethers via YP. Someone more familiar with YP should probably chime in. - todd
Re: unveil(2) getent(1)
Looks good to me. Clever.
Re: unveil(2) getent(1)
Oh boy, I took a brief look into Makefile.yp(8), let's forget about this since ALL of them can have YP maps (except for /etc/shells). On 06:20 Mon 24 Sep , Todd C. Miller wrote: > On Mon, 24 Sep 2018 12:25:51 +0100, Ricardo Mestre wrote: > > > Wouldn't this be already contemplated by pledge(getpw) on both group and > > passwd databases? I'm not touching those since they already whitelist > > all necessary files through pledge(2). > > I think you are correct, it appears the getpw pledge should unveil > what is needed for YP. Note that the ethers database can also be > stored YP. > > - todd >
Re: unveil(2) getent(1)
On Mon, 24 Sep 2018 12:25:51 +0100, Ricardo Mestre wrote: > Wouldn't this be already contemplated by pledge(getpw) on both group and > passwd databases? I'm not touching those since they already whitelist > all necessary files through pledge(2). I think you are correct, it appears the getpw pledge should unveil what is needed for YP. Note that the ethers database can also be stored YP. - todd
Re: unveil(2) getent(1)
Wouldn't this be already contemplated by pledge(getpw) on both group and passwd databases? I'm not touching those since they already whitelist all necessary files through pledge(2). On 05:11 Mon 24 Sep , Todd C. Miller wrote: > I doubt this will work on systems using YP or ypldap. > > - todd
Re: unveil(2) getent(1)
I doubt this will work on systems using YP or ypldap. - todd
unveil(2) getent(1)
Hi,
Since the databases that require rpath only need to access one file we can add
one attribute to the struct getentdb to identify which of those DBs we need
unveiled. For group/hosts/passwd the files are already whitelisted through
pledge(2) so I set them as NULL.
With that information we can now check if the unveil attribute is not NULL and
then unveil(2) the database file as needed. I tested all databases by
enumerating all entries (for the ones that support it) and by selecting
specific entries without any apparent issues.
Comments? OK? The initial pledge(2) is so short lived that I was tempted to
remove it, but I'm open to suggestions :)
Index: getent.c
===
RCS file: /cvs/src/usr.bin/getent/getent.c,v
retrieving revision 1.14
diff -u -p -u -r1.14 getent.c
--- getent.c1 Feb 2016 19:57:28 - 1.14
+++ getent.c24 Sep 2018 09:36:43 -
@@ -77,15 +77,16 @@ static struct getentdb {
const char *name;
int (*fn)(int, char *[]);
const char *pledge;
+ const char *unveil;
} databases[] = {
- { "ethers", ethers, "stdio rpath" },
- { "group",group, "stdio getpw" },
- { "hosts",hosts, "stdio dns" },
- { "passwd", passwd, "stdio getpw" },
- { "protocols",protocols, "stdio rpath" },
- { "rpc", rpc,"stdio rpath" },
- { "services", services, "stdio rpath" },
- { "shells", shells, "stdio rpath" },
+ { "ethers", ethers, "stdio rpath", "/etc/ethers"
},
+ { "group",group, "stdio getpw", NULL},
+ { "hosts",hosts, "stdio dns",NULL},
+ { "passwd", passwd, "stdio getpw", NULL},
+ { "protocols",protocols, "stdio rpath",
"/etc/protocols"},
+ { "rpc", rpc,"stdio rpath", "/etc/rpc"
},
+ { "services", services, "stdio rpath", "/etc/services"
},
+ { "shells", shells, "stdio rpath", "/etc/shells"
},
{ NULL, NULL, },
};
@@ -95,13 +96,17 @@ main(int argc, char *argv[])
{
struct getentdb *curdb;
- if (pledge("stdio dns rpath getpw", NULL) == -1)
+ if (pledge("stdio rpath dns getpw unveil", NULL) == -1)
err(1, "pledge");
if (argc < 2)
usage();
for (curdb = databases; curdb->name != NULL; curdb++) {
if (strcmp(curdb->name, argv[1]) == 0) {
+ if (curdb->unveil != NULL) {
+ if (unveil(curdb->unveil, "r") == -1)
+ err(1, "unveil");
+ }
if (pledge(curdb->pledge, NULL) == -1)
err(1, "pledge");
