Re: add table_procexec in smtpd

2021-06-22 Thread Aisha Tammy 


> >> 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
 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

Re: add table_procexec in smtpd

2021-06-12 Thread Aisha Tammy 

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

2021-06-09 Thread Aisha Tammy 
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("proc");
-   (void)strlcpy(path, backend, sizeof(path));
+   tb = table_backend_lookup("proc-exec");
if

Re: add table_procexec in smtpd

2021-06-09 Thread Aisha Tammy 




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

2021-06-09 Thread Aisha Tammy 

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

2021-06-08 Thread Aisha Tammy 
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 *backend_w, *backend_r;
+   if ((backend_w = fdopen(sp[1], "w")) == NULL)
+

add table_procexec in smtpd

2021-05-31 Thread Aisha Tammy 
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" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND

Re: services(5): more cleanup

2021-05-15 Thread Aisha Tammy 




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

2021-01-27 Thread Aisha Tammy 
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

2020-12-12 Thread Aisha Tammy 
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

2020-09-18 Thread Aisha Tammy 
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

2020-09-18 Thread Aisha Tammy 
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

2020-09-10 Thread Aisha Tammy 
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

2020-09-05 Thread Aisha Tammy 
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

2020-08-12 Thread Aisha Tammy 
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

2020-08-02 Thread Aisha Tammy 
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

2020-07-26 Thread Aisha Tammy 
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

2020-07-26 Thread Aisha Tammy 
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

2020-06-13 Thread Aisha Tammy 
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

2020-06-13 Thread Aisha Tammy 
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
>