Re: make iked not static

2015-10-21 Thread Theo de Raadt
>Already, iked is started after /usr has been mounted, so why the
>static requirement?

Historic theories about ipsec protected nfs?  Who knows.

>> --- etc/rc   18 Oct 2015 21:33:18 -  1.467
>> +++ etc/rc   20 Oct 2015 18:03:58 -
>> @@ -353,7 +353,7 @@ make_keys
>>  
>>  echo -n 'starting early daemons:'
>>  start_daemon syslogd ldattach pflogd nsd unbound ntpd
>> -start_daemon iscsid isakmpd iked sasyncd ldapd npppd
>> +start_daemon iscsid isakmpd sasyncd ldapd npppd
>>  echo '.'
>
>Most of these are dynamically linked.
>
>You can make iked dynamic without moving it in the startup sequence.

Let's focus on that question first.  Where should it be started?

Let's move isakmpd and iked at the same time.  To where?



Re: sync bioctl manual

2015-10-21 Thread Jason McIntyre
On Thu, Oct 22, 2015 at 12:35:53AM +0300, Kirill Bychkov wrote:
> On Thu, October 22, 2015 00:16, Jason McIntyre wrote:
> > On Wed, Oct 21, 2015 at 11:19:12PM +0300, Kirill Bychkov wrote:
> >> Hi!
> >> After halex@ removed a restriction to use passfile for creation of
> >> crypto volume, man page wasn't changed to explain new behaviour.
> >> OK?
> >>
> >
> > why not just remove the sentence? if you really want to keep it, i
> 
> An idea was to explicitly tell about this possibility because people still may
> think it's forbidden.
> 

yes, i see that. but it cuts both ways - in a release or two, no one
will remember we've put this there, and it will sound odd.

i'd just zap it.
jmc

> > suggest using "can" instead of "could also".
> 
> I'll change it, if the line will remain in manual.
> 
> >
> > jmc
> >
> >> Index: bioctl.8
> >> ===
> >> RCS file: /cvs/src/sbin/bioctl/bioctl.8,v
> >> retrieving revision 1.97
> >> diff -u -r1.97 bioctl.8
> >> --- bioctl.8   12 Sep 2015 14:21:25 -  1.97
> >> +++ bioctl.8   21 Oct 2015 20:15:47 -
> >> @@ -260,7 +260,7 @@
> >>  .It Fl p Ar passfile
> >>  Passphrase file used when crypto volumes are brought up.
> >>  This file must be root owned and have 0600 permissions.
> >> -This option cannot be used during the initial creation of the crypto
> >> volume.
> >> +This option also could be used during the initial creation of the crypto
> >> volume.
> >>  .It Fl r Ar rounds
> >>  When creating an encrypted volume, specifies the number of iterations of
> >>  the PBKDF2 algorithm used to convert a passphrase into a key
> >>
> >
> >
> 
> 



Re: sync bioctl manual

2015-10-21 Thread Kirill Bychkov
On Thu, October 22, 2015 00:16, Jason McIntyre wrote:
> On Wed, Oct 21, 2015 at 11:19:12PM +0300, Kirill Bychkov wrote:
>> Hi!
>> After halex@ removed a restriction to use passfile for creation of
>> crypto volume, man page wasn't changed to explain new behaviour.
>> OK?
>>
>
> why not just remove the sentence? if you really want to keep it, i

An idea was to explicitly tell about this possibility because people still may
think it's forbidden.

> suggest using "can" instead of "could also".

I'll change it, if the line will remain in manual.

>
> jmc
>
>> Index: bioctl.8
>> ===
>> RCS file: /cvs/src/sbin/bioctl/bioctl.8,v
>> retrieving revision 1.97
>> diff -u -r1.97 bioctl.8
>> --- bioctl.8 12 Sep 2015 14:21:25 -  1.97
>> +++ bioctl.8 21 Oct 2015 20:15:47 -
>> @@ -260,7 +260,7 @@
>>  .It Fl p Ar passfile
>>  Passphrase file used when crypto volumes are brought up.
>>  This file must be root owned and have 0600 permissions.
>> -This option cannot be used during the initial creation of the crypto
>> volume.
>> +This option also could be used during the initial creation of the crypto
>> volume.
>>  .It Fl r Ar rounds
>>  When creating an encrypted volume, specifies the number of iterations of
>>  the PBKDF2 algorithm used to convert a passphrase into a key
>>
>
>




Re: sync bioctl manual

2015-10-21 Thread Jason McIntyre
On Wed, Oct 21, 2015 at 11:19:12PM +0300, Kirill Bychkov wrote:
> Hi!
> After halex@ removed a restriction to use passfile for creation of
> crypto volume, man page wasn't changed to explain new behaviour.
> OK?
> 

why not just remove the sentence? if you really want to keep it, i
suggest using "can" instead of "could also".

jmc

> Index: bioctl.8
> ===
> RCS file: /cvs/src/sbin/bioctl/bioctl.8,v
> retrieving revision 1.97
> diff -u -r1.97 bioctl.8
> --- bioctl.8  12 Sep 2015 14:21:25 -  1.97
> +++ bioctl.8  21 Oct 2015 20:15:47 -
> @@ -260,7 +260,7 @@
>  .It Fl p Ar passfile
>  Passphrase file used when crypto volumes are brought up.
>  This file must be root owned and have 0600 permissions.
> -This option cannot be used during the initial creation of the crypto volume.
> +This option also could be used during the initial creation of the crypto 
> volume.
>  .It Fl r Ar rounds
>  When creating an encrypted volume, specifies the number of iterations of
>  the PBKDF2 algorithm used to convert a passphrase into a key
> 



Re: smtpd: pledge, chmod and deliver_maildir

2015-10-21 Thread Gregor Best
Nice to see rubber duck debugging working. The attached patch seems to
be enough

-- 
Gregor
--

Index: smtpd.c
===
RCS file: /home/cvs/src/usr.sbin/smtpd/smtpd.c,v
retrieving revision 1.250
diff -u -p -u -r1.250 smtpd.c
--- smtpd.c 17 Oct 2015 16:03:20 -  1.250
+++ smtpd.c 21 Oct 2015 20:35:35 -
@@ -690,7 +690,7 @@ main(int argc, char *argv[])
 
purge_task();
 
-   if (pledge("stdio rpath wpath cpath flock tmppath "
+   if (pledge("fattr stdio rpath wpath cpath flock tmppath "
"getpw sendfd proc exec id inet unix", NULL) == -1)
err(1, "pledge");




smtpd: pledge, chmod and deliver_maildir

2015-10-21 Thread Gregor Best
Hi people,

I've noticed smtpd's deliver_maildir getting killed on syscall 15
(chmod) with the latest snapshot. I've rebuilt and core dumped it as
described by Sebastien and this is the backtrace I got:

#0  0x1d7e8175149a in chmod () at :2
#1  0x1d7c72744ffe in mkdirs (path=0x7f7dd0d0 "/home/gbe/Maildir", 
mode=448)
at /usr/src/usr.sbin/smtpd/smtpd/../util.c:223
#2  0x1d7c727471d0 in delivery_maildir_open
(deliver=0x7f7dd0d0) at 
/usr/src/usr.sbin/smtpd/smtpd/../delivery_maildir.c:99
#3  0x1d7c7273dd1e in parent_imsg (p=0x1d7e87f6a000,
imsg=Variable "imsg" is not available.)
at /usr/src/usr.sbin/smtpd/smtpd/../smtpd.c:1003
#4  0x1d7c7273afeb in imsg_dispatch (p=0x1d7e87f6a000,
imsg=0x7f7df270)
at /usr/src/usr.sbin/smtpd/smtpd/../smtpd.c:1283
#5  0x1d7c7271ed3c in mproc_dispatch (fd=Variable
"fd" is not available.
) at /usr/src/usr.sbin/smtpd/smtpd/../mproc.c:213
#6  0x1d7ec1d22008 in event_base_loop
(base=0x1d7ee286d800, flags=Variable "flags" is not
available.) at /usr/src/lib/libevent/event.c:350
#7  0x1d7c7273bf64 in main (argc=Variable "argc"
is not available.) at /usr/src/usr.sbin/smtpd/smtpd/../smtpd.c:697

smtpd's output looks like this when this happens:
[... noise omitted ...]
delivery: TempFail for 361bd1ee4342b3f8: from=,
to=, user=gbe, method=maildir, delay=0s, stat=Error
(terminated; signal 6)
debug: mda: session 757ea75d406dfe65 done
[... noise omitted ...]
(this is with "abort" added to all pledges)

The weird thing is that as far as I can see the process handling the
delivery seems to correctly pledge wpath. If I remove the pledge from
smtpd.c, line 693, everything works fine.

Any hints on how to debug this further? I can provide core files if
needed.

-- 
Gregor



sync bioctl manual

2015-10-21 Thread Kirill Bychkov
Hi!
After halex@ removed a restriction to use passfile for creation of
crypto volume, man page wasn't changed to explain new behaviour.
OK?

Index: bioctl.8
===
RCS file: /cvs/src/sbin/bioctl/bioctl.8,v
retrieving revision 1.97
diff -u -r1.97 bioctl.8
--- bioctl.812 Sep 2015 14:21:25 -  1.97
+++ bioctl.821 Oct 2015 20:15:47 -
@@ -260,7 +260,7 @@
 .It Fl p Ar passfile
 Passphrase file used when crypto volumes are brought up.
 This file must be root owned and have 0600 permissions.
-This option cannot be used during the initial creation of the crypto volume.
+This option also could be used during the initial creation of the crypto 
volume.
 .It Fl r Ar rounds
 When creating an encrypted volume, specifies the number of iterations of
 the PBKDF2 algorithm used to convert a passphrase into a key



Pledge "id" for identd

2015-10-21 Thread Gregor Best
Hi people,

identd's parent process needs to pledge "id" so it can call setgroups
and friends later.

-- 
Gregor

Index: identd.c
===
RCS file: /mnt/media/cvs/src/usr.sbin/identd/identd.c,v
retrieving revision 1.32
diff -u -p -u -r1.32 identd.c
--- identd.c16 Oct 2015 05:55:23 -  1.32
+++ identd.c21 Oct 2015 19:50:23 -
@@ -314,7 +314,7 @@ main(int argc, char *argv[])
lerr(1, "signal(SIGPIPE)");
 
if (parent) {
-   if (pledge("stdio proc getpw rpath", NULL) == -1)
+   if (pledge("id stdio proc getpw rpath", NULL) == -1)
err(1, "pledge");
 
SIMPLEQ_INIT(&sc.parent.replies);



Re: nd6 lie

2015-10-21 Thread Alexander Bluhm
On Wed, Oct 21, 2015 at 10:16:59AM +0200, Martin Pieuchot wrote:
> Now that we "fixed" this historical hack, we have:
> 
>   rt_ifa->ifa_ifp == rt_ifp
> 
> Ok?

OK bluhm@

> 
> Index: netinet6/nd6.c
> ===
> RCS file: /cvs/src/sys/netinet6/nd6.c,v
> retrieving revision 1.155
> diff -u -p -r1.155 nd6.c
> --- netinet6/nd6.c1 Oct 2015 09:10:22 -   1.155
> +++ netinet6/nd6.c21 Oct 2015 08:11:05 -
> @@ -689,15 +689,10 @@ nd6_lookup(struct in6_addr *addr6, int c
>* route from a parent route that has the L flag (e.g. the default
>* route to a p2p interface) may have the flag, too, while the
>* destination is not actually a neighbor.
> -  * XXX: we can't use rt->rt_ifp to check for the interface, since
> -  *  it might be the loopback interface if the entry is for our
> -  *  own address on a non-loopback interface. Instead, we should
> -  *  use rt->rt_ifa->ifa_ifp, which would specify the REAL
> -  *  interface.
>*/
>   if ((rt->rt_flags & RTF_GATEWAY) || (rt->rt_flags & RTF_LLINFO) == 0 ||
>   rt->rt_gateway->sa_family != AF_LINK || rt->rt_llinfo == NULL ||
> - (ifp && rt->rt_ifa->ifa_ifp != ifp)) {
> + (ifp != NULL && rt->rt_ifp != ifp)) {
>   if (create) {
>   char addr[INET6_ADDRSTRLEN];
>   nd6log((LOG_DEBUG, "%s: failed to lookup %s (if=%s)\n",



ld.so crash fix

2015-10-21 Thread Peter Hajdu
Hi,

There's an old bug in ld.so preventing sdl2 to be ported to openbsd.
Lately I had time to play with it and continued the work of Henri
Kemppainen.  I think I managed to fix the issue.  I tested the patch
with amd64 and i386 builds.  Could someone please have a look at it?

It introduces some more code duplication that I will deal with
providing my diff is ok.

You can use the following repo to reproduce the bug.
https://github.com/peterhajdu/ld_openbsd_bug

Thank you very much in advance,
Peter Hajdu

Index: dlfcn.c
===
RCS file: /cvs/src/libexec/ld.so/dlfcn.c,v
retrieving revision 1.91
diff -u -p -r1.91 dlfcn.c
--- dlfcn.c 19 Sep 2015 20:56:47 -  1.91
+++ dlfcn.c 21 Oct 2015 13:52:46 -
@@ -302,7 +302,7 @@ _dl_real_close(void *handle)
object->opencount--;
_dl_notify_unload_shlib(object);
_dl_run_all_dtors();
-   _dl_unload_shlib(object);
+   _dl_unload_unused();
_dl_cleanup_objects();
return (0);
 }
Index: library.c
===
RCS file: /cvs/src/libexec/ld.so/library.c,v
retrieving revision 1.71
diff -u -p -r1.71 library.c
--- library.c   16 Jan 2015 16:18:07 -  1.71
+++ library.c   21 Oct 2015 13:52:46 -
@@ -74,6 +74,22 @@ _dl_unload_shlib(elf_object_t *object)
}
 }
 
+void
+_dl_unload_unused(void)
+{
+   elf_object_t *obj, *next;
+
+   for (obj = _dl_objects->next; obj != NULL; obj = next) {
+   next = obj->next;
+   if (OBJECT_REF_CNT(obj) != 0 || obj->status & STAT_UNLOADED)
+   continue;
+   obj->status |= STAT_UNLOADED;
+   _dl_load_list_free(obj->load_list);
+   _dl_munmap((void *)obj->load_base, obj->load_size);
+   _dl_remove_object(obj);
+   }
+}
+
 elf_object_t *
 _dl_tryload_shlib(const char *libname, int type, int flags)
 {
Index: library_mquery.c
===
RCS file: /cvs/src/libexec/ld.so/library_mquery.c,v
retrieving revision 1.49
diff -u -p -r1.49 library_mquery.c
--- library_mquery.c22 Jan 2015 05:48:17 -  1.49
+++ library_mquery.c21 Oct 2015 13:52:46 -
@@ -79,6 +79,20 @@ _dl_unload_shlib(elf_object_t *object)
}
 }
 
+void
+_dl_unload_unused(void)
+{
+   elf_object_t *obj, *next;
+
+   for (obj = _dl_objects->next; obj != NULL; obj = next) {
+   next = obj->next;
+   if (OBJECT_REF_CNT(obj) != 0 || obj->status & STAT_UNLOADED)
+   continue;
+   obj->status |= STAT_UNLOADED;
+   _dl_load_list_free(obj->load_list);
+   _dl_remove_object(obj);
+   }
+}
 
 elf_object_t *
 _dl_tryload_shlib(const char *libname, int type, int flags)
Index: resolve.h
===
RCS file: /cvs/src/libexec/ld.so/resolve.h,v
retrieving revision 1.73
diff -u -p -r1.73 resolve.h
--- resolve.h   19 Sep 2015 20:56:47 -  1.73
+++ resolve.h   21 Oct 2015 13:52:46 -
@@ -223,6 +223,7 @@ void _dl_unlink_dlopen(elf_object_t *dep
 void _dl_notify_unload_shlib(elf_object_t *object);
 void _dl_unload_shlib(elf_object_t *object);
 void _dl_unload_dlopen(void);
+void _dl_unload_unused(void);
 
 void _dl_run_all_dtors(void);
 


Re: Kill frag6 dead code

2015-10-21 Thread Alexander Bluhm
On Wed, Oct 21, 2015 at 10:15:08AM +0200, Martin Pieuchot wrote:
> dstifp is never used.
> 
> ok?

OK bluhm@

> 
> Index: netinet6/frag6.c
> ===
> RCS file: /cvs/src/sys/netinet6/frag6.c,v
> retrieving revision 1.64
> diff -u -p -r1.64 frag6.c
> --- netinet6/frag6.c  19 Oct 2015 11:59:26 -  1.64
> +++ netinet6/frag6.c  21 Oct 2015 08:10:41 -
> @@ -164,30 +164,12 @@ frag6_input(struct mbuf **mp, int *offp,
>   int offset = *offp, nxt, i, next;
>   int first_frag = 0;
>   int fragoff, frgpartlen;/* must be larger than u_int16_t */
> - struct ifnet *dstifp;
> - struct sockaddr_in6 dst;
> - struct rtentry *rt;
>   u_int8_t ecn, ecn0;
>  
>   ip6 = mtod(m, struct ip6_hdr *);
>   IP6_EXTHDR_GET(ip6f, struct ip6_frag *, m, offset, sizeof(*ip6f));
>   if (ip6f == NULL)
>   return IPPROTO_DONE;
> -
> - dstifp = NULL;
> - /* find the destination interface of the packet. */
> - memset(&dst, 0, sizeof(dst));
> - dst.sin6_family = AF_INET6;
> - dst.sin6_len = sizeof(struct sockaddr_in6);
> - dst.sin6_addr = ip6->ip6_dst;
> -
> - rt = rtalloc_mpath(sin6tosa(&dst), &ip6->ip6_src.s6_addr32[0],
> - m->m_pkthdr.ph_rtableid);
> - if (rt != NULL) {
> - dstifp = ifatoia6(rt->rt_ifa)->ia_ifp;
> - rtfree(rt);
> - rt = NULL;
> - }
>  
>   /* jumbo payload can't contain a fragment header */
>   if (ip6->ip6_plen == 0) {



Re: pledge(2) hangman(6)

2015-10-21 Thread Sebastien Marie
On Wed, Oct 21, 2015 at 10:14:49AM +0100, Ricardo Mestre wrote:
> Hi Sebastien,
> 
> Sorry, I totally overlooked signal(3) and that it would call die(), and also
> just tried to play several times but since I never tried to escape it  via
> ctrl+c, not exposing the problem, I removed tty. My bad... I will try harder
> next time if I ever (hopefully) send further patches.
>
> Let's hope this version is ok now:

doug: you have my OK if you want to commit it.

> 
> Index: main.c
> ===
> RCS file: /cvs/src/games/hangman/main.c,v
> retrieving revision 1.12
> diff -u -p -u -r1.12 main.c
> --- main.c  7 Feb 2015 01:37:30 -   1.12
> +++ main.c  21 Oct 2015 09:12:13 -
> @@ -43,6 +43,9 @@ main(int argc, char *argv[])
>  {
> int ch;
> 
> +   if (pledge("stdio rpath tty", NULL) == -1)
> +   err(1, "pledge");
> +
> while ((ch = getopt(argc, argv, "d:hk")) != -1) {
> switch (ch) {
> case 'd':
> @@ -69,6 +72,10 @@ main(int argc, char *argv[])
> }
> signal(SIGINT, die);
> setup();
> +
> +   if (pledge("stdio tty", NULL) == -1)
> +   err(1, "pledge");
> +
> for (;;) {
> Wordnum++;
> playgame();
> 
> Best regards,
> Ricardo Mestre
> 
> On 21/10/2015 09:49, Sebastien Marie wrote:
> >On Wed, Oct 21, 2015 at 08:57:22AM +0100, Ricardo Mestre wrote:
> >>Hi Doug,
> >>
> >>Thank you for taking your time into this!
> >>
> >>I followed your advise and changed malloc to stdio but also tweaked a few
> >>lines later. After initscr() [setting up the screen/tty] and setup() [open
> >>the dictionary file/rpath] we can drop priviliges only to stdio and
> >>hangman(6) will live happily after until the end.
> >"until the end" are you sure ?
> >:)
> >
> >$ hangman
> >+ Hit Ctrl+C for quit
> >
> >Killed
> >$ dmesg | tail -1
> >hangman(23047): syscall 54
> >$ grep 54 /usr/include/sys/syscall.h
> >#define SYS_ioctl   54
> >[...]
> >$
> >
> >On Ctrl+C, hangman call die() function, which call endwin(3), the
> >counter-part of initscr(3).
> >
> >>@@ -69,6 +72,10 @@ main(int argc, char *argv[])
> >> }
> >> signal(SIGINT, die);
> >> setup();
> >>+
> >>+   if (pledge("stdio", NULL) == -1)
> >>+   err(1, "pledge");
> >>+
> >so you still need "tty" here.
> >
> >Regards.
> 
> 

-- 
Sebastien Marie



Re: pledge(2) hangman(6)

2015-10-21 Thread Ricardo Mestre

Hi Sebastien,

Sorry, I totally overlooked signal(3) and that it would call die(), and 
also just tried to play several times but since I never tried to escape 
it  via ctrl+c, not exposing the problem, I removed tty. My bad... I 
will try harder next time if I ever (hopefully) send further patches.


Let's hope this version is ok now:

Index: main.c
===
RCS file: /cvs/src/games/hangman/main.c,v
retrieving revision 1.12
diff -u -p -u -r1.12 main.c
--- main.c  7 Feb 2015 01:37:30 -   1.12
+++ main.c  21 Oct 2015 09:12:13 -
@@ -43,6 +43,9 @@ main(int argc, char *argv[])
 {
int ch;

+   if (pledge("stdio rpath tty", NULL) == -1)
+   err(1, "pledge");
+
while ((ch = getopt(argc, argv, "d:hk")) != -1) {
switch (ch) {
case 'd':
@@ -69,6 +72,10 @@ main(int argc, char *argv[])
}
signal(SIGINT, die);
setup();
+
+   if (pledge("stdio tty", NULL) == -1)
+   err(1, "pledge");
+
for (;;) {
Wordnum++;
playgame();

Best regards,
Ricardo Mestre

On 21/10/2015 09:49, Sebastien Marie wrote:

On Wed, Oct 21, 2015 at 08:57:22AM +0100, Ricardo Mestre wrote:

Hi Doug,

Thank you for taking your time into this!

I followed your advise and changed malloc to stdio but also tweaked a few
lines later. After initscr() [setting up the screen/tty] and setup() [open
the dictionary file/rpath] we can drop priviliges only to stdio and
hangman(6) will live happily after until the end.

"until the end" are you sure ?
:)

$ hangman
+ Hit Ctrl+C for quit

Killed
$ dmesg | tail -1
hangman(23047): syscall 54
$ grep 54 /usr/include/sys/syscall.h
#define SYS_ioctl   54
[...]
$

On Ctrl+C, hangman call die() function, which call endwin(3), the
counter-part of initscr(3).


@@ -69,6 +72,10 @@ main(int argc, char *argv[])
 }
 signal(SIGINT, die);
 setup();
+
+   if (pledge("stdio", NULL) == -1)
+   err(1, "pledge");
+

so you still need "tty" here.

Regards.




Re: reference syscall.h in pledge.2

2015-10-21 Thread Theo de Raadt
>Does it make sense to reference the syscall numbers in pledge(2)?

No not really.  By 5.9 release the kernel printf's will go away,
and people won't get such alerts.  Maybe they will get kernel log's,
but I will consider generating them with system call names.



Re: pledge(2) hangman(6)

2015-10-21 Thread Sebastien Marie
On Wed, Oct 21, 2015 at 08:57:22AM +0100, Ricardo Mestre wrote:
> Hi Doug,
> 
> Thank you for taking your time into this!
> 
> I followed your advise and changed malloc to stdio but also tweaked a few
> lines later. After initscr() [setting up the screen/tty] and setup() [open
> the dictionary file/rpath] we can drop priviliges only to stdio and
> hangman(6) will live happily after until the end.

"until the end" are you sure ? 
:)

$ hangman
+ Hit Ctrl+C for quit

Killed
$ dmesg | tail -1
hangman(23047): syscall 54
$ grep 54 /usr/include/sys/syscall.h
#define SYS_ioctl   54
[...]
$ 

On Ctrl+C, hangman call die() function, which call endwin(3), the
counter-part of initscr(3).

> @@ -69,6 +72,10 @@ main(int argc, char *argv[])
> }
> signal(SIGINT, die);
> setup();
> +
> +   if (pledge("stdio", NULL) == -1)
> +   err(1, "pledge");
> +

so you still need "tty" here.

Regards.
-- 
Sebastien Marie



ifa_ifp and RTF_LOCAL routes

2015-10-21 Thread Martin Pieuchot
Now that (rt_ifa->ifa_ifp == rt_ifp) we can simplify the check below.

Ok?

Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.253
diff -u -p -r1.253 route.c
--- net/route.c 16 Oct 2015 12:36:02 -  1.253
+++ net/route.c 21 Oct 2015 08:12:53 -
@@ -1666,8 +1666,7 @@ rt_if_linkstate_change(struct rtentry *r
 {
struct ifnet *ifp = arg;
 
-   if ((rt->rt_ifp != ifp) &&
-   (rt->rt_ifa == NULL || rt->rt_ifa->ifa_ifp != ifp))
+   if (rt->rt_ifp != ifp)
return (0);
 
/* Local routes are always usable. */



Kill frag6 dead code

2015-10-21 Thread Martin Pieuchot
dstifp is never used.

ok?

Index: netinet6/frag6.c
===
RCS file: /cvs/src/sys/netinet6/frag6.c,v
retrieving revision 1.64
diff -u -p -r1.64 frag6.c
--- netinet6/frag6.c19 Oct 2015 11:59:26 -  1.64
+++ netinet6/frag6.c21 Oct 2015 08:10:41 -
@@ -164,30 +164,12 @@ frag6_input(struct mbuf **mp, int *offp,
int offset = *offp, nxt, i, next;
int first_frag = 0;
int fragoff, frgpartlen;/* must be larger than u_int16_t */
-   struct ifnet *dstifp;
-   struct sockaddr_in6 dst;
-   struct rtentry *rt;
u_int8_t ecn, ecn0;
 
ip6 = mtod(m, struct ip6_hdr *);
IP6_EXTHDR_GET(ip6f, struct ip6_frag *, m, offset, sizeof(*ip6f));
if (ip6f == NULL)
return IPPROTO_DONE;
-
-   dstifp = NULL;
-   /* find the destination interface of the packet. */
-   memset(&dst, 0, sizeof(dst));
-   dst.sin6_family = AF_INET6;
-   dst.sin6_len = sizeof(struct sockaddr_in6);
-   dst.sin6_addr = ip6->ip6_dst;
-
-   rt = rtalloc_mpath(sin6tosa(&dst), &ip6->ip6_src.s6_addr32[0],
-   m->m_pkthdr.ph_rtableid);
-   if (rt != NULL) {
-   dstifp = ifatoia6(rt->rt_ifa)->ia_ifp;
-   rtfree(rt);
-   rt = NULL;
-   }
 
/* jumbo payload can't contain a fragment header */
if (ip6->ip6_plen == 0) {



nd6 lie

2015-10-21 Thread Martin Pieuchot
Now that we "fixed" this historical hack, we have:

rt_ifa->ifa_ifp == rt_ifp

Ok?

Index: netinet6/nd6.c
===
RCS file: /cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.155
diff -u -p -r1.155 nd6.c
--- netinet6/nd6.c  1 Oct 2015 09:10:22 -   1.155
+++ netinet6/nd6.c  21 Oct 2015 08:11:05 -
@@ -689,15 +689,10 @@ nd6_lookup(struct in6_addr *addr6, int c
 * route from a parent route that has the L flag (e.g. the default
 * route to a p2p interface) may have the flag, too, while the
 * destination is not actually a neighbor.
-* XXX: we can't use rt->rt_ifp to check for the interface, since
-*  it might be the loopback interface if the entry is for our
-*  own address on a non-loopback interface. Instead, we should
-*  use rt->rt_ifa->ifa_ifp, which would specify the REAL
-*  interface.
 */
if ((rt->rt_flags & RTF_GATEWAY) || (rt->rt_flags & RTF_LLINFO) == 0 ||
rt->rt_gateway->sa_family != AF_LINK || rt->rt_llinfo == NULL ||
-   (ifp && rt->rt_ifa->ifa_ifp != ifp)) {
+   (ifp != NULL && rt->rt_ifp != ifp)) {
if (create) {
char addr[INET6_ADDRSTRLEN];
nd6log((LOG_DEBUG, "%s: failed to lookup %s (if=%s)\n",



Re: pledge(2) hangman(6)

2015-10-21 Thread Ricardo Mestre

Hi Doug,

Thank you for taking your time into this!

I followed your advise and changed malloc to stdio but also tweaked a 
few lines later. After initscr() [setting up the screen/tty] and setup() 
[open the dictionary file/rpath] we can drop priviliges only to stdio 
and hangman(6) will live happily after until the end.


New patch below:

Index: main.c
===
RCS file: /cvs/src/games/hangman/main.c,v
retrieving revision 1.12
diff -u -p -u -r1.12 main.c
--- main.c  7 Feb 2015 01:37:30 -   1.12
+++ main.c  21 Oct 2015 07:46:19 -
@@ -43,6 +43,9 @@ main(int argc, char *argv[])
 {
int ch;

+   if (pledge("stdio rpath tty", NULL) == -1)
+   err(1, "pledge");
+
while ((ch = getopt(argc, argv, "d:hk")) != -1) {
switch (ch) {
case 'd':
@@ -69,6 +72,10 @@ main(int argc, char *argv[])
}
signal(SIGINT, die);
setup();
+
+   if (pledge("stdio", NULL) == -1)
+   err(1, "pledge");
+
for (;;) {
Wordnum++;
playgame();


Best regards,
Ricardo Mestre

On 21/10/2015 07:13, Doug Hogan wrote:

On Tue, Oct 20, 2015 at 09:04:51PM +0100, Ricardo Mestre wrote:

Let's give some pledge(2) love to hangman(6)!

It seems to work fine for me with the patch mentioned below, nevertheless
please be aware that I don't consider myself a developer, just a mere
OpenBSD user with 'security uncle syndrome' :D

That being said please don't beat me to death for trying to turn this lovely
OS a little bit more secure :)

I think you're close.  You should use "stdio" rather than "malloc"
though.  Out of the over 400 calls to pledge, there's only one call
that uses "malloc" directly and that's in regress. :)  "stdio" includes
"malloc" so that's why few include it directly.  My tree has some
uncommitted diffs so your count may differ.

Anyway, your diff looks good to me if you s/malloc/stdio/ in that line.

ok doug@ if anyone else wants to take a look.


Index: src/games/hangman/main.c
===
RCS file: /cvs/src/games/hangman/main.c,v
retrieving revision 1.12
diff -u -p -u -r1.12 main.c
--- src/games/hangman/main.c7 Feb 2015 01:37:30 -   1.12
+++ src/games/hangman/main.c20 Oct 2015 19:54:01 -
@@ -43,6 +43,9 @@ main(int argc, char *argv[])
  {
 int ch;

+   if (pledge("malloc tty rpath", NULL) == -1)
+   err(1, "pledge");
+
 while ((ch = getopt(argc, argv, "d:hk")) != -1) {
 switch (ch) {
 case 'd':

Best regards,
Ricardo Mestre





reference syscall.h in pledge.2

2015-10-21 Thread Jan Stary
Does it make sense to reference the syscall numbers in pledge(2)?

Jan


--- /usr/src/lib/libc/sys/pledge.2  Thu Oct 15 00:39:04 2015
+++ ./pledge.2  Wed Oct 21 09:41:26 2015
@@ -468,6 +468,9 @@ All other paths will return
 .Er ENOENT .
 .Sh RETURN VALUES
 .Rv -std
+.Sh FILES
+.Pa /usr/include/sys/syscall.h
+defines the syscall numbers.
 .Sh ERRORS
 .Fn pledge
 will fail if: