Re: add DIOCRADDADDRS ioctl to kern_pledge pf
On Tue, Jan 14, 2020 at 11:05:38AM -0700, Theo de Raadt wrote: > Some of the pledges (such as "pf") exist to support a cluster of > programs -- not just 1 program -- and improve their security by limiting > what they can do. So that when the program gets subverted due something > on it's input, the damage it can do is limited. > Additionally, the list of operations permitted is kept very small, so that > the switch() case in the kernel don't turn into a bloated fiasco. OK. > Your proposal supports 1 program. Which isn't even in base. There is > no way I'm going accept this change. OK, I see. Yes it was selfish of me. > Beyond that I need to once again point out a major missed step: > > Your proposal is to permit DIOCRADDADDRS in all the current programs > using "pf". But you did not assessment to determine if there is a > downside to giving them such access. You simply wanted to barrel along > forward, to support your one little program. > > Ignoring the impact of your changes on the rest of the ecosystem is > totally nuts. Thanks for pointing that out. I did take a look at one program in base though and noticed it wasn't pledged. But yes I barrelled along. Sorry. Regards, -peter
Re: add DIOCRADDADDRS ioctl to kern_pledge pf
> I'm in the process of building a program that adds IP addresses to a table, > from the network, It is HMAC'ed. > > I was stopped by a pledge, it seems it was not configured. Here is the > ktrace snippet: > > 40051 table-server CALL open(0xbb705fb11f6,0x2) > 40051 table-server NAMI "/dev/pf" > 40051 table-server RET open 4 > 40051 table-server CALL kbind(0x7f7a2b08,24,0x2de4af929c6b5090) > 40051 table-server RET kbind 0 > 40051 table-server CALL ioctl(4,DIOCRADDTABLES,0x7f7a32a8) > 40051 table-server RET ioctl 0 > 40051 table-server CALL kbind(0x7f7a2b08,24,0x2de4af929c6b5090) > 40051 table-server RET kbind 0 > 40051 table-server CALL ioctl(4,DIOCRADDADDRS,0x7f7a32a8) > 40051 table-server PLDG ioctl, "tty", errno 1 Operation not permitted > 40051 table-server PSIG SIGABRT SIG_DFL > 40051 table-server NAMI "table-server.core" > > Here is a patch to consider, it compiles but I haven't tested it yet because > I'm unsure if there is a reason why this DIOCR* was left out. Some of the pledges (such as "pf") exist to support a cluster of programs -- not just 1 program -- and improve their security by limiting what they can do. So that when the program gets subverted due something on it's input, the damage it can do is limited. Additionally, the list of operations permitted is kept very small, so that the switch() case in the kernel don't turn into a bloated fiasco. Your proposal supports 1 program. Which isn't even in base. There is no way I'm going accept this change. Beyond that I need to once again point out a major missed step: Your proposal is to permit DIOCRADDADDRS in all the current programs using "pf". But you did not assessment to determine if there is a downside to giving them such access. You simply wanted to barrel along forward, to support your one little program. Ignoring the impact of your changes on the rest of the ecosystem is totally nuts.
add DIOCRADDADDRS ioctl to kern_pledge pf
Hi, I'm in the process of building a program that adds IP addresses to a table, from the network, It is HMAC'ed. I was stopped by a pledge, it seems it was not configured. Here is the ktrace snippet: 40051 table-server CALL open(0xbb705fb11f6,0x2) 40051 table-server NAMI "/dev/pf" 40051 table-server RET open 4 40051 table-server CALL kbind(0x7f7a2b08,24,0x2de4af929c6b5090) 40051 table-server RET kbind 0 40051 table-server CALL ioctl(4,DIOCRADDTABLES,0x7f7a32a8) 40051 table-server RET ioctl 0 40051 table-server CALL kbind(0x7f7a2b08,24,0x2de4af929c6b5090) 40051 table-server RET kbind 0 40051 table-server CALL ioctl(4,DIOCRADDADDRS,0x7f7a32a8) 40051 table-server PLDG ioctl, "tty", errno 1 Operation not permitted 40051 table-server PSIG SIGABRT SIG_DFL 40051 table-server NAMI "table-server.core" Here is a patch to consider, it compiles but I haven't tested it yet because I'm unsure if there is a reason why this DIOCR* was left out. I'm guessing, if the patch is OK, I'll have to leave the pledge out for 6.6 which is what this is intended for. Sad, but OK, at least there is unveil. Index: kern_pledge.c === RCS file: /cvs/src/sys/kern/kern_pledge.c,v retrieving revision 1.256 diff -u -p -u -r1.256 kern_pledge.c --- kern_pledge.c 8 Dec 2019 23:08:59 - 1.256 +++ kern_pledge.c 14 Jan 2020 17:51:19 - @@ -1205,6 +1205,7 @@ pledge_ioctl(struct proc *p, long com, s case DIOCADDRULE: case DIOCGETSTATUS: case DIOCNATLOOK: + case DIOCRADDADDRS: case DIOCRADDTABLES: case DIOCRCLRADDRS: case DIOCRCLRTABLES: Cheers, -peter
