Re: EVFILT_TIMER add support for different timer precisions NOTE_{,U,N,M}SECONDS
On 22/09/10 01:53PM, Visa Hankala wrote: > On Wed, Aug 31, 2022 at 04:48:37PM -0400, aisha wrote: > > I've added a patch which adds support for NOTE_{,U,M,N}SECONDS for > > EVFILT_TIMER in the kqueue interface. > > It sort of makes sense to add an option to specify timeouts in > sub-millisecond precision. It feels complete overengineering to add > multiple time units on the level of the kernel interface. However, > it looks that FreeBSD and NetBSD have already done this following > macOS' lead... > > > I've also added the NOTE_ABSTIME but haven't done any actual implementation > > there as I am not sure how the `data` field should be interpreted (is it > > absolute time in seconds since epoch?). > > I think FreeBSD and NetBSD take NOTE_ABSTIME as time since the epoch. > > Below is a revised patch that takes into account some corner cases. > It tries to be API-compatible with FreeBSD and NetBSD. I have adjusted > the NOTE_{,M,U,N}SECONDS flags so that they are enum-like. > > The manual page bits are from NetBSD. > > It is quite late to introduce a feature like this within this release > cycle. Until now, the timer code has ignored the fflags field. There > might be pieces of software that are careless with struct kevent and > that would break as a result of this patch. Programs that are widely > used on different BSDs are probably fine already, though. > Sorry, I had forgotten this patch for a long time!!! I've been running with this for a while now and it's been working nicely. OK aisha@ I had an unrelated question inlined. > Index: lib/libc/sys/kqueue.2 > === > RCS file: src/lib/libc/sys/kqueue.2,v > retrieving revision 1.46 > diff -u -p -r1.46 kqueue.2 > --- lib/libc/sys/kqueue.2 31 Mar 2022 17:27:16 - 1.46 > +++ lib/libc/sys/kqueue.2 10 Sep 2022 13:01:36 - > @@ -457,17 +457,71 @@ Establishes an arbitrary timer identifie > .Fa ident . > When adding a timer, > .Fa data > -specifies the timeout period in milliseconds. > -The timer will be periodic unless > +specifies the timeout period in units described below, or, if > +.Dv NOTE_ABSTIME > +is set in > +.Va fflags , > +specifies the absolute time at which the timer should fire. > +The timer will repeat unless > .Dv EV_ONESHOT > -is specified. > +is set in > +.Va flags > +or > +.Dv NOTE_ABSTIME > +is set in > +.Va fflags . > On return, > .Fa data > contains the number of times the timeout has expired since the last call to > .Fn kevent . > -This filter automatically sets the > +This filter automatically sets > .Dv EV_CLEAR > -flag internally. > +in > +.Va flags > +for periodic timers. > +Timers created with > +.Dv NOTE_ABSTIME > +remain activated on the kqueue once the absolute time has passed unless > +.Dv EV_CLEAR > +or > +.Dv EV_ONESHOT > +are also specified. > +.Pp > +The filter accepts the following flags in the > +.Va fflags > +argument: > +.Bl -tag -width NOTE_MSECONDS > +.It Dv NOTE_SECONDS > +The timer value in > +.Va data > +is expressed in seconds. > +.It Dv NOTE_MSECONDS > +The timer value in > +.Va data > +is expressed in milliseconds. > +.It Dv NOTE_USECONDS > +The timer value in > +.Va data > +is expressed in microseconds. > +.It Dv NOTE_NSECONDS > +The timer value in > +.Va data > +is expressed in nanoseconds. > +.It Dv NOTE_ABSTIME > +The timer value is an absolute time with > +.Dv CLOCK_REALTIME > +as the reference clock. > +.El > +.Pp > +Note that > +.Dv NOTE_SECONDS , > +.Dv NOTE_MSECONDS , > +.Dv NOTE_USECONDS , > +and > +.Dv NOTE_NSECONDS > +are mutually exclusive; behavior is undefined if more than one are specified. > +If a timer value unit is not specified, the default is > +.Dv NOTE_MSECONDS . > .Pp > If an existing timer is re-added, the existing timer and related pending > events > will be cancelled. > @@ -557,6 +611,7 @@ No memory was available to register the > The specified process to attach to does not exist. > .El > .Sh SEE ALSO > +.Xr clock_gettime 2 , > .Xr poll 2 , > .Xr read 2 , > .Xr select 2 , > Index: regress/sys/kern/kqueue/kqueue-timer.c > === > RCS file: src/regress/sys/kern/kqueue/kqueue-timer.c,v > retrieving revision 1.4 > diff -u -p -r1.4 kqueue-timer.c > --- regress/sys/kern/kqueue/kqueue-timer.c12 Jun 2021 13:30:14 - > 1.4 > +++ regress/sys/kern/kqueue/kqueue-timer.c10 Sep 2022 13:01:37 - > @@ -22,6 +22,7 @@ > #include > #include > #include > +#include > #include > #include >
Re: add extract example to tar(1) man page
On 23/08/03 04:26PM, Jason McIntyre wrote: > On Thu, Aug 03, 2023 at 11:05:16AM -0400, aisha wrote: > > On 23/08/03 10:51AM, aisha wrote: > > > On 23/08/03 09:45AM, Stuart Henderson wrote: > > > > On 2023/08/03 07:23, Jason McIntyre wrote: > > > > > On Wed, Aug 02, 2023 at 05:52:02PM -0400, aisha wrote: > yes, thanks! ok. > > but you should change the text from > > Create a gzip(1) compressed archive ... to a file called... > to > Create a gzip(1) compressed archive, called XXX, containing the > files ... > > as i suggested, because the syntax "create ... to a file" reads badly. > Thanks, fixed. OK? Index: tar.1 === RCS file: /cvs/src/bin/pax/tar.1,v retrieving revision 1.64 diff -u -p -r1.64 tar.1 --- tar.1 31 Mar 2022 17:27:14 - 1.64 +++ tar.1 3 Aug 2023 16:14:11 - @@ -331,16 +331,16 @@ and .Pp .Dl $ tar c bonvole sekve .Pp -Output a +Create a .Xr gzip 1 -compressed archive containing the files +compressed archive, called +.Pa foriru.tar.gz , +containing the files .Pa bonvole and -.Pa sekve -to a file called -.Pa foriru.tar.gz : +.Pa sekve : .Pp -.Dl $ tar zcf foriru.tar.gz bonvole sekve +.Dl $ tar czf foriru.tar.gz bonvole sekve .Pp Verbosely create an archive, called .Pa backup.tar.gz , @@ -349,7 +349,7 @@ of all files matching the shell function .Pa *.c : .Pp -.Dl $ tar zcvf backup.tar.gz *.c +.Dl $ tar cvzf backup.tar.gz *.c .Pp Verbosely list, but do not extract, all files ending in .Pa .jpeg @@ -358,6 +358,13 @@ from a compressed archive named Note that the glob pattern has been quoted to avoid expansion by the shell: .Pp .Dl $ tar tvzf backup.tar.gz '*.jpeg' +.Pp +Verbosely extract an archive, called +.Pa foo.tar.gz , +to the directory +.Pa /var/foo : +.Pp +.Dl $ tar xvzf foo.tar.gz -C /var/foo .Pp For more detailed examples, see .Xr pax 1 .
Re: add extract example to tar(1) man page
On 23/08/03 10:51AM, aisha wrote: > On 23/08/03 09:45AM, Stuart Henderson wrote: > > On 2023/08/03 07:23, Jason McIntyre wrote: > > > On Wed, Aug 02, 2023 at 05:52:02PM -0400, aisha wrote: > > > > Hi, > > > > Someone - https://www.youtube.com/watch?v=NQ5uD5x8vzg - mentioned > > > > that our man page for tar(1) doesn't have an extract example, so I > > > > thought it would be good to add a simple one which highlights a common > > > > use case. > > > > > > > > OK? > > > > > > > > > > hi. > > > > > > the examples section is small enough that i suppose it wouldn;t be a > > > problem to add another one. > > > > as it must be specified in a non-obvious place on the command line and > > it's not currently explained particularly clearly, I'd welcome an example > > using -C. > > > > > it does add another example with a similar set of options though, all in > > > a different order. i wonder whether we should try and push the action as > > > the first option, so people can see what we're doing. so "cXXX" when the > > > example is to create, "tXXX" when listing? then keep the "vzf" options > > > that are so common in the same order? > > > > > > then the second example is probably more helpful as "Create a gzip(1) > > > compressed archive blah.tgz". > > > > > > i know this isn;t what you're posting about, so feel free to leave alone > > > if you don;t want to tackle that. > > > > > > one more thing. you could as easily remove the text "the folder", but > > > i'd be tempted to use "directory", as that's more standard for our docs, > > > and how -C itself describes it. > > > > strongly agree on options order and "directory". > > Thanks for the comments! > Here's the updated patch. > > OK? > Small change to address the backup/-p comments, I've removed the word -backup from the tar archive name OK? Index: tar.1 === RCS file: /cvs/src/bin/pax/tar.1,v retrieving revision 1.64 diff -u -p -r1.64 tar.1 --- tar.1 31 Mar 2022 17:27:14 - 1.64 +++ tar.1 3 Aug 2023 15:02:49 - @@ -331,7 +331,7 @@ and .Pp .Dl $ tar c bonvole sekve .Pp -Output a +Create a .Xr gzip 1 compressed archive containing the files .Pa bonvole @@ -340,7 +340,7 @@ and to a file called .Pa foriru.tar.gz : .Pp -.Dl $ tar zcf foriru.tar.gz bonvole sekve +.Dl $ tar czf foriru.tar.gz bonvole sekve .Pp Verbosely create an archive, called .Pa backup.tar.gz , @@ -349,7 +349,7 @@ of all files matching the shell function .Pa *.c : .Pp -.Dl $ tar zcvf backup.tar.gz *.c +.Dl $ tar cvzf backup.tar.gz *.c .Pp Verbosely list, but do not extract, all files ending in .Pa .jpeg @@ -358,6 +358,13 @@ from a compressed archive named Note that the glob pattern has been quoted to avoid expansion by the shell: .Pp .Dl $ tar tvzf backup.tar.gz '*.jpeg' +.Pp +Verbosely extract an archive, called +.Pa foo.tar.gz , +to the directory +.Pa /var/foo : +.Pp +.Dl $ tar xvzf foo.tar.gz -C /var/foo .Pp For more detailed examples, see .Xr pax 1 .
Re: add extract example to tar(1) man page
On 23/08/03 09:45AM, Stuart Henderson wrote: > On 2023/08/03 07:23, Jason McIntyre wrote: > > On Wed, Aug 02, 2023 at 05:52:02PM -0400, aisha wrote: > > > Hi, > > > Someone - https://www.youtube.com/watch?v=NQ5uD5x8vzg - mentioned that > > > our man page for tar(1) doesn't have an extract example, so I thought it > > > would be good to add a simple one which highlights a common use case. > > > > > > OK? > > > > > > > hi. > > > > the examples section is small enough that i suppose it wouldn;t be a > > problem to add another one. > > as it must be specified in a non-obvious place on the command line and > it's not currently explained particularly clearly, I'd welcome an example > using -C. > > > it does add another example with a similar set of options though, all in > > a different order. i wonder whether we should try and push the action as > > the first option, so people can see what we're doing. so "cXXX" when the > > example is to create, "tXXX" when listing? then keep the "vzf" options > > that are so common in the same order? > > > > then the second example is probably more helpful as "Create a gzip(1) > > compressed archive blah.tgz". > > > > i know this isn;t what you're posting about, so feel free to leave alone > > if you don;t want to tackle that. > > > > one more thing. you could as easily remove the text "the folder", but > > i'd be tempted to use "directory", as that's more standard for our docs, > > and how -C itself describes it. > > strongly agree on options order and "directory". Thanks for the comments! Here's the updated patch. OK? Index: tar.1 === RCS file: /cvs/src/bin/pax/tar.1,v retrieving revision 1.64 diff -u -p -r1.64 tar.1 --- tar.1 31 Mar 2022 17:27:14 - 1.64 +++ tar.1 3 Aug 2023 14:46:23 - @@ -331,7 +331,7 @@ and .Pp .Dl $ tar c bonvole sekve .Pp -Output a +Create a .Xr gzip 1 compressed archive containing the files .Pa bonvole @@ -340,7 +340,7 @@ and to a file called .Pa foriru.tar.gz : .Pp -.Dl $ tar zcf foriru.tar.gz bonvole sekve +.Dl $ tar czf foriru.tar.gz bonvole sekve .Pp Verbosely create an archive, called .Pa backup.tar.gz , @@ -349,7 +349,7 @@ of all files matching the shell function .Pa *.c : .Pp -.Dl $ tar zcvf backup.tar.gz *.c +.Dl $ tar cvzf backup.tar.gz *.c .Pp Verbosely list, but do not extract, all files ending in .Pa .jpeg @@ -358,6 +358,13 @@ from a compressed archive named Note that the glob pattern has been quoted to avoid expansion by the shell: .Pp .Dl $ tar tvzf backup.tar.gz '*.jpeg' +.Pp +Verbosely extract an archive, called +.Pa foo-backup.tar.gz , +to the directory +.Pa /var/foo : +.Pp +.Dl $ tar xvzf foo-backup.tar.gz -C /var/foo .Pp For more detailed examples, see .Xr pax 1 .
add extract example to tar(1) man page
Hi, Someone - https://www.youtube.com/watch?v=NQ5uD5x8vzg - mentioned that our man page for tar(1) doesn't have an extract example, so I thought it would be good to add a simple one which highlights a common use case. OK? Index: tar.1 === RCS file: /cvs/src/bin/pax/tar.1,v retrieving revision 1.64 diff -u -p -r1.64 tar.1 --- tar.1 31 Mar 2022 17:27:14 - 1.64 +++ tar.1 2 Aug 2023 21:47:12 - @@ -359,6 +359,13 @@ Note that the glob pattern has been quot .Pp .Dl $ tar tvzf backup.tar.gz '*.jpeg' .Pp +Verbosely extract an archive, called +.Pa foo-backup.tar.gz , +to the folder +.Pa /var/foo : +.Pp +.Dl $ tar xzvf foo-backup.tar.gz -C /var/foo +.Pp For more detailed examples, see .Xr pax 1 . .Sh DIAGNOSTICS
fix markup in glob(7)
Hi, Was teaching glob(7) syntax to my brother and noticed that the markup for [...] character classes was broken in HTML format from mandoc. Attached patch renders the correct HTML. Sidenote, mdoc(7) says 'Li' is deprecated but none of the suggested replacements were working in this context. OK? Cheers, Aisha diff --git a/share/man/man7/glob.7 b/share/man/man7/glob.7 index 037cb52e438..443ef3c7b6f 100644 --- a/share/man/man7/glob.7 +++ b/share/man/man7/glob.7 @@ -85,9 +85,9 @@ and stands for the list of all characters belonging to that class. Supported character classes: .Bl -column "xdigit" "xdigit" "xdigit" -offset indent -.It Li "alnum" Ta "cntrl" Ta "lower" Ta "space" -.It Li "alpha" Ta "digit" Ta "print" Ta "upper" -.It Li "blank" Ta "graph" Ta "punct" Ta "xdigit" +.It Li "alnum" Ta Li "cntrl" Ta Li "lower" Ta Li "space" +.It Li "alpha" Ta Li "digit" Ta Li "print" Ta Li "upper" +.It Li "blank" Ta Li "graph" Ta Li "punct" Ta Li "xdigit" .El .Pp These match characters using the macros specified in
add table-procexec to smtpd
Hi, This is another try to add table-procexec to smtpd. This allows for table backends to communicate with smtpd with a very simple line protocol, similar to filter proc-exec. The code is simple enough and after a bit of time can be used as a replace for table-proc (which uses imsg). Currently it is not replacing anything and is just available as an extra. I have a WIP perl-ldap table which can talk this line protocol and its on github right now (quite old) - https://github.com/bsd-ac/table-ldap_perl OK to import? Cheers, Aisha diff --git a/usr.sbin/smtpd/smtpctl/Makefile b/usr.sbin/smtpd/smtpctl/Makefile index ef8148be8c9..2e8beff1ad1 100644 --- a/usr.sbin/smtpd/smtpctl/Makefile +++ b/usr.sbin/smtpd/smtpctl/Makefile @@ -48,6 +48,7 @@ SRCS+=table_static.c SRCS+= table_db.c SRCS+= table_getpwnam.c SRCS+= table_proc.c +SRCS+= table_procexec.c SRCS+= unpack_dns.c SRCS+= spfwalk.c diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h index 125a6a5dfbe..ca54d54ea66 100644 --- a/usr.sbin/smtpd/smtpd.h +++ b/usr.sbin/smtpd/smtpd.h @@ -1662,6 +1662,7 @@ int table_regex_match(const char *, const char *); void table_open_all(struct smtpd *); void table_dump_all(struct smtpd *); void table_close_all(struct smtpd *); +const char *table_service_name(enum table_service ); /* to.c */ diff --git a/usr.sbin/smtpd/smtpd/Makefile b/usr.sbin/smtpd/smtpd/Makefile index d914b43f705..3fcfcd1c19d 100644 --- a/usr.sbin/smtpd/smtpd/Makefile +++ b/usr.sbin/smtpd/smtpd/Makefile @@ -63,6 +63,7 @@ SRCS+=compress_gzip.c SRCS+= table_db.c SRCS+= table_getpwnam.c SRCS+= table_proc.c +SRCS+= table_procexec.c SRCS+= table_static.c SRCS+= queue_fs.c diff --git a/usr.sbin/smtpd/table.c b/usr.sbin/smtpd/table.c index 7328cf5df6e..4f9adfe4c57 100644 --- a/usr.sbin/smtpd/table.c +++ b/usr.sbin/smtpd/table.c @@ -36,8 +36,8 @@ extern struct table_backend table_backend_static; extern struct table_backend table_backend_db; extern struct table_backend table_backend_getpwnam; extern struct table_backend table_backend_proc; +extern struct table_backend table_backend_procexec; -static const char * table_service_name(enum table_service); static int table_parse_lookup(enum table_service, const char *, const char *, union lookup *); static int parse_sockaddr(struct sockaddr *, int, const char *); @@ -49,6 +49,7 @@ static struct table_backend *backends[] = { _backend_db, _backend_getpwnam, _backend_proc, + _backend_procexec, NULL }; @@ -67,7 +68,7 @@ table_backend_lookup(const char *backend) return NULL; } -static const char * +const char * table_service_name(enum table_service s) { switch (s) { diff --git a/usr.sbin/smtpd/table_procexec.c b/usr.sbin/smtpd/table_procexec.c new file mode 100644 index 000..9375da5c0ad --- /dev/null +++ b/usr.sbin/smtpd/table_procexec.c @@ -0,0 +1,326 @@ +/* $OpenBSD$ */ + +/* + * Copyright (c) 2023 Aisha Tammy + * Copyright (c) 2020 Gilles Chehade + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "log.h" +#include "smtpd.h" + +#define PROCEXEC_VERSION "1" +#define PROCEXEC_TIMEOUT 500 + +static int table_procexec_open(struct table *); +static int table_procexec_update(struct table *); +static void table_procexec_close(struct table *); +static int table_procexec_lookup(struct table *, enum table_service, + const char *, char **); +static int table_procexec_fetch(struct table *, enum table_service, char **); + +enum procexec_query; +static int table_procexec_helper(struct table *, enum procexec_query, + enum table_service, const char *, char **); + +struct table_backend table_backend_procexec = { +"proc-exec", +K_ANY, +NULL, +NULL, +NULL, +table_procexec_open, +table_procexec_update, +table_procexec_close, +table_procexec_lookup, +table_procexec_f
remove reference to auth_getchallenge
The auth_getchallenge function doesn't seem to exist in the code base. OK to remove this reference? diff --git a/lib/libc/gen/auth_subr.3 b/lib/libc/gen/auth_subr.3 index c7bd965f944..24ea850eb9e 100644 --- a/lib/libc/gen/auth_subr.3 +++ b/lib/libc/gen/auth_subr.3 @@ -328,10 +328,6 @@ function. The generated challenge is returned. .Dv NULL is returned on error or if no challenge was generated. -The challenge can also be extracted by the -.Fn auth_getchallenge -function, which simply returns the last challenge generated -for this session. .Pp The .Fn auth_check_change
Re: table-procexec for opensmtpd (another try)
A very small change with fixing an off by one copy to the return value. It was also copying the '\n' character by mistake. diff --git a/usr.sbin/smtpd/parse.y b/usr.sbin/smtpd/parse.y index 832f4f2aec9..ff7b9a9a340 100644 --- a/usr.sbin/smtpd/parse.y +++ b/usr.sbin/smtpd/parse.y @@ -2543,13 +2543,6 @@ table: TABLE STRING STRING { config = p+1; } } - if (config != NULL && *config != '/') { - yyerror("invalid backend parameter for table: %s", - $2); - free($2); - free($3); - YYERROR; - } table = table_create(conf, backend, $2, config); if (!table_config(table)) { yyerror("invalid configuration file %s for table %s", diff --git a/usr.sbin/smtpd/smtpctl/Makefile b/usr.sbin/smtpd/smtpctl/Makefile index ef8148be8c9..46831d647dc 100644 --- a/usr.sbin/smtpd/smtpctl/Makefile +++ b/usr.sbin/smtpd/smtpctl/Makefile @@ -47,7 +47,7 @@ SRCS+=table.c SRCS+= table_static.c SRCS+= table_db.c SRCS+= table_getpwnam.c -SRCS+= table_proc.c +SRCS+= table_procexec.c SRCS+= unpack_dns.c SRCS+= spfwalk.c diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h index e6fc114d0a6..8ef80add4e7 100644 --- a/usr.sbin/smtpd/smtpd.h +++ b/usr.sbin/smtpd/smtpd.h @@ -1663,6 +1663,7 @@ int table_regex_match(const char *, const char *); void table_open_all(struct smtpd *); void table_dump_all(struct smtpd *); void table_close_all(struct smtpd *); +const char *table_service_name(enum table_service ); /* to.c */ diff --git a/usr.sbin/smtpd/smtpd/Makefile b/usr.sbin/smtpd/smtpd/Makefile index b31d4e42224..64e73c3bb70 100644 --- a/usr.sbin/smtpd/smtpd/Makefile +++ b/usr.sbin/smtpd/smtpd/Makefile @@ -62,7 +62,7 @@ SRCS+=compress_gzip.c SRCS+= table_db.c SRCS+= table_getpwnam.c -SRCS+= table_proc.c +SRCS+= table_procexec.c SRCS+= table_static.c SRCS+= queue_fs.c diff --git a/usr.sbin/smtpd/table.c b/usr.sbin/smtpd/table.c index 7328cf5df6e..81102ef90e1 100644 --- a/usr.sbin/smtpd/table.c +++ b/usr.sbin/smtpd/table.c @@ -35,9 +35,8 @@ struct table_backend *table_backend_lookup(const char *); extern struct table_backend table_backend_static; extern struct table_backend table_backend_db; extern struct table_backend table_backend_getpwnam; -extern struct table_backend table_backend_proc; +extern struct table_backend table_backend_procexec; -static const char * table_service_name(enum table_service); static int table_parse_lookup(enum table_service, const char *, const char *, union lookup *); static int parse_sockaddr(struct sockaddr *, int, const char *); @@ -48,7 +47,7 @@ static struct table_backend *backends[] = { _backend_static, _backend_db, _backend_getpwnam, - _backend_proc, + _backend_procexec, NULL }; @@ -67,7 +66,7 @@ table_backend_lookup(const char *backend) return NULL; } -static const char * +const char * table_service_name(enum table_service s) { switch (s) { @@ -198,10 +197,9 @@ table_create(struct smtpd *conf, const char *backend, const char *name, PATH_LIBEXEC"/table-%s\"", backend); } if (stat(path, ) == 0) { - tb = table_backend_lookup("proc"); - (void)strlcpy(path, backend, sizeof(path)); + tb = table_backend_lookup("proc-exec"); if (config) { - (void)strlcat(path, ":", sizeof(path)); + (void)strlcat(path, " ", sizeof(path)); if (strlcat(path, config, sizeof(path)) >= sizeof(path)) fatalx("table_create: config file path too long"); diff --git a/usr.sbin/smtpd/table_proc.c b/usr.sbin/smtpd/table_proc.c deleted file mode 100644 index 56893a0fb61..000 --- a/usr.sbin/smtpd/table_proc.c +++ /dev/null @@ -1,265 +0,0 @@ -/* $OpenBSD: table_proc.c,v 1.17 2021/06/14 17:58:16 eric Exp $*/ - -/* - * Copyright (c) 2013 Eric Faurot - * - * Permission to use, copy, modify, and distribute this software for any - * purpose with or without fee is hereby granted, provided that the above - * copyright notice and this permission notice appear in all copies. - * - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES -
table-procexec for opensmtpd (another try)
Hi all, I've made a refactored version of table-procexec, hopefully with a lot less redundancy in code. This patch adds the table-procexec backend which is configured with a timeout of 500 milliseconds. Currently this is hardcoded, but that is easy enough to change and shouldnt be the holdback. In case a table times out and the response has not reached smtpd, this sets the table status to indicate that and also starts an event to discard the next line coming on the socket. After which we are "clear" for communication. Comments would be very welcome and testing even more so. I am not the most proficient C coder... Cheers, Aisha diff --git a/usr.sbin/smtpd/parse.y b/usr.sbin/smtpd/parse.y index 832f4f2aec9..ff7b9a9a340 100644 --- a/usr.sbin/smtpd/parse.y +++ b/usr.sbin/smtpd/parse.y @@ -2543,13 +2543,6 @@ table: TABLE STRING STRING { config = p+1; } } - if (config != NULL && *config != '/') { - yyerror("invalid backend parameter for table: %s", - $2); - free($2); - free($3); - YYERROR; - } table = table_create(conf, backend, $2, config); if (!table_config(table)) { yyerror("invalid configuration file %s for table %s", diff --git a/usr.sbin/smtpd/smtpctl/Makefile b/usr.sbin/smtpd/smtpctl/Makefile index ef8148be8c9..46831d647dc 100644 --- a/usr.sbin/smtpd/smtpctl/Makefile +++ b/usr.sbin/smtpd/smtpctl/Makefile @@ -47,7 +47,7 @@ SRCS+=table.c SRCS+= table_static.c SRCS+= table_db.c SRCS+= table_getpwnam.c -SRCS+= table_proc.c +SRCS+= table_procexec.c SRCS+= unpack_dns.c SRCS+= spfwalk.c diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h index e6fc114d0a6..8ef80add4e7 100644 --- a/usr.sbin/smtpd/smtpd.h +++ b/usr.sbin/smtpd/smtpd.h @@ -1663,6 +1663,7 @@ int table_regex_match(const char *, const char *); void table_open_all(struct smtpd *); void table_dump_all(struct smtpd *); void table_close_all(struct smtpd *); +const char *table_service_name(enum table_service ); /* to.c */ diff --git a/usr.sbin/smtpd/smtpd/Makefile b/usr.sbin/smtpd/smtpd/Makefile index b31d4e42224..64e73c3bb70 100644 --- a/usr.sbin/smtpd/smtpd/Makefile +++ b/usr.sbin/smtpd/smtpd/Makefile @@ -62,7 +62,7 @@ SRCS+=compress_gzip.c SRCS+= table_db.c SRCS+= table_getpwnam.c -SRCS+= table_proc.c +SRCS+= table_procexec.c SRCS+= table_static.c SRCS+= queue_fs.c diff --git a/usr.sbin/smtpd/table.c b/usr.sbin/smtpd/table.c index 7328cf5df6e..81102ef90e1 100644 --- a/usr.sbin/smtpd/table.c +++ b/usr.sbin/smtpd/table.c @@ -35,9 +35,8 @@ struct table_backend *table_backend_lookup(const char *); extern struct table_backend table_backend_static; extern struct table_backend table_backend_db; extern struct table_backend table_backend_getpwnam; -extern struct table_backend table_backend_proc; +extern struct table_backend table_backend_procexec; -static const char * table_service_name(enum table_service); static int table_parse_lookup(enum table_service, const char *, const char *, union lookup *); static int parse_sockaddr(struct sockaddr *, int, const char *); @@ -48,7 +47,7 @@ static struct table_backend *backends[] = { _backend_static, _backend_db, _backend_getpwnam, - _backend_proc, + _backend_procexec, NULL }; @@ -67,7 +66,7 @@ table_backend_lookup(const char *backend) return NULL; } -static const char * +const char * table_service_name(enum table_service s) { switch (s) { @@ -198,10 +197,9 @@ table_create(struct smtpd *conf, const char *backend, const char *name, PATH_LIBEXEC"/table-%s\"", backend); } if (stat(path, ) == 0) { - tb = table_backend_lookup("proc"); - (void)strlcpy(path, backend, sizeof(path)); + tb = table_backend_lookup("proc-exec"); if (config) { - (void)strlcat(path, ":", sizeof(path)); + (void)strlcat(path, " ", sizeof(path)); if (strlcat(path, config, sizeof(path)) >= sizeof(path)) fatalx("table_create: config file path too long"); diff --git a/usr.sbin/smtpd/table_proc.c b/usr.sbin/smtpd/table_proc.c deleted file mode 100644 index 56893a0fb61..000 --- a/usr.sbin/smtpd/table_proc.c +++ /dev/null @@ -1,265 +0,0 @@ -/* $
Re: smtpd: move authentication to table backends
On 21/10/08 05:34PM, aisha wrote: > Hi all, > I am still working on the table-procexec for opensmtpd > and while there, I was thinking of how to do authentication > using LDAP, which the current table-ldap from ports does not > support. > The primary reason for that, I believe, is that LDAP > authentication should be done by bind and not by returning > the userPassword and us doing the authentication with > crypt_checkpass. That kind of defeats one of the uses of LDAP. > > Here I've added a patch which pushes the authentication step > to the table backend and it only returns the final AUTH/NOAUTH > kind of values. > > While here, I also made another small change with mailaddrmap, > where instead of returning ALL possible aliases that a user > may use, we now pass the current mailaddr to the table, so > it can now return a smaller set of addresses. > > It should not affect any workflow, so testing from others > would be appreciated. > > Cheers, > Aisha > Same patch but change my horrible enums representation to bitshifts diff --git a/usr.sbin/smtpd/aliases.c b/usr.sbin/smtpd/aliases.c index a473aeca189..8e3835f78a6 100644 --- a/usr.sbin/smtpd/aliases.c +++ b/usr.sbin/smtpd/aliases.c @@ -45,7 +45,7 @@ aliases_get(struct expand *expand, const char *username) /* first, check if entry has a user-part tag */ pbuf = strchr(buf, *env->sc_subaddressing_delim); if (pbuf) { - ret = table_lookup(mapping, K_ALIAS, buf, ); + ret = table_lookup(mapping, K_ALIAS, buf, NULL, ); if (ret < 0) return (-1); if (ret) @@ -54,7 +54,7 @@ aliases_get(struct expand *expand, const char *username) } /* no user-part tag, try looking up user */ - ret = table_lookup(mapping, K_ALIAS, buf, ); + ret = table_lookup(mapping, K_ALIAS, buf, NULL, ); if (ret <= 0) return ret; @@ -116,7 +116,7 @@ aliases_virtual_get(struct expand *expand, const struct mailaddr *maddr) if (!bsnprintf(buf, sizeof(buf), "%s%c%s@%s", user, *env->sc_subaddressing_delim, tag, domain)) return 0; - ret = table_lookup(mapping, K_ALIAS, buf, ); + ret = table_lookup(mapping, K_ALIAS, buf, NULL, ); if (ret < 0) return (-1); if (ret) @@ -126,7 +126,7 @@ aliases_virtual_get(struct expand *expand, const struct mailaddr *maddr) /* then, check if entry exists without user-part tag */ if (!bsnprintf(buf, sizeof(buf), "%s@%s", user, domain)) return 0; - ret = table_lookup(mapping, K_ALIAS, buf, ); + ret = table_lookup(mapping, K_ALIAS, buf, NULL, ); if (ret < 0) return (-1); if (ret) @@ -137,7 +137,7 @@ aliases_virtual_get(struct expand *expand, const struct mailaddr *maddr) if (!bsnprintf(buf, sizeof(buf), "%s%c%s", user, *env->sc_subaddressing_delim, tag)) return 0; - ret = table_lookup(mapping, K_ALIAS, buf, ); + ret = table_lookup(mapping, K_ALIAS, buf, NULL, ); if (ret < 0) return (-1); if (ret) @@ -147,7 +147,7 @@ aliases_virtual_get(struct expand *expand, const struct mailaddr *maddr) /* Failed ? We lookup for username only */ if (!bsnprintf(buf, sizeof(buf), "%s", user)) return 0; - ret = table_lookup(mapping, K_ALIAS, buf, ); + ret = table_lookup(mapping, K_ALIAS, buf, NULL, ); if (ret < 0) return (-1); if (ret) @@ -160,14 +160,14 @@ aliases_virtual_get(struct expand *expand, const struct mailaddr *maddr) if (!bsnprintf(buf, sizeof(buf), "@%s", domain)) return 0; /* Failed ? We lookup for catch all for virtual domain */ - ret = table_lookup(mapping, K_ALIAS, buf, ); + ret = table_lookup(mapping, K_ALIAS, buf, NULL, ); if (ret < 0) return (-1); if (ret) goto expand; /* Failed ? We lookup for a *global* catch all */ - ret = table_lookup(mapping, K_ALIAS, "@", ); + ret = table_lookup(mapping, K_ALIAS, "@", NULL, ); if (ret <= 0) return (ret); diff --git a/usr.sbin/smtpd/lka.c b/usr.sbin/smtpd/lka.c index 764130d6078..3354ccde7d7 100644 --- a/usr.sbin/smtpd/lka.c +++ b/usr.sbin/smtpd/lka.c @@ -268,7 +268,7 @@ lka_imsg(struct mproc *p, struct imsg *imsg) if (domain == NULL) ret = table_fetch(table, K_RELAYHOST, ); else - r
smtpd: move authentication to table backends
Hi all, I am still working on the table-procexec for opensmtpd and while there, I was thinking of how to do authentication using LDAP, which the current table-ldap from ports does not support. The primary reason for that, I believe, is that LDAP authentication should be done by bind and not by returning the userPassword and us doing the authentication with crypt_checkpass. That kind of defeats one of the uses of LDAP. Here I've added a patch which pushes the authentication step to the table backend and it only returns the final AUTH/NOAUTH kind of values. While here, I also made another small change with mailaddrmap, where instead of returning ALL possible aliases that a user may use, we now pass the current mailaddr to the table, so it can now return a smaller set of addresses. It should not affect any workflow, so testing from others would be appreciated. Cheers, Aisha diff --git a/usr.sbin/smtpd/aliases.c b/usr.sbin/smtpd/aliases.c index a473aeca189..8e3835f78a6 100644 --- a/usr.sbin/smtpd/aliases.c +++ b/usr.sbin/smtpd/aliases.c @@ -45,7 +45,7 @@ aliases_get(struct expand *expand, const char *username) /* first, check if entry has a user-part tag */ pbuf = strchr(buf, *env->sc_subaddressing_delim); if (pbuf) { - ret = table_lookup(mapping, K_ALIAS, buf, ); + ret = table_lookup(mapping, K_ALIAS, buf, NULL, ); if (ret < 0) return (-1); if (ret) @@ -54,7 +54,7 @@ aliases_get(struct expand *expand, const char *username) } /* no user-part tag, try looking up user */ - ret = table_lookup(mapping, K_ALIAS, buf, ); + ret = table_lookup(mapping, K_ALIAS, buf, NULL, ); if (ret <= 0) return ret; @@ -116,7 +116,7 @@ aliases_virtual_get(struct expand *expand, const struct mailaddr *maddr) if (!bsnprintf(buf, sizeof(buf), "%s%c%s@%s", user, *env->sc_subaddressing_delim, tag, domain)) return 0; - ret = table_lookup(mapping, K_ALIAS, buf, ); + ret = table_lookup(mapping, K_ALIAS, buf, NULL, ); if (ret < 0) return (-1); if (ret) @@ -126,7 +126,7 @@ aliases_virtual_get(struct expand *expand, const struct mailaddr *maddr) /* then, check if entry exists without user-part tag */ if (!bsnprintf(buf, sizeof(buf), "%s@%s", user, domain)) return 0; - ret = table_lookup(mapping, K_ALIAS, buf, ); + ret = table_lookup(mapping, K_ALIAS, buf, NULL, ); if (ret < 0) return (-1); if (ret) @@ -137,7 +137,7 @@ aliases_virtual_get(struct expand *expand, const struct mailaddr *maddr) if (!bsnprintf(buf, sizeof(buf), "%s%c%s", user, *env->sc_subaddressing_delim, tag)) return 0; - ret = table_lookup(mapping, K_ALIAS, buf, ); + ret = table_lookup(mapping, K_ALIAS, buf, NULL, ); if (ret < 0) return (-1); if (ret) @@ -147,7 +147,7 @@ aliases_virtual_get(struct expand *expand, const struct mailaddr *maddr) /* Failed ? We lookup for username only */ if (!bsnprintf(buf, sizeof(buf), "%s", user)) return 0; - ret = table_lookup(mapping, K_ALIAS, buf, ); + ret = table_lookup(mapping, K_ALIAS, buf, NULL, ); if (ret < 0) return (-1); if (ret) @@ -160,14 +160,14 @@ aliases_virtual_get(struct expand *expand, const struct mailaddr *maddr) if (!bsnprintf(buf, sizeof(buf), "@%s", domain)) return 0; /* Failed ? We lookup for catch all for virtual domain */ - ret = table_lookup(mapping, K_ALIAS, buf, ); + ret = table_lookup(mapping, K_ALIAS, buf, NULL, ); if (ret < 0) return (-1); if (ret) goto expand; /* Failed ? We lookup for a *global* catch all */ - ret = table_lookup(mapping, K_ALIAS, "@", ); + ret = table_lookup(mapping, K_ALIAS, "@", NULL, ); if (ret <= 0) return (ret); diff --git a/usr.sbin/smtpd/lka.c b/usr.sbin/smtpd/lka.c index 764130d6078..3354ccde7d7 100644 --- a/usr.sbin/smtpd/lka.c +++ b/usr.sbin/smtpd/lka.c @@ -268,7 +268,7 @@ lka_imsg(struct mproc *p, struct imsg *imsg) if (domain == NULL) ret = table_fetch(table, K_RELAYHOST, ); else - ret = table_lookup(table, K_RELAYHOST, domain, ); + ret = table_lookup(table, K_RELAYHOST, domain, NULL, ); if (ret == -1) m_add_int(p, LKA_TEMPFAI
Re: add table_procexec in smtpd
> >> First, if the two implementations are not going to coexists, > >> we can just replace table_proc.c. > > > > True, though proc-exec was the name used for filters so it may be a good to > > unify and drop the legacy “proc” name. > > This will be hidden to users so quite frankly it’s a matter of dev > > preference. I've kept procexec for now. > > > >> Second, since the goal is to expose the protocol directly, instead of > >> relying on wrappers (through opensmtpd-extras api), we should take > >> time to refine it properly before, and avoid having to bump it every > >> other day. For example, I don't see the point of adding timestamps in > >> the request. Do we want to be case-sensitive on the commands? Do we > >> need some kind of handshake? Also, there can be long-term plan to use > >> the same process for different tables, or even for other backends. > > > > I’m unsure I understand your point: > > > > The protocol is based on the filter protocol, follows the same logic and > > line header to solve the same issues, this is precisely so that there’s no > > need to bump every other day as we already figured what was needed for > > third party adding to interoperate with smtpd. > > This also has the advantage that you can have a single parser handle these > > different API instead of having a filter protocol parser, a table protocol > > parser (and maybe in the future a queue protocol parser… etc). > > > > WRT timestamps there were many uses for them ranging from timeout detection > > both in daemon / add-ons, profiling, logging the time an event was > > generated vs processes overhead, etc… > > It allowed ensuring that all addons handling the same event have the exact > > same timestamp associated to the event. > > > > WRT to case-sensitivity, I don’t recall using upper-case myself, to me the > > protocol is case-sensitive and as far as I can remember I always used > > lowercase :-/ > > I’m all for lowering case here. > > > > WRT to handshake, it has multiple uses ranging from ensuring the addons get > > their configuration from the daemon to synchronising the daemon start with > > addons readiness. > > Remember, we didn’t have this with filters and realised we couldn’t do > > without, it is the same for tables, they need to get their “table name” at > > start so we need the daemon to pass them, and we also need the daemon to > > not start using them before they are ready. > > I am unsure what you mean by a handshake. Would something like the following be good - on exec the table process has to print out a string "TABLE-READY" which ensures us that the process is ready. I am not exactly sure how I would implement this, my guess would be to use event(3) with EV_READ on backend_r (or something like that). > > > >> Finally the implementation could be factorized a bit, but that's a > >> detail at this time. I think the close operation (is it really useful > >> anyway?) > >> should use fclose() instead of kill(), and maybe wait() too? I've moved it to fclose(), I've not used wait() for now. > > > > The implementation can probably be improved, this was a PoC that allowed me > > to write various table backends but it was never meant to be committed in > > the first place. > If you can clarify what the handshake means, that would be nice. Cheers, Aisha diff --git a/usr.sbin/smtpd/parse.y b/usr.sbin/smtpd/parse.y index 011e306ac61..1b0ee5ad38f 100644 --- a/usr.sbin/smtpd/parse.y +++ b/usr.sbin/smtpd/parse.y @@ -2557,13 +2557,6 @@ table: TABLE STRING STRING { config = p+1; } } - if (config != NULL && *config != '/') { - yyerror("invalid backend parameter for table: %s", - $2); - free($2); - free($3); - YYERROR; - } table = table_create(conf, backend, $2, config); if (!table_config(table)) { yyerror("invalid configuration file %s for table %s", diff --git a/usr.sbin/smtpd/smtpctl/Makefile b/usr.sbin/smtpd/smtpctl/Makefile index ef8148be8c9..46831d647dc 100644 --- a/usr.sbin/smtpd/smtpctl/Makefile +++ b/usr.sbin/smtpd/smtpctl/Makefile @@ -47,7 +47,7 @@ SRCS+=table.c SRCS+= table_static.c SRCS+= table_db.c SRCS+= table_getpwnam.c -SRCS+= table_proc.c +SRCS+= table_procexec.c
Re: add table_procexec in smtpd
On 6/12/21 9:15 AM, Eric Faurot wrote: On Wed, Jun 09, 2021 at 05:41:36PM -0400, Aisha Tammy wrote: Hi, Here is the updated diff, which removes table_proc and adds table_procexec as the default backend when no backend name matches. Hi. I'm not opposed to the idea, but I have a couple of comments: First, if the two implementations are not going to coexists, we can just replace table_proc.c. Sounds good with me :D Second, since the goal is to expose the protocol directly, instead of relying on wrappers (through opensmtpd-extras api), we should take time to refine it properly before, and avoid having to bump it every other day. For example, I don't see the point of adding timestamps in the request. My WIP LDAP wrapper does not use the timestamps. I have not been privy to the historical context of the development of this protocol, so I do not know why it exists. I am fine with removing it. Do we want to be case-sensitive on the commands? I reused the current table_service_name function present in table.c and hence set it to all-caps. I think keeping it such is not an issue but I don't care if we move to lowercase. I would prefer being case sensitive, so as not to overly-complicate our checks. Do we need some kind of handshake? I do not see a need for this between the table-open and the table-process. The table-process may do handshakes (or whatever) in its backend, which we should not be worrying about. Also, there can be long-term plan to use the same process for different tables, or even for other backends. Finally the implementation could be factorized a bit, but that's a detail at this time. I think the close operation (is it really useful anyway?) should use fclose() instead of kill(), and maybe wait() too? I agree, I am not sure if the table-close is very useful. I am also fine with moving it to fclose, though I don't know how to manually close a table >.< BTW, can I remove the table_check function? I haven't seen a use for it even after scrounging around a bit. Unless any comments, I'll send the next patch (soonish) with fclose and timestamps, table_check removed. Cheers, Aisha Eric.
Re: add table_procexec in smtpd
Hi, Here is the updated diff, which removes table_proc and adds table_procexec as the default backend when no backend name matches. With this diff, I have the following configuration for smtpd: # $OpenBSD: smtpd.conf,v 1.14 2019/11/26 20:14:38 gilles Exp $ # This is the smtpd server system-wide configuration file. # See smtpd.conf(5) for more information. table aliases aliases:root.t...@bsd.ac listen on socket # To accept external mail, replace with: listen on all # listen on lo0 action "local_mail" mbox alias action "outbound" relay action "bsd.ac" relay host smtp://10.7.0.1 # Uncomment the following to accept external mail for domain "example.org" # # match from any for domain "example.org" action "local_mail" match from local for local action "local_mail" match from local for domain "bsd.ac" action "bsd.ac" match from local for any action "outbound" where my /usr/local/libexec/smtpd/table-aliases contains: #!/bin/ksh user="${1:-r...@bsd.ac}" while read line do reqid="$(echo $line | awk -F'|' '{ print $5; }')" reply="TABLE-RESULT|$reqid|FOUND|$user" echo $reply done < /dev/stdin exit 0 This should hopefully satisfy the requirements for transparency and sanity. I will work on the opensmtpd-extras and make a PR in the github separately, if that sounds fine. Cheers, Aisha diff --git a/usr.sbin/smtpd/parse.y b/usr.sbin/smtpd/parse.y index 011e306ac61..1b0ee5ad38f 100644 --- a/usr.sbin/smtpd/parse.y +++ b/usr.sbin/smtpd/parse.y @@ -2557,13 +2557,6 @@ table: TABLE STRING STRING { config = p+1; } } - if (config != NULL && *config != '/') { - yyerror("invalid backend parameter for table: %s", - $2); - free($2); - free($3); - YYERROR; - } table = table_create(conf, backend, $2, config); if (!table_config(table)) { yyerror("invalid configuration file %s for table %s", diff --git a/usr.sbin/smtpd/smtpctl/Makefile b/usr.sbin/smtpd/smtpctl/Makefile index ef8148be8c9..46831d647dc 100644 --- a/usr.sbin/smtpd/smtpctl/Makefile +++ b/usr.sbin/smtpd/smtpctl/Makefile @@ -47,7 +47,7 @@ SRCS+=table.c SRCS+= table_static.c SRCS+= table_db.c SRCS+= table_getpwnam.c -SRCS+= table_proc.c +SRCS+= table_procexec.c SRCS+= unpack_dns.c SRCS+= spfwalk.c diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h index be934112103..221f24fbdc4 100644 --- a/usr.sbin/smtpd/smtpd.h +++ b/usr.sbin/smtpd/smtpd.h @@ -1656,6 +1656,7 @@ int table_regex_match(const char *, const char *); void table_open_all(struct smtpd *); void table_dump_all(struct smtpd *); void table_close_all(struct smtpd *); +const char *table_service_name(enum table_service ); /* to.c */ diff --git a/usr.sbin/smtpd/smtpd/Makefile b/usr.sbin/smtpd/smtpd/Makefile index b31d4e42224..64e73c3bb70 100644 --- a/usr.sbin/smtpd/smtpd/Makefile +++ b/usr.sbin/smtpd/smtpd/Makefile @@ -62,7 +62,7 @@ SRCS+=compress_gzip.c SRCS+= table_db.c SRCS+= table_getpwnam.c -SRCS+= table_proc.c +SRCS+= table_procexec.c SRCS+= table_static.c SRCS+= queue_fs.c diff --git a/usr.sbin/smtpd/table.c b/usr.sbin/smtpd/table.c index 1d82d88b81a..a09229ca174 100644 --- a/usr.sbin/smtpd/table.c +++ b/usr.sbin/smtpd/table.c @@ -45,9 +45,8 @@ struct table_backend *table_backend_lookup(const char *); extern struct table_backend table_backend_static; extern struct table_backend table_backend_db; extern struct table_backend table_backend_getpwnam; -extern struct table_backend table_backend_proc; +extern struct table_backend table_backend_procexec; -static const char * table_service_name(enum table_service); static int table_parse_lookup(enum table_service, const char *, const char *, union lookup *); static int parse_sockaddr(struct sockaddr *, int, const char *); @@ -58,7 +57,7 @@ static struct table_backend *backends[] = { _backend_static, _backend_db, _backend_getpwnam, - _backend_proc, + _backend_procexec, NULL }; @@ -77,7 +76,7 @@ table_backend_lookup(const char *backend) return NULL; } -static const char * +const char * table_service_name(enum table_service s) { switch (s) { @@ -208,10 +207,9 @@ table_create(struct smtpd *conf, const char *backend, const char *name, PATH_LIBEXEC"/table-%s\"", backend); } if (stat(path, ) == 0) { - tb = table_backend_lookup
Re: add table_procexec in smtpd
On 6/9/21 10:34 AM, Gilles CHEHADE wrote: On 9 Jun 2021, at 15:47, Aisha Tammy wrote: On 6/9/21 5:19 AM, Gilles CHEHADE wrote: Hi, I wrote table_procexec (despite the copyright which I copy-pasted and forgot to replace author) so just providing a bit of insight: Ah, I did not know that. I will fix the header in the next patch No problem, here’s a write-up for reference: https://poolp.org/posts/2020-05-28/may-2020-opensmtpd-6.7.1p1-release-table-procexec-and-many-pocs/ Yes, this was very helpful and was also where I started from :D table_procexec was written as a proof of concept for a new table protocol inspired by the filter protocol to make it easier to write privsep table backends using any language or library available without requiring dependencies in the daemon. I implemented it as a table backend because it was the easiest way to show a working implementation of the protocol without making changes in the daemon, the table_procexec backend sits in between the daemon and “new” tables to translate old protocol queries into new protocol queries and new protocol results into old protocol results, but the intent was to move the underlying implementation of the table API to the protocol and not retain this table proxy. Do you mean that ALL table backends should be replaced by the new model? Yes, it should be possible to do that but I don't know if that should be done simultaneously as introducing procexec. Builtin tables within the daemon do not need an update because they are built in the lookup process and do not rely on IPC. Table backends from opensmtpd-extras would all need an update, yes, but the change only affects the protocol and not the interface so the change doesn’t happen in the backends but in the api library that abstracts communication with the daemon, backends just need to be relinked to the new library, since the opensmtpd-extras are expected to match specific opensmtpd releases (no backwards compatibility) and they are rebuilt whenever a package is created, this isn’t as big and hurtful as it looks. I agree that maybe this should not be done simultaneously to introducing procexec but I still don’t think the way it is introduced here is the proper way: Ultimately you want to be able to do: table foobar mytable:/etc/smtpd/mytable.conf and not be aware that there’s a table-procexec or a procexec protocol ensuring communication with the daemon. The fact that you introduce “procexec” in a way that requires it to appear in the config will make this very hard to hide/remove/change in the future as people will start relying on it. Ah, this makes it a lot more clearer on what should be done. Now I can work towards this in a better fashion. Maybe the simplest way is to temporarily introduce procexec as a builtin table backend, like you did, but do the necessary work in parse.y and table.c to detect that a table backend in smtpd.conf is meant to be executed by table-procexec so the daemon can do it transparently. This might be hard to concurrently with having table_proc present, as it does exactly this - use table_proc backend if the backend name is not present in the table names. So what I could do is to remove table_proc and introduce table_procexec and simultaneously update the opensmtpd-extras port to use table_procexec API. Does that sound like a feasible approach? I'll send an updated diff soonish. Cheers, Aisha This way, if or when the table API relies on the procexec protocol instead of the old protocol, the table-procexec layer can be removed and people will not see a difference. Not my call but I don’t think what you’re proposing is the proper way to integrate it. There are a few downsides in plugging it this way but the most annoying one is that instead of starting a table with a configuration parameter, this is tricking it into executing the table_procexec and using the configuration path as the path to the “new” table, which means that you have no way to provide the new table with a configuration for itself. I don't know if this is true. I was able to provide parameters to the procexec executable via Yes, it works (and is how I did my testing), but this is kind of abusing the mechanism it is built upon. In theory you should provide the backend + a configuration file path: table aliases aliases_procexec:/path/to/config not: table aliases "proc-exec:/usr/local/bin/aliases_procexec root.t...@bsd.ac" What you did works but is not how it is intended to work and will be at odds with other backends that expect a config file or a table mapping. I think your diff is very close to something nice but it lacks some adaptations in parse.y / table.c to hide the plumbing of table_procexec which should remain invisible to the user IMO. This is just my 2cts, maybe others have a different opinion on this of course Gilles
Re: add table_procexec in smtpd
On 6/9/21 5:19 AM, Gilles CHEHADE wrote: Hi, I wrote table_procexec (despite the copyright which I copy-pasted and forgot to replace author) so just providing a bit of insight: Ah, I did not know that. I will fix the header in the next patch table_procexec was written as a proof of concept for a new table protocol inspired by the filter protocol to make it easier to write privsep table backends using any language or library available without requiring dependencies in the daemon. I implemented it as a table backend because it was the easiest way to show a working implementation of the protocol without making changes in the daemon, the table_procexec backend sits in between the daemon and “new” tables to translate old protocol queries into new protocol queries and new protocol results into old protocol results, but the intent was to move the underlying implementation of the table API to the protocol and not retain this table proxy. Do you mean that ALL table backends should be replaced by the new model? Yes, it should be possible to do that but I don't know if that should be done simultaneously as introducing procexec. Not my call but I don’t think what you’re proposing is the proper way to integrate it. There are a few downsides in plugging it this way but the most annoying one is that instead of starting a table with a configuration parameter, this is tricking it into executing the table_procexec and using the configuration path as the path to the “new” table, which means that you have no way to provide the new table with a configuration for itself. I don't know if this is true. I was able to provide parameters to the procexec executable via $ cat /etc/mail/smtpd.conf # $OpenBSD: smtpd.conf,v 1.14 2019/11/26 20:14:38 gilles Exp $ # This is the smtpd server system-wide configuration file. # See smtpd.conf(5) for more information. table paliases file:/etc/mail/aliases table aliases "proc-exec:/usr/local/bin/aliases_procexec root.t...@bsd.ac" listen on socket # To accept external mail, replace with: listen on all # listen on lo0 action "local_mail" mbox alias action "outbound" relay action "bsd.ac" relay host smtp://10.7.0.1 # Uncomment the following to accept external mail for domain "example.org" # # match from any for domain "example.org" action "local_mail" match from local for local action "local_mail" match from local for domain "bsd.ac" action "bsd.ac" match from local for any action "outbound" $ cat /usr/local/bin/aliases_procexec #!/bin/ksh user="${1:-r...@bsd.ac}" while read line do reqid="$(echo $line | awk -F'|' '{ print $5; }')" reply="TABLE-RESULT|$reqid|FOUND|$user" echo $reply done < /dev/stdin exit 0 Does this not work as providing configuration parameters? So if we replace all backends by procexec model it will be possible to have configuration such as table aliasesA file /etc/mail/aliases table aliasesB db /etc/mail/aliases.db table aliasesC /usr/local/bin/aliases_procexec "root.t...@bsd.ac" table where the 'file', 'db' can be executables in /usr/libexec/smtpd or absolute paths. This may be a possible thing to do but maybe it can be done after procexec is tested a bit. Hopefully I've addressed the proper concerns. Best, Aisha Gilles On 8 Jun 2021, at 23:04, Aisha Tammy wrote: Hi, I've attached a slightly updated patch for the procexec. Ping for someone to take a look :) Cheers, Aisha diff --git a/usr.sbin/smtpd/smtpctl/Makefile b/usr.sbin/smtpd/smtpctl/Makefile index ef8148be8c9..2e8beff1ad1 100644 --- a/usr.sbin/smtpd/smtpctl/Makefile +++ b/usr.sbin/smtpd/smtpctl/Makefile @@ -48,6 +48,7 @@ SRCS+=table_static.c SRCS+= table_db.c SRCS+= table_getpwnam.c SRCS+= table_proc.c +SRCS+= table_procexec.c SRCS+= unpack_dns.c SRCS+= spfwalk.c diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h index be934112103..221f24fbdc4 100644 --- a/usr.sbin/smtpd/smtpd.h +++ b/usr.sbin/smtpd/smtpd.h @@ -1656,6 +1656,7 @@ int table_regex_match(const char *, const char *); voidtable_open_all(struct smtpd *); voidtable_dump_all(struct smtpd *); voidtable_close_all(struct smtpd *); +const char *table_service_name(enum table_service ); /* to.c */ diff --git a/usr.sbin/smtpd/smtpd/Makefile b/usr.sbin/smtpd/smtpd/Makefile index b31d4e42224..debfc8d8ab7 100644 --- a/usr.sbin/smtpd/smtpd/Makefile +++ b/usr.sbin/smtpd/smtpd/Makefile @@ -63,6 +63,7 @@ SRCS+=compress_gzip.c SRCS+= table_db.c SRCS+= table_getpwnam.c SRCS+= table_proc.c +SRCS+= table_procexec.c SRCS+= table_static.c SRCS+= queue_fs.c diff --git a/usr.sbin/smtpd/table.c b/usr.sbin/smtpd/table.c index 1d82d88b81a..0c67d
Re: add table_procexec in smtpd
Hi, I've attached a slightly updated patch for the procexec. Ping for someone to take a look :) Cheers, Aisha diff --git a/usr.sbin/smtpd/smtpctl/Makefile b/usr.sbin/smtpd/smtpctl/Makefile index ef8148be8c9..2e8beff1ad1 100644 --- a/usr.sbin/smtpd/smtpctl/Makefile +++ b/usr.sbin/smtpd/smtpctl/Makefile @@ -48,6 +48,7 @@ SRCS+=table_static.c SRCS+= table_db.c SRCS+= table_getpwnam.c SRCS+= table_proc.c +SRCS+= table_procexec.c SRCS+= unpack_dns.c SRCS+= spfwalk.c diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h index be934112103..221f24fbdc4 100644 --- a/usr.sbin/smtpd/smtpd.h +++ b/usr.sbin/smtpd/smtpd.h @@ -1656,6 +1656,7 @@ int table_regex_match(const char *, const char *); void table_open_all(struct smtpd *); void table_dump_all(struct smtpd *); void table_close_all(struct smtpd *); +const char *table_service_name(enum table_service ); /* to.c */ diff --git a/usr.sbin/smtpd/smtpd/Makefile b/usr.sbin/smtpd/smtpd/Makefile index b31d4e42224..debfc8d8ab7 100644 --- a/usr.sbin/smtpd/smtpd/Makefile +++ b/usr.sbin/smtpd/smtpd/Makefile @@ -63,6 +63,7 @@ SRCS+=compress_gzip.c SRCS+= table_db.c SRCS+= table_getpwnam.c SRCS+= table_proc.c +SRCS+= table_procexec.c SRCS+= table_static.c SRCS+= queue_fs.c diff --git a/usr.sbin/smtpd/table.c b/usr.sbin/smtpd/table.c index 1d82d88b81a..0c67d205065 100644 --- a/usr.sbin/smtpd/table.c +++ b/usr.sbin/smtpd/table.c @@ -46,8 +46,8 @@ extern struct table_backend table_backend_static; extern struct table_backend table_backend_db; extern struct table_backend table_backend_getpwnam; extern struct table_backend table_backend_proc; +extern struct table_backend table_backend_procexec; -static const char * table_service_name(enum table_service); static int table_parse_lookup(enum table_service, const char *, const char *, union lookup *); static int parse_sockaddr(struct sockaddr *, int, const char *); @@ -59,6 +59,7 @@ static struct table_backend *backends[] = { _backend_db, _backend_getpwnam, _backend_proc, + _backend_procexec, NULL }; @@ -77,7 +78,7 @@ table_backend_lookup(const char *backend) return NULL; } -static const char * +const char * table_service_name(enum table_service s) { switch (s) { diff --git a/usr.sbin/smtpd/table_procexec.c b/usr.sbin/smtpd/table_procexec.c new file mode 100644 index 000..88bfc435fb3 --- /dev/null +++ b/usr.sbin/smtpd/table_procexec.c @@ -0,0 +1,346 @@ +/* + * Copyright (c) 2013 Eric Faurot + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "smtpd.h" +#include "log.h" + +#define PROTOCOL_VERSION "1" + +static int table_procexec_open(struct table *); +static int table_procexec_update(struct table *); +static void table_procexec_close(struct table *); +static int table_procexec_lookup(struct table *, enum table_service, const char *, char **); +static int table_procexec_fetch(struct table *, enum table_service, char **); + +struct table_backend table_backend_procexec = { + "proc-exec", + K_ANY, + NULL, + NULL, + NULL, + table_procexec_open, + table_procexec_update, + table_procexec_close, + table_procexec_lookup, + table_procexec_fetch, +}; + +struct procexec_handle { + FILE*backend_w; + FILE*backend_r; + pid_t pid; +}; + +static int +table_procexec_open(struct table *t) { + struct procexec_handle *pe_handle; + pid_t pid; + int sp[2]; + int execr; + char exec[_POSIX_ARG_MAX]; + + if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, sp) == -1){ + fatalx("procexec - socket pair: %s", t->t_name); + } + + pe_handle = xcalloc(1, sizeof(*pe_handle)); + + if ((pid = fork()) == -1) { + fatalx("procexec - fork: %s", t->t_name); + } + + if (pid > 0) { + close(sp[0]); + FILE *back
add table_procexec in smtpd
Hi all, I've attached a diff to add table_procexec as a table backend in smtpd(8). This imports the table_procexec from opensmtpd-extras, which is available upstream but is not present in the port. I've successfully replaced the standard aliases table table aliases file:/etc/mail/aliases with a very simple proof of concept shell script which forwards all mail to a single account (r...@bsd.ac) table aliases proc-exec:/usr/local/bin/aliases_procexec The shell script (/usr/local/bin/aliases_procexec) is #!/bin/ksh while read line do reqid="$(echo $line | awk -F'|' '{ print $5; }')" reply="table-result|$reqid|found|r...@bsd.ac" echo $reply done < /dev/stdin exit 0 The full /etc/mail/smtpd.conf is # $OpenBSD: smtpd.conf,v 1.14 2019/11/26 20:14:38 gilles Exp $ # This is the smtpd server system-wide configuration file. # See smtpd.conf(5) for more information. table aliases proc-exec:/usr/local/bin/aliases_procexec listen on socket # To accept external mail, replace with: listen on all # listen on lo0 action "local_mail" mbox alias action "outbound" relay action "bsd.ac" relay host smtp://10.7.0.1 # Uncomment the following to accept external mail for domain "example.org" # # match from any for domain "example.org" action "local_mail" match from local for local action "local_mail" match from local for domain "bsd.ac" action "bsd.ac" match from local for any action "outbound" This diff is still a very early work, so I'm hoping for comments to improve the work. Some points that are still left to do - document the line protocol in table(5) - add a better reference implementation, maybe a replacement for aliases - I've left in the table_procexec_check but commented it out. I am unsure if that is needed at all. - Maybe my method of closing, in table_procexec_close is not the best. (and maybe more are still left but those can be thought of later) This is my first diff for anything in base, so I may have made some mistakes. Comments and improvements would be really nice. Cheers, Aisha PS: I've cc'ed eric@ as this was their original work and I'm just modifying it. diff --git a/usr.sbin/smtpd/smtpctl/Makefile b/usr.sbin/smtpd/smtpctl/Makefile index ef8148be8c9..2e8beff1ad1 100644 --- a/usr.sbin/smtpd/smtpctl/Makefile +++ b/usr.sbin/smtpd/smtpctl/Makefile @@ -48,6 +48,7 @@ SRCS+=table_static.c SRCS+= table_db.c SRCS+= table_getpwnam.c SRCS+= table_proc.c +SRCS+= table_procexec.c SRCS+= unpack_dns.c SRCS+= spfwalk.c diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h index be934112103..221f24fbdc4 100644 --- a/usr.sbin/smtpd/smtpd.h +++ b/usr.sbin/smtpd/smtpd.h @@ -1656,6 +1656,7 @@ int table_regex_match(const char *, const char *); void table_open_all(struct smtpd *); void table_dump_all(struct smtpd *); void table_close_all(struct smtpd *); +const char *table_service_name(enum table_service ); /* to.c */ diff --git a/usr.sbin/smtpd/smtpd/Makefile b/usr.sbin/smtpd/smtpd/Makefile index b31d4e42224..e2f6e82c6e8 100644 --- a/usr.sbin/smtpd/smtpd/Makefile +++ b/usr.sbin/smtpd/smtpd/Makefile @@ -64,6 +64,7 @@ SRCS+=table_db.c SRCS+= table_getpwnam.c SRCS+= table_proc.c SRCS+= table_static.c +SRCS+= table_procexec.c SRCS+= queue_fs.c SRCS+= queue_null.c diff --git a/usr.sbin/smtpd/table.c b/usr.sbin/smtpd/table.c index 1d82d88b81a..0c67d205065 100644 --- a/usr.sbin/smtpd/table.c +++ b/usr.sbin/smtpd/table.c @@ -46,8 +46,8 @@ extern struct table_backend table_backend_static; extern struct table_backend table_backend_db; extern struct table_backend table_backend_getpwnam; extern struct table_backend table_backend_proc; +extern struct table_backend table_backend_procexec; -static const char * table_service_name(enum table_service); static int table_parse_lookup(enum table_service, const char *, const char *, union lookup *); static int parse_sockaddr(struct sockaddr *, int, const char *); @@ -59,6 +59,7 @@ static struct table_backend *backends[] = { _backend_db, _backend_getpwnam, _backend_proc, + _backend_procexec, NULL }; @@ -77,7 +78,7 @@ table_backend_lookup(const char *backend) return NULL; } -static const char * +const char * table_service_name(enum table_service s) { switch (s) { diff --git a/usr.sbin/smtpd/table_procexec.c b/usr.sbin/smtpd/table_procexec.c new file mode 100644 index 000..269c55ecd9f --- /dev/null +++ b/usr.sbin/smtpd/table_procexec.c @@ -0,0 +1,355 @@ +/* + * Copyright (c) 2013 Eric Faurot + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS&q
Re: services(5): more cleanup
On 5/11/21 9:04 PM, Kurt Mosiejczuk wrote: On Wed, May 12, 2021 at 01:13:55AM +0200, Jeremie Courreges-Anglas wrote: I'd like to drop SWAT, unofficial and dropped by the samba project around the switch to samba4. - moved smtps/465 to the standards section (rfc8314) The new service was named "submissions". I guess we should use both that and the "smtps" alias. https://datatracker.ietf.org/doc/html/rfc8314#section-7.3 ok? A quick question, does this mean that the port in pf.conf will also have to be renamed? I have a few machines which use something to the effect of `pass in on egress proto tcp to port smtps ...`. Will that be broken by this? Similarly smtpd.conf? Or do they do this port-name translation separately? Best, Aisha ok kmos --Kurt Index: services === RCS file: /d/cvs/src/etc/services,v retrieving revision 1.100 diff -u -p -p -u -r1.100 services --- services5 May 2021 11:49:17 - 1.100 +++ services11 May 2021 23:03:12 - @@ -123,7 +123,7 @@ microsoft-ds445/tcp # Microsoft-DS microsoft-ds 445/udp # Microsoft-DS kpasswd 464/tcp # Kerberos 5 password changing kpasswd 464/udp # Kerberos 5 password changing -smtps 465/tcp # mail message submission (TLS) +submissions465/tcp smtps # mail message submission (TLS) photuris 468/tcp # Photuris Key Management photuris 468/udp isakmp500/udp # ISAKMP key management @@ -296,7 +296,6 @@ kerberos_master 751/udp # Kerberos 4 kerberos_master 751/tcp # Kerberos 4 kadmin krb_prop 754/tcp hprop # Kerberos slave propagation krbupdate 760/tcp kreg# BSD Kerberos registration -swat 901/tcp # Samba Web Administration Tool datametrics 1645/udp ekshell2 2106/tcp# Encrypted kshell - UColorado, Boulder webster 2627/tcp# Network dictionary -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: smtpd: use libtls
On 1/27/21 7:29 AM, gil...@poolp.org wrote: > January 27, 2021 9:47 AM, "Lauri Tirkkonen" wrote: > >> On Wed, Jan 27 2021 09:36:31 +0100, Eric Faurot wrote: >> >>> There has been a plan for some time now to make smtpd use libtls >>> instead of openssl. Recent changes in libtls allow to move forward >>> with this. Here is a diff to start the switch. I've tried to keep >>> it as small as possible, sticking to the necessary changes. There is >>> still a lot of code that can be removed but that will be done in a >>> second time. >> >> I'm all for this, and sorry for screaming from the gallery, but I want to >> ask - >> is there a plan relating to libtls for portable OpenSMTPD? As it stands, >> OpenSSL-based systems are largely unable to use libtls (which in itself is a >> shame) - how would this change make it to portable? >> > > TL;DR: > In January 2020, I adapted OpenSMTPD to libtls for the first time and did it > both > for OpenBSD and portable. Since many systems didn't have LibreSSL available, > this > resulted in libtls being brought to the openbsd-compat layer and adapted to > build > with OpenSSL. The plan is to use libtls from LibreSSL if detected, otherwise > take > the openbsd-compat version if OpenSSL is detected. > > More (outdated) details here: > > https://poolp.org/posts/2020-01-22/january-2020-opensmtpd-work-libasr-and-libtls/ > > > As a side note: > > The work eric@ did on the libtls conversion was based on my diff but diverged > and > I will have to adapt my work from last year to make it work again. I'll take > care > of making it work again once his work is committed. > > As of today, there's no one but me working on the portable release so it > would be > nice if people interested in a portable release would step up to help. > Is it not possible to use libretls - https://git.causal.agency/libretls/about/ They plan to maintain such a compatibility layer of libtls with openssl with minimal changes. It might be better to use their effort rather than adding a burden of both a compat libtls and opensmtpd in the portable version. Quite a lot of distributions already have this present so this might be a good idea to use their work. Thoughts? Best, Aisha
small typo in imsg_init.3
Hi, While creating a portable version of imsg, I noticed a small typo in the imsg_init.3 man page which says the returned value is 'len' instead of 'datalen'. Attached the patch to fix it. OK? Cheers, Aisha diff --git a/lib/libutil/imsg_init.3 b/lib/libutil/imsg_init.3 index 18720d1d59b..e7dc6ab928d 100644 --- a/lib/libutil/imsg_init.3 +++ b/lib/libutil/imsg_init.3 @@ -186,7 +186,7 @@ appends to bytes of ancillary data pointed to by .Fa data . It returns -.Fa len +.Fa datalen if it succeeds, \-1 otherwise. .Pp .Fn imsg_close
Re: Fwd: opensmtpd can't handle long lines in aliases table
On 9/18/20 8:55 AM, Martijn van Duren wrote: > That's what you get if you're doing a quick diff on the job. > Missed a free case which is in Edgar's original diff. > > Noticed by tb@ > Thanks a lot for modifications. Yes, I've tested the new version and am able to send mails with a ~3050 line file in the table. Aisha > On Fri, 2020-09-18 at 14:46 +0200, Martijn van Duren wrote: >> Could you try the diff below? >> It should do exactly the same thing with less code. >> >> martijn@ >> >> On Fri, 2020-09-18 at 08:30 -0400, Aisha Tammy wrote: >>> Hi, >>> >>> Edgar (cc'ed) has kindly provided patches to fix a buffer error in >>> mailaddr.c >>> for opensmtpd. >>> >>> I've minimally tested it and am forwarding the patches. >>> >>> Would like to be able to get them into 6.8 release as this is quite >>> problematic >>> with lots of aliases. >>> >>> Thanks, >>> Aisha >>> >>> >>> Forwarded Message >>> Subject: Re: opensmtpd can't handle long lines in aliases table >>> Date: Thu, 6 Aug 2020 19:47:33 -0500 >>> From: Edgar Pettijohn >>> To: AIsha Tammy >>> >>> Here are a few simple patches as discussed. These were written to apply >>> against current. However, they are pretty simple and may well apply to >>> others. With that in mind if you are using any filters they may not >>> work. My production system is still a couple version behind and the >>> current smtpd wouldn't work with some of my custom filters. So I had to >>> use a fairly basic temporary config for testing. I'm also including my >>> test table. >>> >>> Steps involved: (untested off memory mostly, use doas as necessary) >>> >>> cd /usr >>> cvs -d $CVSROOT checkout src >>> >>> cp *.patch /usr/src/usr.sbin/smtpd >>> cd /usr/src/usr.sbin/smtpd >>> >>> for file in `ls *.patch` >>> do >>> patch < $file >>> done >>> >>> make >>> rcctl stop smtpd >>> >>> #use the just built version at /usr/src/usr.sbin/smtpd/smtpd/smtpd >>> smtpd/smtpd -d -T expand >>> >>> send a test email >>> >>> if all goes well run it for an appropriat amount of time and make sure >>> there are not issues. If your satisfied send the patches to bugs@. >>> >>> Enjoy, >>> >>> Edgar >>> > Index: mailaddr.c > === > RCS file: /cvs/src/usr.sbin/smtpd/mailaddr.c,v > retrieving revision 1.3 > diff -u -p -r1.3 mailaddr.c > --- mailaddr.c31 May 2018 21:06:12 - 1.3 > +++ mailaddr.c18 Sep 2020 12:55:41 - > @@ -80,12 +80,10 @@ int > mailaddr_line(struct maddrmap *maddrmap, const char *s) > { > struct maddrnodemn; > - charbuffer[LINE_MAX]; > - char *p, *subrcpt; > + char *p, *subrcpt, *buffer; > int ret; > > - memset(buffer, 0, sizeof buffer); > - if (strlcpy(buffer, s, sizeof buffer) >= sizeof buffer) > + if ((buffer = strdup(s)) == NULL) > return 0; > > p = buffer; > @@ -93,11 +91,15 @@ mailaddr_line(struct maddrmap *maddrmap, > subrcpt = strip(subrcpt); > if (subrcpt[0] == '\0') > continue; > - if (!text_to_mailaddr(, subrcpt)) > + if (!text_to_mailaddr(, subrcpt)) { > + free(buffer); > return 0; > + } > log_debug("subrcpt: [%s]", subrcpt); > maddrmap_insert(maddrmap, ); > } > + > + free(buffer); > > if (ret >= 0) > return 1; >
Fwd: opensmtpd can't handle long lines in aliases table
Hi, Edgar (cc'ed) has kindly provided patches to fix a buffer error in mailaddr.c for opensmtpd. I've minimally tested it and am forwarding the patches. Would like to be able to get them into 6.8 release as this is quite problematic with lots of aliases. Thanks, Aisha Forwarded Message Subject: Re: opensmtpd can't handle long lines in aliases table Date: Thu, 6 Aug 2020 19:47:33 -0500 From: Edgar Pettijohn To: AIsha Tammy Here are a few simple patches as discussed. These were written to apply against current. However, they are pretty simple and may well apply to others. With that in mind if you are using any filters they may not work. My production system is still a couple version behind and the current smtpd wouldn't work with some of my custom filters. So I had to use a fairly basic temporary config for testing. I'm also including my test table. Steps involved: (untested off memory mostly, use doas as necessary) cd /usr cvs -d $CVSROOT checkout src cp *.patch /usr/src/usr.sbin/smtpd cd /usr/src/usr.sbin/smtpd for file in `ls *.patch` do patch < $file done make rcctl stop smtpd #use the just built version at /usr/src/usr.sbin/smtpd/smtpd/smtpd smtpd/smtpd -d -T expand send a test email if all goes well run it for an appropriat amount of time and make sure there are not issues. If your satisfied send the patches to bugs@. Enjoy, Edgar Index: mailaddr.c === RCS file: /cvs/src/usr.sbin/smtpd/mailaddr.c,v retrieving revision 1.3 diff -u -p -u -r1.3 mailaddr.c --- mailaddr.c 31 May 2018 21:06:12 - 1.3 +++ mailaddr.c 7 Aug 2020 00:14:24 - @@ -77,15 +77,16 @@ mailaddr_line_split(char **line, char ** } int -mailaddr_line(struct maddrmap *maddrmap, const char *s) +mailaddr_line(struct maddrmap *maddrmap, const char *s, size_t len) { struct maddrnodemn; - charbuffer[LINE_MAX]; - char *p, *subrcpt; + char *p, *subrcpt, *buffer; int ret; - memset(buffer, 0, sizeof buffer); - if (strlcpy(buffer, s, sizeof buffer) >= sizeof buffer) + if ((buffer = calloc(len + 1, sizeof(char *))) == NULL) + return 0; + + if (strlcpy(buffer, s, len + 1) >= len + 1) return 0; p = buffer; @@ -93,11 +94,15 @@ mailaddr_line(struct maddrmap *maddrmap, subrcpt = strip(subrcpt); if (subrcpt[0] == '\0') continue; - if (!text_to_mailaddr(, subrcpt)) + if (!text_to_mailaddr(, subrcpt)) { + free(buffer); return 0; + } log_debug("subrcpt: [%s]", subrcpt); maddrmap_insert(maddrmap, ); } + + free(buffer); if (ret >= 0) return 1; Index: smtpd.h === RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v retrieving revision 1.656 diff -u -p -u -r1.656 smtpd.h --- smtpd.h 8 Apr 2020 07:30:44 - 1.656 +++ smtpd.h 7 Aug 2020 00:20:32 - @@ -1438,7 +1438,7 @@ int makemap(int, int, char **); /* mailaddr.c */ -int mailaddr_line(struct maddrmap *, const char *); +int mailaddr_line(struct maddrmap *, const char *, size_t); void maddrmap_init(struct maddrmap *); void maddrmap_insert(struct maddrmap *, struct maddrnode *); void maddrmap_free(struct maddrmap *); Index: table.c === RCS file: /cvs/src/usr.sbin/smtpd/table.c,v retrieving revision 1.48 diff -u -p -u -r1.48 table.c --- table.c 10 Jan 2019 07:40:52 - 1.48 +++ table.c 7 Aug 2020 00:20:08 - @@ -600,7 +600,7 @@ table_parse_lookup(enum table_service se if (lk->maddrmap == NULL) return (-1); maddrmap_init(lk->maddrmap); - if (!mailaddr_line(lk->maddrmap, line)) { + if (!mailaddr_line(lk->maddrmap, line, len)) { maddrmap_free(lk->maddrmap); return (-1); }
Re: ldapd: adding bsd.schema
On 9/10/20 2:03 AM, Robert Klein wrote: > On Sat, 5 Sep 2020 18:47:08 -0400 > Aisha Tammy wrote: > >> Sorry for the late reply. >> >> On 8/12/20 8:19 AM, Robert Klein wrote: >>> Hi, >>> >>> On Wed, 12 Aug 2020 09:00:18 +0200 >>> Theo Buehler wrote: >>> >>>> On Tue, Aug 11, 2020 at 10:22:51PM -0400, Aisha Tammy wrote: >>>>> Another bump. >>>> >>>> I think this is useful and am ok with this. >>>> >>>> Are there any concerns? If not, I'm going to commit it tomorrow. >>> >>> for an sshPublicKey attribute, there's a “openssh-lpk” schema which >>> seems to be in common use. It's defined as >>> >>> # octetString SYNTAX >>> attributetype ( 1.3.6.1.4.1.24552.500.1.1.1.13 NAME 'sshPublicKey' >>> DESC 'OpenSSH Public key' >>> EQUALITY octetStringMatch >>> SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 ) >>> >> I prefer the non-octet version mostly because of inconsistent spacing >> when >> >> copy pasting. > > IA5Match precludes non-ascii comments. BTW, your version has 'SSH > public key' as DESC. I suppose it means a 'OpenSSH public key', as > above, not a RFC4716 public key which wouldn't make much sense in > OpenBSD context I guess. > Haha, I wasn't even aware SSH public key was a different thing >.< (how do ya'll know/remember these weird RFCs...) Updated patch with OpenSSH public key. OK? Aisha > >> >> >> >>> # printableString SYNTAX yes|no >>> objectclass ( 1.3.6.1.4.1.24552.500.1.1.2.0 NAME 'ldapPublicKey' SUP >>> top AUXILIARY DESC 'OpenSSH LPK objectclass' >>> MUST uid >>> MAY sshPublicKey >>> ) >>> >>> though there are versions of the “ldapPublicKey” definitions with >>> both uid and sshPublicKye in the MUST and both in the MAY clause. >>> The “both MAY” version is imho more flexible. >>> >>> >>> The original mail proposing bsd.schema seems to have added both >>> “shadowPassword” and “bsdaccount” more as an afterthought, it seems. >>> >> The bsd account is a bit more flexible than the ldapPublicKey and can >> be substituted for this. >> I am fine with moving the `uid` to MAY as well, that would be very >> nice for virtual user setups, where uid is unimportant and not used. > > +1 > > > Best regards > Robert > > >> >> I've attached the updated patch which moves uid to MAY. >> I would really like this to be in 6.8. >> >> OK? >> >> Thanks, >> Aisha >> >>> >>> Best regards >>> Robert >>> >>> >>>> >>>> Index: etc/examples/ldapd.conf >>>> === >>>> RCS file: /cvs/src/etc/examples/ldapd.conf,v >>>> retrieving revision 1.1 >>>> diff -u -p -u -p -r1.1 ldapd.conf >>>> --- etc/examples/ldapd.conf11 Jul 2014 21:20:10 - >>>> 1.1 +++ etc/examples/ldapd.conf18 May 2018 10:09:45 - >>>> @@ -3,6 +3,7 @@ >>>> schema "/etc/ldap/core.schema" >>>> schema "/etc/ldap/inetorgperson.schema" >>>> schema "/etc/ldap/nis.schema" >>>> +schema "/etc/ldap/bsd.schema" >>>> >>>> listen on lo0 >>>> listen on "/var/run/ldapi" >>>> Index: usr.sbin/ldapd/Makefile >>>> === >>>> RCS file: /cvs/src/usr.sbin/ldapd/Makefile,v >>>> retrieving revision 1.15 >>>> diff -u -p -u -p -r1.15 Makefile >>>> --- usr.sbin/ldapd/Makefile20 Jan 2017 11:55:08 - >>>> 1.15 +++ usr.sbin/ldapd/Makefile 18 May 2018 10:09:45 - >>>> @@ -17,7 +17,8 @@ CFLAGS+= -Wshadow -Wpointer-arith -Wcast >>>> CFLAGS+= -Wsign-compare >>>> CLEANFILES+= y.tab.h parse.c >>>> >>>> -SCHEMA_FILES= core.schema \ >>>> +SCHEMA_FILES= bsd.schema \ >>>> + core.schema \ >>>>inetorgperson.schema \ >>>>nis.schema >>>> >>>> Index: usr.sbin/ldapd/schema/bsd.schema >>>> === >>>> RCS file: usr.sbin/ldapd/schema/bsd.schema >>>> diff -N usr.sbin/ldapd/schema/bsd.schema >>>> --- /dev/null 1 Jan 1970 00:00:0
Re: ldapd: adding bsd.schema
Sorry for the late reply. On 8/12/20 8:19 AM, Robert Klein wrote: > Hi, > > On Wed, 12 Aug 2020 09:00:18 +0200 > Theo Buehler wrote: > >> On Tue, Aug 11, 2020 at 10:22:51PM -0400, Aisha Tammy wrote: >>> Another bump. >> >> I think this is useful and am ok with this. >> >> Are there any concerns? If not, I'm going to commit it tomorrow. > > for an sshPublicKey attribute, there's a “openssh-lpk” schema which > seems to be in common use. It's defined as > > # octetString SYNTAX > attributetype ( 1.3.6.1.4.1.24552.500.1.1.1.13 NAME 'sshPublicKey' > DESC 'OpenSSH Public key' > EQUALITY octetStringMatch > SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 ) > I prefer the non-octet version mostly because of inconsistent spacing when copy pasting. > # printableString SYNTAX yes|no > objectclass ( 1.3.6.1.4.1.24552.500.1.1.2.0 NAME 'ldapPublicKey' SUP > top AUXILIARY DESC 'OpenSSH LPK objectclass' > MUST uid > MAY sshPublicKey > ) > > though there are versions of the “ldapPublicKey” definitions with both > uid and sshPublicKye in the MUST and both in the MAY clause. The > “both MAY” version is imho more flexible. > > > The original mail proposing bsd.schema seems to have added both > “shadowPassword” and “bsdaccount” more as an afterthought, it seems. > The bsd account is a bit more flexible than the ldapPublicKey and can be substituted for this. I am fine with moving the `uid` to MAY as well, that would be very nice for virtual user setups, where uid is unimportant and not used. I've attached the updated patch which moves uid to MAY. I would really like this to be in 6.8. OK? Thanks, Aisha > > Best regards > Robert > > >> >> Index: etc/examples/ldapd.conf >> === >> RCS file: /cvs/src/etc/examples/ldapd.conf,v >> retrieving revision 1.1 >> diff -u -p -u -p -r1.1 ldapd.conf >> --- etc/examples/ldapd.conf 11 Jul 2014 21:20:10 - >> 1.1 +++ etc/examples/ldapd.conf 18 May 2018 10:09:45 - >> @@ -3,6 +3,7 @@ >> schema "/etc/ldap/core.schema" >> schema "/etc/ldap/inetorgperson.schema" >> schema "/etc/ldap/nis.schema" >> +schema "/etc/ldap/bsd.schema" >> >> listen on lo0 >> listen on "/var/run/ldapi" >> Index: usr.sbin/ldapd/Makefile >> === >> RCS file: /cvs/src/usr.sbin/ldapd/Makefile,v >> retrieving revision 1.15 >> diff -u -p -u -p -r1.15 Makefile >> --- usr.sbin/ldapd/Makefile 20 Jan 2017 11:55:08 - >> 1.15 +++ usr.sbin/ldapd/Makefile 18 May 2018 10:09:45 - >> @@ -17,7 +17,8 @@ CFLAGS+= -Wshadow -Wpointer-arith -Wcast >> CFLAGS+=-Wsign-compare >> CLEANFILES+=y.tab.h parse.c >> >> -SCHEMA_FILES= core.schema \ >> +SCHEMA_FILES= bsd.schema \ >> +core.schema \ >> inetorgperson.schema \ >> nis.schema >> >> Index: usr.sbin/ldapd/schema/bsd.schema >> === >> RCS file: usr.sbin/ldapd/schema/bsd.schema >> diff -N usr.sbin/ldapd/schema/bsd.schema >> --- /dev/null1 Jan 1970 00:00:00 - >> +++ usr.sbin/ldapd/schema/bsd.schema 18 May 2018 10:09:45 - >> @@ -0,0 +1,17 @@ >> +attributetype ( 1.3.6.1.4.1.30155.115.2 NAME 'shadowPassword' >> +DESC 'POSIX hashed password' >> +EQUALITY caseExactIA5Match >> +SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 ) >> + >> +attributetype ( 1.3.6.1.4.1.30155.115.3 NAME 'sshPublicKey' >> +DESC 'SSH public key' >> +EQUALITY caseExactIA5Match >> +SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 ) >> + >> +objectclass ( 1.3.6.1.4.1.30155.115.1 NAME 'bsdAccount' >> +SUP top >> +AUXILIARY >> +DESC 'Abstraction of an account with OpenBSD attributes' >> +MUST ( uid ) >> +MAY ( shadowPassword $ shadowExpire $ modifyTimestamp $ >> userClass $ >> +sshPublicKey )) >> > diff --git a/etc/examples/ldapd.conf b/etc/examples/ldapd.conf index 1bc6aa462c1..183563d6f9a 100644 --- a/etc/examples/ldapd.conf +++ b/etc/examples/ldapd.conf @@ -3,6 +3,7 @@ schema "/etc/ldap/core.schema" schema "/etc/ldap/inetorgperson.schema" schema "/etc/ldap/nis.schema" +schema "/etc/ldap/bsd.schema" listen on lo0 listen on "/var/run/ldapi" diff --git a/usr.sbin/ldapd/Makefile b/usr.sbin/ldapd/Makefile index bf445832576..5af25895787 100644 ---
Re: ldapd: adding bsd.schema
On 8/2/20 9:34 AM, Aisha Tammy wrote: > On 7/26/20 5:25 PM, Aisha Tammy wrote: >> On 7/26/20 5:21 PM, Aisha Tammy wrote: >>> Hi, >>> Am reviving an old thread from >>> https://marc.info/?l=openbsd-tech=152663835315469=4 >>> (i did cc reyk@ sorry if it is noise) >>> >>> For some reason seems like the patch didn't go through... >>> >>> I am reattaching it here, maybe someone can take a look and >>> see if it can be merged ? >>> Getting sshPublicKey would be really nice! >>> >>> Aisha >>> >> >> >> reattaching it because thunderbird >> > > Bump, can anyone see if this is fine ? > > Thanks, > Aisha > Another bump. Aisha
Re: ldapd: adding bsd.schema
On 7/26/20 5:25 PM, Aisha Tammy wrote: > On 7/26/20 5:21 PM, Aisha Tammy wrote: >> Hi, >> Am reviving an old thread from >> https://marc.info/?l=openbsd-tech=152663835315469=4 >> (i did cc reyk@ sorry if it is noise) >> >> For some reason seems like the patch didn't go through... >> >> I am reattaching it here, maybe someone can take a look and >> see if it can be merged ? >> Getting sshPublicKey would be really nice! >> >> Aisha >> > > > reattaching it because thunderbird > Bump, can anyone see if this is fine ? Thanks, Aisha
Re: ldapd: adding bsd.schema
On 7/26/20 5:21 PM, Aisha Tammy wrote: > Hi, > Am reviving an old thread from > https://marc.info/?l=openbsd-tech=152663835315469=4 > (i did cc reyk@ sorry if it is noise) > > For some reason seems like the patch didn't go through... > > I am reattaching it here, maybe someone can take a look and > see if it can be merged ? > Getting sshPublicKey would be really nice! > > Aisha > reattaching it because thunderbird Index: etc/examples/ldapd.conf === RCS file: /cvs/src/etc/examples/ldapd.conf,v retrieving revision 1.1 diff -u -p -u -p -r1.1 ldapd.conf --- etc/examples/ldapd.conf 11 Jul 2014 21:20:10 - 1.1 +++ etc/examples/ldapd.conf 18 May 2018 10:09:45 - @@ -3,6 +3,7 @@ schema "/etc/ldap/core.schema" schema "/etc/ldap/inetorgperson.schema" schema "/etc/ldap/nis.schema" +schema "/etc/ldap/bsd.schema" listen on lo0 listen on "/var/run/ldapi" Index: usr.sbin/ldapd/Makefile === RCS file: /cvs/src/usr.sbin/ldapd/Makefile,v retrieving revision 1.15 diff -u -p -u -p -r1.15 Makefile --- usr.sbin/ldapd/Makefile 20 Jan 2017 11:55:08 - 1.15 +++ usr.sbin/ldapd/Makefile 18 May 2018 10:09:45 - @@ -17,7 +17,8 @@ CFLAGS+= -Wshadow -Wpointer-arith -Wcast CFLAGS+= -Wsign-compare CLEANFILES+= y.tab.h parse.c -SCHEMA_FILES= core.schema \ +SCHEMA_FILES= bsd.schema \ + core.schema \ inetorgperson.schema \ nis.schema Index: usr.sbin/ldapd/schema/bsd.schema === RCS file: usr.sbin/ldapd/schema/bsd.schema diff -N usr.sbin/ldapd/schema/bsd.schema --- /dev/null 1 Jan 1970 00:00:00 - +++ usr.sbin/ldapd/schema/bsd.schema 18 May 2018 10:09:45 - @@ -0,0 +1,17 @@ +attributetype ( 1.3.6.1.4.1.30155.115.2 NAME 'shadowPassword' + DESC 'POSIX hashed password' + EQUALITY caseExactIA5Match + SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 ) + +attributetype ( 1.3.6.1.4.1.30155.115.3 NAME 'sshPublicKey' + DESC 'SSH public key' + EQUALITY caseExactIA5Match + SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 ) + +objectclass ( 1.3.6.1.4.1.30155.115.1 NAME 'bsdAccount' + SUP top + AUXILIARY + DESC 'Abstraction of an account with OpenBSD attributes' + MUST ( uid ) + MAY ( shadowPassword $ shadowExpire $ modifyTimestamp $ userClass $ + sshPublicKey ))
ldapd: adding bsd.schema
Hi, Am reviving an old thread from https://marc.info/?l=openbsd-tech=152663835315469=4 (i did cc reyk@ sorry if it is noise) For some reason seems like the patch didn't go through... I am reattaching it here, maybe someone can take a look and see if it can be merged ? Getting sshPublicKey would be really nice! Aisha Index: etc/examples/ldapd.conf === RCS file: /cvs/src/etc/examples/ldapd.conf,v retrieving revision 1.1 diff -u -p -u -p -r1.1 ldapd.conf --- etc/examples/ldapd.conf 11 Jul 2014 21:20:10 - 1.1 +++ etc/examples/ldapd.conf 18 May 2018 10:09:45 - @@ -3,6 +3,7 @@ schema "/etc/ldap/core.schema" schema "/etc/ldap/inetorgperson.schema" schema "/etc/ldap/nis.schema" +schema "/etc/ldap/bsd.schema" listen on lo0 listen on "/var/run/ldapi" Index: usr.sbin/ldapd/Makefile === RCS file: /cvs/src/usr.sbin/ldapd/Makefile,v retrieving revision 1.15 diff -u -p -u -p -r1.15 Makefile --- usr.sbin/ldapd/Makefile 20 Jan 2017 11:55:08 - 1.15 +++ usr.sbin/ldapd/Makefile 18 May 2018 10:09:45 - @@ -17,7 +17,8 @@ CFLAGS+= -Wshadow -Wpointer-arith -Wcast CFLAGS+= -Wsign-compare CLEANFILES+= y.tab.h parse.c -SCHEMA_FILES= core.schema \ +SCHEMA_FILES= bsd.schema \ + core.schema \ inetorgperson.schema \ nis.schema Index: usr.sbin/ldapd/schema/bsd.schema === RCS file: usr.sbin/ldapd/schema/bsd.schema diff -N usr.sbin/ldapd/schema/bsd.schema --- /dev/null 1 Jan 1970 00:00:00 - +++ usr.sbin/ldapd/schema/bsd.schema18 May 2018 10:09:45 - @@ -0,0 +1,17 @@ +attributetype ( 1.3.6.1.4.1.30155.115.2 NAME 'shadowPassword' + DESC 'POSIX hashed password' + EQUALITY caseExactIA5Match + SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 ) + +attributetype ( 1.3.6.1.4.1.30155.115.3 NAME 'sshPublicKey' + DESC 'SSH public key' + EQUALITY caseExactIA5Match + SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 ) + +objectclass ( 1.3.6.1.4.1.30155.115.1 NAME 'bsdAccount' + SUP top + AUXILIARY + DESC 'Abstraction of an account with OpenBSD attributes' + MUST ( uid ) + MAY ( shadowPassword $ shadowExpire $ modifyTimestamp $ userClass $ + sshPublicKey ))
Re: symmetric toeplitz hashing
On 6/13/20 2:47 PM, Theo Buehler wrote: >>> Yes. The thing is that you need to convince yourself that this is still >>> uniformly distributed over the wanted numbers. But it's correct. In >>> fact, it's enough to flip a fixed bit, so you can get away with one call >>> to arc4random(). >> >> Its not immediately obvious because this is not always true. >> Only true if your bad seed was at least surjective onto the desired codomain. > > Since we start from a uniform distribution, surjective is not good > enough. It needs to be n:1. > Ah, yes, I missed that we're starting from a uniform distribution. I assumed the even the original generator one was non uniform. Which now that I think about it would be non trivial to convert to uniform. > Flipping the k-th bit swaps the numbers of odd parity (good seeds) with > the numbers of even parity (bad seeds). The suggested construction of > keeping a good seed and flipping the k-th bit of a bad one yields a 2:1 > mapping from all 16-bit numbers onto those with odd parity. We're good. > > David Higgs's code is also correct. Probably the easiest way to see that > is to start from a uniform distribution on [0, 2^16-1] x [0, 15] and map > it 32:1 onto the good seeds -- (x, k) maps to x if x is good, and to x > with the k-th bit flipped if x is bad. > Yep, thanks this follows once I get rid of my assumption. Aisha
Re: symmetric toeplitz hashing
On 6/13/20 9:19 AM, Theo Buehler wrote: > On Sat, Jun 13, 2020 at 08:46:13AM -0400, David Higgs wrote: >> On Fri, Jun 12, 2020 at 9:41 AM Theo Buehler wrote: >> >>> I finally found the time to think about the mathematics of this some >>> more and I'm now convinced that it's a sound construction. I hope that >>> one or the other observation below will be useful for you. >>> >>> The hash as it is now can be proved to produce values in the full range >>> of uint16_t, so that's good. >>> >>> As we discussed already, you can simplify the construction further. >>> >> >> [snip] >> >> >>> Next, I don't particularly like this magic STOEPLITZ_KEYSEED number, but >>> I guess we can live with it. >>> >>> Another option would be to generate the key seed randomly. You will get >>> a "good" hash that spreads out over all 16 bit values if and only if the >>> random value has an odd number of binary digits. >>> >>> I haven't thought hard about this, but I don't immediately see a better >>> way of generating such numbers than: >>> >>> int >>> stoeplitz_good_seed(uint16_t seed) >>> { >>> int ones = 0; >>> >>> while (seed > 0) { >>> ones += seed % 2; >>> seed /= 2; >>> } >>> >>> return ones % 2; >>> } >>> >>> uint16_t >>> stoeplitz_seed_init(void) >>> { >>> uint16_tseed; >>> >>> do { >>> seed = arc4random() & UINT16_MAX; >>> } while (!stoeplitz_good_seed(seed)); >>> >>> return seed; >>> } >>> >>> This will loop as long as it needs to get a good toeplitz key seed. >>> Each time there is a 50% chance that it will find one, so this will >>> need to loop n times with probability 1 / 2**n. This is basically the >>> same situation as for arc4random_uniform() with an upper bound that is >>> not a power of two. >>> >> >> I neither read nor understand the math but assuming you've described it >> correctly, you can do this more simply by realizing that a random bad seed >> can be made into a good seed by toggling one (random) bit. > > Yes. The thing is that you need to convince yourself that this is still > uniformly distributed over the wanted numbers. But it's correct. In > fact, it's enough to flip a fixed bit, so you can get away with one call > to arc4random(). Its not immediately obvious because this is not always true. Only true if your bad seed was at least surjective onto the desired codomain. > >> You also >> replace stoeplitz_good_seed() with __builtin_parity() and perhaps leverage >> some intrinsics? > > Others have pointed out off-list that one can use __builtin_popcount(), > but __builtin_parity() is exactly what I want. Is it available on all > architectures? > >> >> uint16_t >> stoeplitz_seed_init(void) { >> uint16_t seed; >> seed = arc4random() & UINT16_MAX; >> if (!stoeplitz_good_seed(seed)) >> seed ^= 1 << (arc4random() % 16); >> } >> >> --david >