Re: UBSan instrumentation vs -fno-wrapv

2022-01-30 Thread Otto Moerbeek
On Sun, Jan 30, 2022 at 04:46:36PM -0800, Greg Steuck wrote:

> In case somebody hits this, here's a resolved issue: -fno-wrapv is
> matters for UBSan coverage.
> 
> Confusion starts with:
> 
> $ uname -srm; cat a.c && clang -fsanitize=undefined a.c -c -o a.o && nm a.o
> OpenBSD 7.0 amd64
> int main(int argc, char **argv) {
>   int k = 0x7fff;
>   k += argc;
>   return 0;
> }
>  W __llvm_retpoline_r11
>  W __retguard_2371
>  F a.c
>  T main
> 
> Notice the lack of `__ubsan` symbols. Adding -fno-wrav (which I found in
> kernel Makefile.amd64) restores the desired instrumentation:
> 
> % clang -fsanitize=undefined -fno-wrapv a.c -c -o a.o && nm a.o
>  W __llvm_retpoline_r11
>  W __retguard_2371
>  U __ubsan_handle_add_overflow
>  F a.c
>  T main
> 

With -fwrapv the addition is no longer undefined behaviour, so the
compiler does not need to insert the __ubsan_handle_add_overflow hook. 

-Otto



UBSan instrumentation vs -fno-wrapv

2022-01-30 Thread Greg Steuck
In case somebody hits this, here's a resolved issue: -fno-wrapv is
matters for UBSan coverage.

Confusion starts with:

$ uname -srm; cat a.c && clang -fsanitize=undefined a.c -c -o a.o && nm a.o
OpenBSD 7.0 amd64
int main(int argc, char **argv) {
  int k = 0x7fff;
  k += argc;
  return 0;
}
 W __llvm_retpoline_r11
 W __retguard_2371
 F a.c
 T main

Notice the lack of `__ubsan` symbols. Adding -fno-wrav (which I found in
kernel Makefile.amd64) restores the desired instrumentation:

% clang -fsanitize=undefined -fno-wrapv a.c -c -o a.o && nm a.o
 W __llvm_retpoline_r11
 W __retguard_2371
 U __ubsan_handle_add_overflow
 F a.c
 T main



Re: go, pledge, and dns

2022-01-30 Thread Theo de Raadt
This change is OK with me.

The mdns.allow stuff should be fixed by go recognizing that it doesn't
exist in OpenBSD, and not attempting the open.

Ted Unangst  wrote:

> A go program that uses pledge("dns") mostly works except for two
> incompatibilities with the way golang's dns library works. Otherwise
> pledge("rpath") is required.
> 
> 1. go likes to stat /etc/hosts to check for changes. I think this is
> reasonable behavior. Patch below adds a whitelist to the kernel to permit
> this. (libc does not currently cache results, but it could..?)
> 
> 2. go tries to look a file called mdns.allow which does not exist on openbsd.
> There are several platform dependent branches in go/src/net/conf.go, trying to
> read this file should be avoided on openbsd. Patch left as an exercise.
> 
> Point 2 is also trivially worked around by performing a dummy lookup of
> localhost before enabling pledge, so no urgency, but point 1 requires a code
> change somewhere.
> 
> 
> Index: kern_pledge.c
> ===
> RCS file: /cvs/src/sys/kern/kern_pledge.c,v
> retrieving revision 1.278
> diff -u -p -r1.278 kern_pledge.c
> --- kern_pledge.c 20 Jan 2022 03:43:30 -  1.278
> +++ kern_pledge.c 30 Jan 2022 21:01:43 -
> @@ -733,12 +733,17 @@ pledge_namei(struct proc *p, struct name
>  
>   break;
>   case SYS_stat:
> - /* DNS needs /etc/resolv.conf. */
> + /* DNS needs /etc/{resolv.conf,hosts}. */
>   if ((ni->ni_pledge == PLEDGE_RPATH) &&
> - (pledge & PLEDGE_DNS) &&
> - strcmp(path, "/etc/resolv.conf") == 0) {
> - ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
> - return (0);
> + (pledge & PLEDGE_DNS)) {
> + if (strcmp(path, "/etc/resolv.conf") == 0) {
> + ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
> + return (0);
> + }
> + if (strcmp(path, "/etc/hosts") == 0) {
> + ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
> + return (0);
> + }
>   }
>   break;
>   }
> 



go, pledge, and dns

2022-01-30 Thread Ted Unangst
A go program that uses pledge("dns") mostly works except for two
incompatibilities with the way golang's dns library works. Otherwise
pledge("rpath") is required.

1. go likes to stat /etc/hosts to check for changes. I think this is
reasonable behavior. Patch below adds a whitelist to the kernel to permit
this. (libc does not currently cache results, but it could..?)

2. go tries to look a file called mdns.allow which does not exist on openbsd.
There are several platform dependent branches in go/src/net/conf.go, trying to
read this file should be avoided on openbsd. Patch left as an exercise.

Point 2 is also trivially worked around by performing a dummy lookup of
localhost before enabling pledge, so no urgency, but point 1 requires a code
change somewhere.


Index: kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.278
diff -u -p -r1.278 kern_pledge.c
--- kern_pledge.c   20 Jan 2022 03:43:30 -  1.278
+++ kern_pledge.c   30 Jan 2022 21:01:43 -
@@ -733,12 +733,17 @@ pledge_namei(struct proc *p, struct name
 
break;
case SYS_stat:
-   /* DNS needs /etc/resolv.conf. */
+   /* DNS needs /etc/{resolv.conf,hosts}. */
if ((ni->ni_pledge == PLEDGE_RPATH) &&
-   (pledge & PLEDGE_DNS) &&
-   strcmp(path, "/etc/resolv.conf") == 0) {
-   ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
-   return (0);
+   (pledge & PLEDGE_DNS)) {
+   if (strcmp(path, "/etc/resolv.conf") == 0) {
+   ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
+   return (0);
+   }
+   if (strcmp(path, "/etc/hosts") == 0) {
+   ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
+   return (0);
+   }
}
break;
}



Re: Missing UBSan libs

2022-01-30 Thread Patrick Wildt
Am Sun, Jan 30, 2022 at 11:40:29AM -0800 schrieb Greg Steuck:
> Greg Steuck  writes:
> 
> >> I notice people keep sending fixes to problems reported by UBSan. I
> >> wanted to join the club, but the trivial thing listed at
> >> https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html doesn't
> >> work:
> 
> My confusion is easily resolved. People use UBSan in the kernel where
> anton@ did the integration work whereas userspace work is yet to happen.
> 
> Thanks
> Greg

Hi,

regarding the missing userpace support:  Since a few clang updates ago
we import more than just the builtins of compiler-rt.  This means we
should have at least some related code in our tree, even if it is not
built/complete.  From the recent static analyzer mail thread it looks
like people prefer to have such stuff in ports-clang, so, whatever.

Cheers,
Patrick



Re: Missing UBSan libs

2022-01-30 Thread Greg Steuck
Greg Steuck  writes:

>> I notice people keep sending fixes to problems reported by UBSan. I
>> wanted to join the club, but the trivial thing listed at
>> https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html doesn't
>> work:

My confusion is easily resolved. People use UBSan in the kernel where
anton@ did the integration work whereas userspace work is yet to happen.

Thanks
Greg



Re: Missing UBSan libs

2022-01-30 Thread Greg Steuck
To add a bit more color, the same problem happens with ports clang from
llvm package:

% pkg_info -I llvm
llvm-13.0.0 modular, fast C/C++/ObjC compiler, static analyzer and tools
% cat a.cc; /usr/local/bin/clang++ -fsanitize=undefined a.cc; ./a.out 
int main(int argc, char **argv) {
  int k = 0x7fff;
  k += argc;
  return 0;
}
ld: error: cannot open 
/usr/local/lib/clang/13.0.0/lib/openbsd/libclang_rt.ubsan_standalone-x86_64.a: 
No such file or directory
ld: error: cannot open 
/usr/local/lib/clang/13.0.0/lib/openbsd/libclang_rt.ubsan_standalone_cxx-x86_64.a:
 No such file or directory
clang-13: error: linker command failed with exit code 1 (use -v to see 
invocation)

Greg Steuck  writes:

> I notice people keep sending fixes to problems reported by UBSan. I
> wanted to join the club, but the trivial thing listed at
> https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html doesn't
> work:
>
> $ cat a.cc; clang++ -fsanitize=undefined a.cc; ./a.out
> int main(int argc, char **argv) {
>   int k = 0x7fff;
>   k += argc;
>   return 0;
> }
> ld: error: cannot open 
> /usr/lib/clang/13.0.0/lib/openbsd/libclang_rt.ubsan_standalone-x86_64.a: No 
> such file or directory
> ld: error: cannot open 
> /usr/lib/clang/13.0.0/lib/openbsd/libclang_rt.ubsan_standalone_cxx-x86_64.a: 
> No such file or directory
> clang++: error: linker command failed with exit code 1 (use -v to see 
> invocation)
>
> Did we forget to ship the ubsan libs? The only related files in sets:
>
> ./usr/lib/clang/13.0.0/lib
> ./usr/lib/clang/13.0.0/lib/libclang_rt.profile.a
>
> Thanks
> Greg



Re: in4_cksum changes, step 1

2022-01-30 Thread Miod Vallat
> > -   sum += in_cksumdata((caddr_t) &ipov, sizeof(ipov));
> > +   sum += in_cksumdata((caddr_t) &ipov + 8, sizeof(ipov) - 8);
> 
> I think this would be clearer with a comment.

Sure, added one.

> Please remove the trailing space that some of the changed lines have.

Ok. Updated patch below.

> > Index: sys/arch/sparc64/sparc64/in4_cksum.c

> > +   __asm volatile(
> > +   " lduw [%5 + 12], %1; "
> > +   " lduw [%5 + 16], %2; "
> > " mov -1, %3; add %0, %1, %0; "
> > " srl %3, 0, %3; add %0, %2, %0; "
> > -   " srlx %0, 32, %2; and %0, %3, %1; "
> > +   " srlx %0, 32, %2; "
> > " add %0, %2, %0; "
> > : "=r" (sum), "=&r" (tmp1), "=&r" (tmp2), "=&r" (tmp3)
> > -   : "0" (sum), "r" (w));
> > +   : "0" (sum), "r" (&ipov));
> 
> I might be missing something, but is the temporary register %3 needed
> at all?

Yes, it is set to -1 and used for shifts, at the moment.

Miod

Index: sys/arch/alpha/alpha/in_cksum.c
===
RCS file: /OpenBSD/src/sys/arch/alpha/alpha/in_cksum.c,v
retrieving revision 1.9
diff -u -p -r1.9 in_cksum.c
--- sys/arch/alpha/alpha/in_cksum.c 21 Aug 2014 14:24:08 -  1.9
+++ sys/arch/alpha/alpha/in_cksum.c 30 Jan 2022 18:35:18 -
@@ -200,7 +200,7 @@ in4_cksum(struct mbuf *m, u_int8_t nxt, 
int clen = 0;
caddr_t addr;
union q_util q_util;
-   union l_util l_util; 
+   union l_util l_util;
struct ipovly ipov;
 
if (nxt != 0) {
@@ -212,14 +212,14 @@ in4_cksum(struct mbuf *m, u_int8_t nxt, 
panic("in4_cksum: bad mbuf chain");
 #endif
 
-   memset(&ipov, 0, sizeof(ipov));
-
-   ipov.ih_len = htons(len);
+   ipov.ih_x1[8] = 0;
ipov.ih_pr = nxt;
+   ipov.ih_len = htons(len);
ipov.ih_src = mtod(m, struct ip *)->ip_src;
ipov.ih_dst = mtod(m, struct ip *)->ip_dst;
 
-   sum += in_cksumdata((caddr_t) &ipov, sizeof(ipov));
+   /* first 8 bytes are zeroes */
+   sum += in_cksumdata((caddr_t) &ipov + 8, sizeof(ipov) - 8);
}
 
/* skip over unnecessary part */
@@ -241,7 +241,7 @@ in4_cksum(struct mbuf *m, u_int8_t nxt, 
sum += in_cksumdata(addr, mlen) << 8;
else
sum += in_cksumdata(addr, mlen);
- 
+
clen += mlen;
len -= mlen;
}
Index: sys/arch/m88k/m88k/in_cksum.c
===
RCS file: /OpenBSD/src/sys/arch/m88k/m88k/in_cksum.c,v
retrieving revision 1.4
diff -u -p -r1.4 in_cksum.c
--- sys/arch/m88k/m88k/in_cksum.c   21 Aug 2014 14:24:08 -  1.4
+++ sys/arch/m88k/m88k/in_cksum.c   30 Jan 2022 18:35:18 -
@@ -95,19 +95,22 @@ in4_cksum(struct mbuf *m, uint8_t nxt, i
 {
u_int16_t *w;
u_int sum = 0;
-   struct ipovly ipov;
+   union {
+   struct ipovly ipov;
+   u_int16_t w[10];
+   } u;
 
if (nxt != 0) {
/* pseudo header */
-   bzero(&ipov, sizeof(ipov));
-   ipov.ih_len = htons(len);
-   ipov.ih_pr = nxt; 
-   ipov.ih_src = mtod(m, struct ip *)->ip_src; 
-   ipov.ih_dst = mtod(m, struct ip *)->ip_dst;
-   w = (u_int16_t *)&ipov;
-   /* assumes sizeof(ipov) == 20 */
-   sum += w[0]; sum += w[1]; sum += w[2]; sum += w[3]; sum += w[4];
-   sum += w[5]; sum += w[6]; sum += w[7]; sum += w[8]; sum += w[9];
+   u.ipov.ih_x1[8] = 0;
+   u.ipov.ih_pr = nxt;
+   u.ipov.ih_len = htons(len);
+   u.ipov.ih_src = mtod(m, struct ip *)->ip_src;
+   u.ipov.ih_dst = mtod(m, struct ip *)->ip_dst;
+   w = u.w;
+   /* assumes sizeof(ipov) == 20 and first 8 bytes are zeroes */
+   sum += w[4]; sum += w[5]; sum += w[6];
+   sum += w[7]; sum += w[8]; sum += w[9];
}
 
/* skip unnecessary part */
Index: sys/arch/powerpc/powerpc/in_cksum.c
===
RCS file: /OpenBSD/src/sys/arch/powerpc/powerpc/in_cksum.c,v
retrieving revision 1.10
diff -u -p -r1.10 in_cksum.c
--- sys/arch/powerpc/powerpc/in_cksum.c 22 Jul 2014 10:35:35 -  1.10
+++ sys/arch/powerpc/powerpc/in_cksum.c 30 Jan 2022 18:35:18 -
@@ -87,7 +87,7 @@ in_cksum_internal(struct mbuf *m, int of
 * of a word spanning between this mbuf and the
 * last mbuf.
 *
-* s_util.c[0] is already saved when scanning previous 
+* s_util.c[0] is

tr(1): improve table names

2022-01-30 Thread Scott Cheloha
In tr(1), we have these two global arrays, "string1" and "string2".

I have a few complaints:

1. They are not strings.  They are lookup tables.  The names are
   misleading.

2. The arguments given to tr(1) in argv[] are indeed called "string1"
   and "string2".  These are the names used in the standard, the manpage,
   and the usage printout.

   However, the lookup tables are merely *described* by these arguments.
   They are not the arguments themselves.

3. The meaning of the contents of these lookup tables changes depending
   on which of the five different operating modes tr(1) is running in.

   string1[i] might mean "delete byte i" or "squeeze byte i" or
   "replace byte i with the value string1[i]" depending on how
   tr(1) was invoked.

Given this, I think it'd be a lot nicer if we named the tables to
indicate which transformation operation they correspond to.

We have three such operations: "delete", "squeeze", and "translate".
So we ought to have a table for each.  And in setup() we should call
the table a "table", not a "string".

Now when you look at the loops in main() you can immediately
understand which operation is happening without needing to consult the
manpage or the comments.  (Seriously, look.)

I have more cleanup I want to do here in tr.c, but I think renaming
these tables first is going to make the rest of it a lot easier to
review.

ok?

Index: tr.c
===
RCS file: /cvs/src/usr.bin/tr/tr.c,v
retrieving revision 1.20
diff -u -p -r1.20 tr.c
--- tr.c2 Nov 2021 15:45:52 -   1.20
+++ tr.c30 Jan 2022 16:14:21 -
@@ -40,7 +40,8 @@
 
 #include "extern.h"
 
-static int string1[NCHARS] = {
+int delete[NCHARS], squeeze[NCHARS];
+int translate[NCHARS] = {
0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, /* ASCII */
0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
@@ -73,7 +74,7 @@ static int string1[NCHARS] = {
0xe8, 0xe9, 0xea, 0xeb, 0xec, 0xed, 0xee, 0xef,
0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7,
0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff,
-}, string2[NCHARS];
+};
 
 STR s1 = { STRING1, NORMAL, 0, OOBCH, { 0, OOBCH }, NULL, NULL };
 STR s2 = { STRING2, NORMAL, 0, OOBCH, { 0, OOBCH }, NULL, NULL };
@@ -122,11 +123,11 @@ main(int argc, char *argv[])
if (argc != 2)
usage();
 
-   setup(string1, argv[0], &s1, cflag);
-   setup(string2, argv[1], &s2, 0);
+   setup(delete, argv[0], &s1, cflag);
+   setup(squeeze, argv[1], &s2, 0);
 
for (lastch = OOBCH; (ch = getchar()) != EOF;)
-   if (!string1[ch] && (!string2[ch] || lastch != ch)) {
+   if (!delete[ch] && (!squeeze[ch] || lastch != ch)) {
lastch = ch;
(void)putchar(ch);
}
@@ -141,10 +142,10 @@ main(int argc, char *argv[])
if (argc != 1)
usage();
 
-   setup(string1, argv[0], &s1, cflag);
+   setup(delete, argv[0], &s1, cflag);
 
while ((ch = getchar()) != EOF)
-   if (!string1[ch])
+   if (!delete[ch])
(void)putchar(ch);
exit(0);
}
@@ -154,10 +155,10 @@ main(int argc, char *argv[])
 * Squeeze all characters (or complemented characters) in string1.
 */
if (sflag && argc == 1) {
-   setup(string1, argv[0], &s1, cflag);
+   setup(squeeze, argv[0], &s1, cflag);
 
for (lastch = OOBCH; (ch = getchar()) != EOF;)
-   if (!string1[ch] || lastch != ch) {
+   if (!squeeze[ch] || lastch != ch) {
lastch = ch;
(void)putchar(ch);
}
@@ -177,7 +178,7 @@ main(int argc, char *argv[])
s2.str = (unsigned char *)argv[1];
 
if (cflag)
-   for (cnt = NCHARS, p = string1; cnt--;)
+   for (cnt = NCHARS, p = translate; cnt--;)
*p++ = OOBCH;
 
if (!next(&s2))
@@ -187,45 +188,45 @@ main(int argc, char *argv[])
ch = s2.lastch;
if (sflag)
while (next(&s1)) {
-   string1[s1.lastch] = ch = s2.lastch;
-   string2[ch] = 1;
+   translate[s1.lastch] = ch = s2.lastch;
+   squeeze[ch] = 1;
(void)next(&s2);
}
else
while (next(&s1)) {
-   string1[s1.lastch] = ch = s2.lastch;
+   translate[s1.lastch] = ch = s2.lastch;
(void)next(&s2);
}
 
if (cflag)
-   f

Re: syslogd(8): Add hostname parsing support

2022-01-30 Thread Martijn van Duren
On Wed, 2022-01-26 at 09:18 -0700, Theo de Raadt wrote:
> > However, as things stand interpretation can be broken with the base
> > tools. I can't fix garbage input.
> 
> Your proposal builds a mechanism which encourages making decisions based
> upon parsing garbage input.

So let's just focus on my original diff and let that one stand on its
own.

Say I have a sender with the following line:
sender$ tail -2 /etc/syslog.conf
!doas
*.* @100.64.2.2

and a receiver with:
receiver$ tail -6 /etc/syslog.conf 
+100.64.2.3
!doas
*.* /tmp/prog_doas

!sender
*.* /tmp/prog_sender

receiver runs syslogd with -U100.64.2.2

When running syslogd on sender without any flags and doing a simple
`doas ls`:
receiver$ tail /tmp/prog_*
==> /tmp/prog_doas <==
Jan 30 15:03:51 100.64.2.3 doas: martijn ran command ls as root from 
/home/martijn

==> /tmp/prog_sender <==
receiver$ 

When running syslogd on sender with -h and doing a simple `doas ls`:
receiver$ tail /tmp/prog_*
==> /tmp/prog_doas <==

==> /tmp/prog_sender <==
Jan 30 15:08:08 100.64.2.3 sender doas: martijn ran command ls as root from 
/home/martijn
receiver$ 

With my diff applied and without -h:
receiver$ tail /tmp/prog_*
==> /tmp/prog_doas <==
Jan 30 15:11:55 100.64.2.3 doas: martijn ran command ls as root from 
/home/martijn

==> /tmp/prog_sender <==
receiver$ 

With my diff applied and with -h:
receiver$ tail /tmp/prog_*
==> /tmp/prog_doas <==
Jan 30 15:12:47 100.64.2.3 doas: martijn ran command ls as root from 
/home/martijn

==> /tmp/prog_sender <==
receiver$ 

So no new mechanism is introduced, but I am trying to make sure that
existing mechanisms work more consistent with what we already offer
out of the box in base.

martijn@

Index: parsemsg.c
===
RCS file: /cvs/src/usr.sbin/syslogd/parsemsg.c,v
retrieving revision 1.1
diff -u -p -r1.1 parsemsg.c
--- parsemsg.c  13 Jan 2022 10:34:07 -  1.1
+++ parsemsg.c  30 Jan 2022 14:20:36 -
@@ -17,8 +17,14 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include 
+
+#include 
+#include 
+
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -29,27 +35,42 @@
 
 size_t parsemsg_timestamp_bsd(const char *, char *);
 size_t parsemsg_timestamp_v1(const char *, char *);
+size_t parsemsg_hostname(const char *, char *);
 size_t parsemsg_prog(const char *, char *);
 
 struct msg *
 parsemsg(const char *msgstr, struct msg *msg)
 {
-   size_t n;
+   size_t timelen, proglen;
+   const char *hostname;
 
msg->m_pri = -1;
msgstr += parsemsg_priority(msgstr, &msg->m_pri);
if (msg->m_pri &~ (LOG_FACMASK|LOG_PRIMASK))
msg->m_pri = -1;
 
-   if ((n = parsemsg_timestamp_bsd(msgstr, msg->m_timestamp)) == 0)
-   n = parsemsg_timestamp_v1(msgstr, msg->m_timestamp);
-   msgstr += n;
+   if ((timelen = parsemsg_timestamp_bsd(msgstr, msg->m_timestamp)) == 0)
+   timelen = parsemsg_timestamp_v1(msgstr, msg->m_timestamp);
+   msgstr += timelen;
 
-   while (isspace(msgstr[0]))
+   while (isspace((unsigned char)msgstr[0]))
msgstr++;
 
-   parsemsg_prog(msgstr, msg->m_prog);
+   hostname = msgstr;
+   msgstr += parsemsg_hostname(msgstr, msg->m_hostname);
+ 
+   while (isspace((unsigned char)msgstr[0]))
+   msgstr++;
 
+   proglen = parsemsg_prog(msgstr, msg->m_prog);
+
+   /*
+* Without timestamp and tag, assume hostname as part of message.
+*/
+   if (!timelen && !proglen) {
+   msg->m_hostname[0] = '\0';
+   msgstr = hostname;
+   }
strlcpy(msg->m_msg, msgstr, sizeof(msg->m_msg));
 
return msg;
@@ -169,6 +190,47 @@ parsemsg_timestamp_v1(const char *msgstr
return msg - msgstr;
 }
 
+/*
+ * Parse the ip address or hostname according to inet_pton and res_hnok and
+ * return the length of the hostname including the trailing space if available.
+ */
+size_t
+parsemsg_hostname(const char *msgstr, char *hostname)
+{
+   size_t len;
+   struct in_addr buf4;
+   struct in6_addr buf6;
+
+   if (msgstr[0] == '-' && (msgstr[1] == ' ' || msgstr[1] == '\0')) {
+   hostname[0] = '\0';
+   if (msgstr[1] == '\0')
+   return 1;
+   return 2;
+   }
+
+   if ((len = strcspn(msgstr, " ")) > HOST_NAME_MAX)
+   return 0;
+   strlcpy(hostname, msgstr, len + 1);
+   if (msgstr[len] == ' ')
+   len++;
+
+   if (inet_pton(AF_INET, hostname, &buf4) == 1 ||
+   inet_pton(AF_INET6, hostname, &buf6) == 1) {
+   return len;
+   }
+
+   if (res_hnok(hostname) == 0) {
+   hostname[0] = '\0';
+   return 0;
+   }
+   

smtpd: use libtls signer

2022-01-30 Thread Eric Faurot
Hi.

This diff makes use of the new libtls signer api to simplify tls privsep.

Eric.

Index: ca.c
===
RCS file: /cvs/src/usr.sbin/smtpd/ca.c,v
retrieving revision 1.40
diff -u -p -r1.40 ca.c
--- ca.c14 Jun 2021 17:58:15 -  1.40
+++ ca.c26 Jan 2022 14:01:15 -
@@ -1,6 +1,7 @@
 /* $OpenBSD: ca.c,v 1.40 2021/06/14 17:58:15 eric Exp $*/
 
 /*
+ * Copyright (c) 2021 Eric Faurot 
  * Copyright (c) 2014 Reyk Floeter 
  * Copyright (c) 2012 Gilles Chehade 
  *
@@ -17,45 +18,23 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-#include 
-#include 
+#include 
 #include 
 #include 
+#include 
 #include 
+#include 
 #include 
 
 #include "smtpd.h"
-#include "log.h"
 #include "ssl.h"
+#include "log.h"
 
-static int  ca_verify_cb(int, X509_STORE_CTX *);
-
-static int  rsae_send_imsg(int, const unsigned char *, unsigned char *,
-   RSA *, int, unsigned int);
-static int  rsae_pub_enc(int, const unsigned char *, unsigned char *,
-   RSA *, int);
-static int  rsae_pub_dec(int,const unsigned char *, unsigned char *,
-   RSA *, int);
-static int  rsae_priv_enc(int, const unsigned char *, unsigned char *,
-   RSA *, int);
-static int  rsae_priv_dec(int, const unsigned char *, unsigned char *,
-   RSA *, int);
-static int  rsae_mod_exp(BIGNUM *, const BIGNUM *, RSA *, BN_CTX *);
-static int  rsae_bn_mod_exp(BIGNUM *, const BIGNUM *, const BIGNUM *,
-   const BIGNUM *, BN_CTX *, BN_MONT_CTX *);
-static int  rsae_init(RSA *);
-static int  rsae_finish(RSA *);
-static int  rsae_keygen(RSA *, int, BIGNUM *, BN_GENCB *);
-
-static ECDSA_SIG *ecdsae_do_sign(const unsigned char *, int, const BIGNUM *,
-const BIGNUM *, EC_KEY *);
-static int ecdsae_sign_setup(EC_KEY *, BN_CTX *, BIGNUM **, BIGNUM **);
-static int ecdsae_do_verify(const unsigned char *, int, const ECDSA_SIG *,
-EC_KEY *);
-
+static void ca_imsg(struct mproc *, struct imsg *);
+static void ca_init(void);
 
-static struct dict pkeys;
-static uint64_t reqid = 0;
+static struct tls_signer *signer;
+static uint64_t reqid = 0;
 
 static void
 ca_shutdown(void)
@@ -69,7 +48,9 @@ ca(void)
 {
struct passwd   *pw;
 
-   
purge_config(PURGE_LISTENERS|PURGE_TABLES|PURGE_RULES|PURGE_DISPATCHERS);
+   ca_init();
+
+   purge_config(PURGE_EVERYTHING);
 
if ((pw = getpwnam(SMTPD_USER)) == NULL)
fatalx("unknown user " SMTPD_USER);
@@ -98,9 +79,6 @@ ca(void)
config_peer(PROC_PARENT);
config_peer(PROC_DISPATCHER);
 
-   /* Ignore them until we get our config */
-   mproc_disable(p_dispatcher);
-
if (pledge("stdio", NULL) == -1)
fatal("pledge");
 
@@ -110,120 +88,35 @@ ca(void)
return (0);
 }
 
-void
+static void
 ca_init(void)
 {
-   BIO *in = NULL;
-   EVP_PKEY*pkey = NULL;
-   struct pki  *pki;
-   const char  *k;
-   void*iter_dict;
-   char*hash;
+   struct pki *pki;
+   void *iter_dict;
+
+   if ((signer = tls_signer_new()) == NULL)
+   fatal("tls_signer_new");
 
-   log_debug("debug: init private ssl-tree");
-   dict_init(&pkeys);
iter_dict = NULL;
-   while (dict_iter(env->sc_pki_dict, &iter_dict, &k, (void **)&pki)) {
+   while (dict_iter(env->sc_pki_dict, &iter_dict, NULL, (void **)&pki)) {
if (pki->pki_key == NULL)
continue;
-
-   in = BIO_new_mem_buf(pki->pki_key, pki->pki_key_len);
-   if (in == NULL)
-   fatalx("ca_init: key");
-   pkey = PEM_read_bio_PrivateKey(in, NULL, NULL, NULL);
-   if (pkey == NULL)
-   fatalx("ca_init: PEM");
-   BIO_free(in);
-
-   hash = ssl_pubkey_hash(pki->pki_cert, pki->pki_cert_len);
-   if (dict_check(&pkeys, hash))
-   EVP_PKEY_free(pkey);
-   else
-   dict_xset(&pkeys, hash, pkey);
-   free(hash);
+   if (tls_signer_add_keypair_mem(signer, pki->pki_cert,
+   pki->pki_cert_len, pki->pki_key, pki->pki_key_len) == -1)
+   fatalx("ca_init: tls_signer_add_keypair_mem");
}
 }
 
-static int
-ca_verify_cb(int ok, X509_STORE_CTX *ctx)
-{
-   switch (X509_STORE_CTX_get_error(ctx)) {
-   case X509_V_OK:
-   break;
-case X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT:
-   break;
-case X509_V_ERR_CERT_NOT_YET_VALID:
-case X509_V_ERR_ERROR_IN_CERT_NOT_BEFORE_FIELD:
-   break;
-case X509_V_ERR_CERT_HAS_EXPIRED:
-case X509_V_ERR_ERROR_IN_CERT_NOT_AFTER_FIELD:
-   break;
-case X509_V_ERR_NO_