Re: OpenSMTPD docs: smtpctl.8

2021-02-12 Thread Jason McIntyre
On Fri, Feb 12, 2021 at 03:23:38PM +, Larry Hynes wrote:
> 
> Index: smtpctl.8
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpctl.8,v
> retrieving revision 1.65
> diff -u -p -r1.65 smtpctl.8
> --- smtpctl.8 14 Sep 2020 09:48:08 -  1.65
> +++ smtpctl.8 12 Feb 2021 15:22:58 -
> @@ -172,8 +172,8 @@ Status of last delivery.
>  .It Cm show message Ar envelope-id
>  Display message content for the given ID.
>  .It Cm show queue
> -Display information concerning envelopes that are currently in the queue.
> -Each line of output describes a single envelope.
> +Display information about envelopes that are currently in the queue.
> +Each line of output relates to a single envelope.
>  It consists of the following fields, separated by a "|":
>  .Pp
>  .Bl -bullet -compact
> @@ -239,7 +239,7 @@ The route has a timeout registered to lo
>  reactivate or discard it.
>  .El
>  .It Cm show stats
> -Displays runtime statistics concerning
> +Displays runtime statistics for
>  .Xr smtpd 8 .
>  .It Cm show status
>  Shows if MTA, MDA and SMTP systems are currently running or paused.
> 

not taken - i think it's fine as is.
jmc



Re: OpenSMTPD docs: newaliases.8

2021-02-12 Thread Jason McIntyre
On Fri, Feb 12, 2021 at 03:21:16PM +, Larry Hynes wrote:
> 
> Index: newaliases.8
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/newaliases.8,v
> retrieving revision 1.12
> diff -u -p -r1.12 newaliases.8
> --- newaliases.8  20 Jul 2018 15:35:33 -  1.12
> +++ newaliases.8  12 Feb 2021 15:21:00 -
> @@ -27,7 +27,7 @@
>  .Sh DESCRIPTION
>  The
>  .Nm
> -utility makes changes to the mail aliases file visible to
> +utility updates a mail aliases file in use by
>  .Xr smtpd 8 .
>  It should be run every time the
>  .Xr aliases 5
> 

not taken - i think it's already clear enough.
jmc



Re: OpenSMTPD docs: makemap.8

2021-02-12 Thread Jason McIntyre
On Fri, Feb 12, 2021 at 03:19:53PM +, Larry Hynes wrote:
> Note: DNS is mentioned in this file as a way of accessing maps. Aside
> from this mention it appears to be undocumented.
> 
> Index: makemap.8
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/makemap.8,v
> retrieving revision 1.30
> diff -u -p -r1.30 makemap.8
> --- makemap.8 25 Nov 2018 14:41:16 -  1.30
> +++ makemap.8 12 Feb 2021 15:17:42 -
> @@ -29,17 +29,18 @@
>  .Op Fl t Ar type
>  .Ar file
>  .Sh DESCRIPTION
> -Maps provide a generic interface for associating textual key to a value.
> -Such associations may be accessed through a plaintext file, database, or DNS.
> +Maps provide a generic interface for associating keys with values.

i simply added "a" before "textual key" here.

> +The resultant values may be accessed through a plain text file,
> +database, or DNS.
>  The format of these file types is described below.
>  .Nm
> -itself creates the database maps used by keyed map lookups specified in
> +creates the database maps used for lookups specified in
>  .Xr smtpd.conf 5 .
>  .Pp
>  .Nm
>  reads input from
>  .Ar file
> -and writes data to a file whose name is made by adding a
> +and, by default, writes data to a file which is named by adding a

i took the "which is named" part

>  .Dq .db
>  suffix to
>  .Ar file .
> @@ -76,7 +77,7 @@ Specify the format of the resulting map 
>  The default map format is suitable for storing simple, unstructured,
>  key-to-value string associations.
>  However, if the mapped value has special meaning,
> -as in the case of the virtual domains file,
> +as in the case of a virtual domains file,

taken

>  a suitable
>  .Ar type
>  must be provided.
> @@ -94,15 +95,14 @@ This format can be used for building pri
>  .It Fl U
>  Instead of generating a database map from text input,
>  dump the contents of a database map as text
> -with the key and value separated with a tab.
> +with the key and value separated by a tab.
>  .El
>  .Sh PRIMARY DOMAINS
> -Primary domains can be kept in tables.
>  To create a primary domain table, add each primary domain on a
>  single line by itself.
>  .Pp
>  In addition to adding an entry to the primary domain map,
> -one must add a filter rule that accepts mail for the domain
> +one must add a match directive that accepts mail for the domain
>  map, for example:
>  .Bd -literal -offset indent
>  table domains db:/etc/mail/domains.db
> @@ -112,11 +112,10 @@ action "local" mbox
>  match for domain  action "local"
>  .Ed
>  .Sh VIRTUAL DOMAINS
> -Virtual domains may also be kept in tables.
>  To create a virtual domain table, add each virtual domain on a
>  single line by itself.
>  .Pp
> -Virtual domains expect a mapping of virtual users to real users
> +Virtual domains expect a mapping of virtual to real users
>  in order to determine if a recipient is accepted or not.
>  The mapping format is an extension to
>  .Xr aliases 5 ,
> @@ -130,17 +129,17 @@ to provide a catch-all for the specified
>  .Dq @
>  to provide a global catch-all for all domains.
>  .Xr smtpd 8
> -will perform the lookups in that specific order.
> +will perform lookups in that order.
>  .Pp
> -To create single virtual address, add
> +To create a single virtual address, add
>  .Dq u...@example.com user
> -to the users map.
> -To handle all mail destined to any user at example.com, add
> +to the user map.
> +To handle all mail addressed to any user at example.com, add
>  .Dq @example.com user
>  to the virtual map.
>  .Pp
>  In addition to adding an entry to the virtual map,
> -one must add a filter rule that accepts mail for virtual domains,
> +one must add a match directive that accepts mail for virtual domains,
>  for example:
>  .Bd -literal -offset indent
>  table vdomains db:/etc/mail/vdomains.db
> 

any changes not noted above were not committed (on the basis that i
cannot discern any clear improvement).

jmc



Re: video(4) multiple opens

2021-02-12 Thread Claudio Jeker
On Fri, Feb 12, 2021 at 10:59:05PM +0100, Jeremie Courreges-Anglas wrote:
> On Wed, Feb 10 2021, Martin Pieuchot  wrote:
> 
> [...]
> 
> > Which fields is the new lock protecting?  Why isn't the KERNEL_LOCK()
> > enough?
> 
> When I mentioned this potential lack of locking to Marcus, I was
> mistaken.  Some of the functions in video.c are called from syscalls
> that are marked NOLOCK.  But now that I have added some printf
> debugging, I can see that the kernel lock is held in places where
> I didn't expect it to be held (videoioctl, videoread, videoclose...).
> So there's probably a layer of locking that I am missing.
> 
> Marcus, sorry for my misleading diff. O:)
> 
> *crawls back under his rock*

Just because a syscall is marked NOLOCK does not mean that all of it will
run without the KERNEL_LOCK. For example read and write are marked NOLOCK
but the KERNEL_LOCK is grabbed later for all non-socket filedescriptors.
This is why video(4) still runs with the KERNEL_LOCK.
The NOLOCK in syscalls.master just tell the kernel to not grab the
KERNEL_LOCK right at the start of the syscall.

-- 
:wq Claudio



Re: OpenSMTPD docs: mail.maildir.8

2021-02-12 Thread Jason McIntyre
On Fri, Feb 12, 2021 at 03:16:57PM +, Larry Hynes wrote:
> 
> Index: mail.maildir.8
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/mail.maildir.8,v
> retrieving revision 1.5
> diff -u -p -r1.5 mail.maildir.8
> --- mail.maildir.830 May 2018 12:37:57 -  1.5
> +++ mail.maildir.812 Feb 2021 15:16:19 -
> @@ -36,7 +36,8 @@ located in the user's home directory.
>  The options are as follows:
>  .Bl -tag -width Ds
>  .It Fl j
> -Scan message for X-Spam and move to Junk folder if result is positive.
> +Scan message for an X-Spam header and move to Junk folder if the
> +result is positive.
>  .El
>  .Sh EXIT STATUS
>  .Ex -std mail.maildir
> 

fixed, thanks,
jmc



Re: OpenSMTPD docs: forward.5

2021-02-12 Thread Jason McIntyre
On Fri, Feb 12, 2021 at 03:15:47PM +, Larry Hynes wrote:
> 
> Index: forward.5
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/forward.5,v
> retrieving revision 1.9
> diff -u -p -r1.9 forward.5
> --- forward.5 13 Mar 2015 22:41:54 -  1.9
> +++ forward.5 12 Feb 2021 15:14:44 -
> @@ -49,10 +49,10 @@ group or world-writable;
>  if the home directory is group writeable;
>  or if the file is not owned by the user.
>  .Pp
> -Users should avoid editing directly the
> +Users should avoid editing the
>  .Nm .forward
> -file to prevent delivery failures from occurring if a message
> -arrives while the file is not fully written.
> +file directly, to prevent delivery failures from occurring if
> +a message arrives while the file is not fully written.
>  The best option is to use a temporary file and use the
>  .Xr mv 1
>  command to atomically overwrite the former
> 

fixed, thanks.
jmc



Re: OpenSMTPD docs: aliases.5

2021-02-12 Thread Jason McIntyre
On Fri, Feb 12, 2021 at 03:14:26PM +, Larry Hynes wrote:
> 

hi.

i've taken the changes in the first sentence - i agree that the comma
placement reads better in your version.

i'm working my way through your diffs just now, but to be honest most of
these changes (like the rest in this diff) look like changing words
needlessly. i don;t want to change stuff unless there seems to be a
clear improvement.

jmc

> Index: aliases.5
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/aliases.5,v
> retrieving revision 1.16
> diff -u -p -r1.16 aliases.5
> --- aliases.5 23 Apr 2020 21:28:10 -  1.16
> +++ aliases.5 12 Feb 2021 15:12:41 -
> @@ -25,19 +25,18 @@ This manual page describes the format of
>  .Nm
>  file, as used by
>  .Xr smtpd 8 .
> -An alias in its simplest form is used to assign an arbitrary name
> -to an email address, or a group of email addresses.
> -This provides a convenient way to send mail.
> -For example an alias could refer to all users of a group:
> +An alias, in its simplest form, is used to assign an arbitrary name
> +to an email address or a group of email addresses.
> +For example, an alias can refer to all users in a group:
>  email to that alias would be sent to all members of the group.
> -Much more complex aliases can be defined however:
> +More complex aliases can be defined, however:
>  an alias can refer to other aliases,
> -be used to send mail to a file instead of another person,
> +be used to write mail to a file,
>  or to execute various commands.
>  .Pp
>  Within the file,
>  .Ql #
> -is a comment delimiter; anything placed after it is discarded.
> +is a comment delimiter; anything placed after it is ignored.
>  The file consists of key/value mappings of the form:
>  .Bd -filled -offset indent
>  key: value1, value2, value3, ...
> 



Re: video(4) multiple opens

2021-02-12 Thread Jeremie Courreges-Anglas
On Wed, Feb 10 2021, Martin Pieuchot  wrote:

[...]

> Which fields is the new lock protecting?  Why isn't the KERNEL_LOCK()
> enough?

When I mentioned this potential lack of locking to Marcus, I was
mistaken.  Some of the functions in video.c are called from syscalls
that are marked NOLOCK.  But now that I have added some printf
debugging, I can see that the kernel lock is held in places where
I didn't expect it to be held (videoioctl, videoread, videoclose...).
So there's probably a layer of locking that I am missing.

Marcus, sorry for my misleading diff. O:)

*crawls back under his rock*
-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



OpenSMTPD docs: smtpd.conf.5

2021-02-12 Thread Larry Hynes


Index: smtpd.conf.5
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
retrieving revision 1.256
diff -u -p -r1.256 smtpd.conf.5
--- smtpd.conf.527 Jan 2021 14:59:10 -  1.256
+++ smtpd.conf.512 Feb 2021 15:28:04 -
@@ -184,7 +184,7 @@ Specify how long a message may remain in
 .It Cm user Ar username
 Specify the
 .Ar username
-for performing the delivery, to be looked up with
+to perform the delivery, looked up with
 .Xr getpwnam 3 .
 .Pp
 This is used for virtual hosting where a single username
@@ -202,7 +202,7 @@ function.
 .Pp
 The
 .Cm userbase
-does not apply for the
+does not apply to the
 .Cm user
 option.
 .It Cm virtual Pf < Ar table Ns >
@@ -322,13 +322,13 @@ If the list contains more than one addre
 in such a way that traffic is routed as efficiently as possible.
 .El
 .It Ic admd Ar authservid
-The Administrative Management Domain this mailserver belongs to.
+The Administrative Management Domain this mail server belongs to.
 The authservid will be forwarded to filters using it to identify or mark
 authentication-results headers.
 If omitted it defaults to the server name.
 .It Ic bounce Cm warn-interval Ar delay Op , Ar delay ...
 Send warning messages to the envelope sender when temporary delivery
-failures cause a message to remain on the queue for longer than
+failures cause a message to remain in the queue for longer than
 .Ar delay .
 Each
 .Ar delay
@@ -359,11 +359,11 @@ directive.
 .It Ic filter Ar chain-name Ic chain Brq Ar filter-name Op , Ar ...
 Register a chain of filters
 .Ar chain-name ,
-consisting of the filters listed from
+consisting of the filters listed in
 .Ar filter-name .
-Filters part of a filter chain are executed in order of declaration for
-each phase that they are registered for.
-A filter chain may be used in place of a filter for any directive but
+Filters in a filter chain are executed in order of declaration
+for each phase that they are registered for.
+A filter chain may be used in place of a filter for any directive except
 filter chains themselves.
 .It Ic filter Ar filter-name Ic phase Ar phase-name Ic match Ar conditions 
decision
 Register a filter
@@ -372,8 +372,9 @@ A
 .Ar decision
 about what to do with the mail is taken at phase
 .Ar phase-name
-when matching
-.Ar conditions .
+when
+.Ar conditions
+are matched.
 Phases, matching conditions, and decisions are described in
 .Sx MAIL FILTERING ,
 below.
@@ -387,15 +388,15 @@ backed by the
 process.
 .It Ic filter Ar filter-name Ic proc-exec Ar command
 Register and execute
-.Qq proc
+.Ar command
+as
+.Qq proc-exec
 filter
-.Ar filter-name
-from
-.Ar command .
+.Ar filter-name .
 If
 .Ar command
 starts with a slash it is executed with an absolute path,
-else it will be run from
+otherwise it will be run from
 .Dq /usr/local/libexec/smtpd/ .
 .It Ic include Qq Ar pathname
 Replace this directive with the content of the additional configuration
@@ -404,7 +405,7 @@ file at the absolute
 .It Ic listen on Ar interface Oo Ar family Oc Op Ar options
 Listen on the
 .Ar interface
-for incoming connections, using the same syntax as for
+for incoming connections, using the same syntax as
 .Xr ifconfig 8 .
 The
 .Ar interface
@@ -461,7 +462,7 @@ Override the server name for specific ad
 The
 .Ar names
 table contains a mapping of IP addresses to hostnames.
-If the address on which the connection arrives appears in the mapping,
+If the address on which the connection is made appears in the mapping,
 the associated hostname is used.
 .It Cm mask-src
 Omit the
@@ -485,7 +486,7 @@ Listen on the given
 instead of the default port 25.
 .It Cm proxy-v2
 Support the PROXYv2 protocol,
-rewriting appropriately source address received from proxy.
+appropriately rewriting the source address passed from the proxy.
 .It Cm received-auth
 In
 .Dq Received
@@ -555,7 +556,8 @@ matching envelope, and atomically save t
 spool for later processing by the respective dispatcher
 .Ar name .
 .Pp
-The following matching options are supported and can all be negated:
+The following session matching options are supported and can all
+be negated:
 .Bl -tag -width Ds
 .It Xo
 .Op Ic \&!
@@ -691,7 +693,7 @@ Specify that session may only originate 
 which can be a specific address or a subnet expressed in CIDR-notation.
 .El
 .Pp
-In addition, the following transaction options:
+In addition, the following transaction options may be matched:
 .Bl -tag -width Ds
 .It Xo
 .Op Ic \&!
@@ -731,14 +733,14 @@ Specify that session's HELO / EHLO shoul
 .Cm mail-from
 .Ar sender | Pf < Ar sender Ns >
 .Xc
-Specify that transactions's MAIL FROM should match the string or list table
+Specify that transaction's MAIL FROM should match the string or list table
 .Ar sender .
 .It Xo
 .Op Ic \&!
 .Cm mail-from regex
 .Ar sender | Pf < Ar sender Ns >
 .Xc
-Specify that transactions's MAIL FROM should match the regex or regex table
+Specify that transaction's MAIL FROM 

OpenSMTPD docs: table.5

2021-02-12 Thread Larry Hynes


Index: table.5
===
RCS file: /cvs/src/usr.sbin/smtpd/table.5,v
retrieving revision 1.11
diff -u -p -r1.11 table.5
--- table.5 11 Aug 2019 13:00:57 -  1.11
+++ table.5 12 Feb 2021 15:29:25 -
@@ -52,7 +52,7 @@ value3
 .Ed
 .Pp
 A mapping will be written with each key and value on a line,
-whitespaces separating both columns:
+whitespace separating both columns:
 .Bd -literal -offset indent
 key1   value1
 key2   value2
@@ -90,7 +90,7 @@ user3 otheru...@example.com
 .Ed
 .Pp
 In a virtual domain context, the key is either a user part, a full email
-address or a catch all, following selection rules described in
+address or a catch-all, following selection rules described in
 .Xr smtpd.conf 5 ,
 and the value is one or many recipients as described in
 .Xr aliases 5 :
@@ -164,7 +164,7 @@ They can only be used in the following c
 When used as a "from source", the address of a client is compared to the list
 of addresses in the table until a match is found.
 .Pp
-A netaddr table can contain exact addresses or netmasks, and looks as follow:
+A netaddr table can contain exact addresses or netmasks, as follows:
 .Bd -literal -offset indent
 192.168.1.1
 ::1
@@ -172,7 +172,7 @@ ipv6:::1
 192.168.1.0/24
 .Ed
 .Ss Userinfo tables
-User info tables are used in rule context to specify an alternate user base,
+Userinfo tables are used in a rule context to specify an alternate userbase,
 mapping virtual users to local system users by UID, GID and home directory.
 .Pp
 .D1 Ic action Ar name method Cm userbase Pf < Ar table Ns >
@@ -234,15 +234,15 @@ user@*.domain
 .Ed
 .Ss Addrname tables
 Addrname tables are used to map IP addresses to hostnames.
-They can be used in both listen context and relay context:
+They can be used in both listen and relay contexts:
 .Bd -unfilled -offset indent
 .Ic listen on Ar interface Cm hostnames Pf < Ar table Ns >
 .Ic action Ar name Cm relay helo\-src Pf < Ar table Ns >
 .Ed
 .Pp
-In listen context, the table is used to look up the server name to advertise
+In a listen context, the table is used to look up the server name to advertise
 depending on the local address of the socket on which a connection is accepted.
-In relay context, the table is used to determine the hostname for the HELO
+In a relay context, the table is used to determine the hostname for the HELO
 sequence of the SMTP protocol, depending on the local address used for the
 outgoing connection.
 .Pp



OpenSMTPD docs: smtp.1

2021-02-12 Thread Larry Hynes
Note: the command 'smtp -h' only returns usage. Usage exits with '1'
(I assume this is to satisfy the default case in switch (ch) in
main()), even when called from the correct use of the '-h' flag,
which I don't think is correct.

Index: smtp.1
===
RCS file: /cvs/src/usr.sbin/smtpd/smtp.1,v
retrieving revision 1.8
diff -u -p -r1.8 smtp.1
--- smtp.1  21 Dec 2020 11:48:38 -  1.8
+++ smtp.1  12 Feb 2021 17:30:55 -
@@ -58,7 +58,7 @@ Default to the current username.
 .It Fl H Ar helo
 Define the hostname to advertise (HELO) when establishing the SMTP session.
 .It Fl h
-Display version and usage.
+Display usage.
 .It Fl n
 Do not actually execute a transaction,
 just try to establish an SMTP session and quit.



OpenSMTPD docs: smtpctl.8

2021-02-12 Thread Larry Hynes


Index: smtpctl.8
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpctl.8,v
retrieving revision 1.65
diff -u -p -r1.65 smtpctl.8
--- smtpctl.8   14 Sep 2020 09:48:08 -  1.65
+++ smtpctl.8   12 Feb 2021 15:22:58 -
@@ -172,8 +172,8 @@ Status of last delivery.
 .It Cm show message Ar envelope-id
 Display message content for the given ID.
 .It Cm show queue
-Display information concerning envelopes that are currently in the queue.
-Each line of output describes a single envelope.
+Display information about envelopes that are currently in the queue.
+Each line of output relates to a single envelope.
 It consists of the following fields, separated by a "|":
 .Pp
 .Bl -bullet -compact
@@ -239,7 +239,7 @@ The route has a timeout registered to lo
 reactivate or discard it.
 .El
 .It Cm show stats
-Displays runtime statistics concerning
+Displays runtime statistics for
 .Xr smtpd 8 .
 .It Cm show status
 Shows if MTA, MDA and SMTP systems are currently running or paused.



OpenSMTPD docs: makemap.8

2021-02-12 Thread Larry Hynes
Note: DNS is mentioned in this file as a way of accessing maps. Aside
from this mention it appears to be undocumented.

Index: makemap.8
===
RCS file: /cvs/src/usr.sbin/smtpd/makemap.8,v
retrieving revision 1.30
diff -u -p -r1.30 makemap.8
--- makemap.8   25 Nov 2018 14:41:16 -  1.30
+++ makemap.8   12 Feb 2021 15:17:42 -
@@ -29,17 +29,18 @@
 .Op Fl t Ar type
 .Ar file
 .Sh DESCRIPTION
-Maps provide a generic interface for associating textual key to a value.
-Such associations may be accessed through a plaintext file, database, or DNS.
+Maps provide a generic interface for associating keys with values.
+The resultant values may be accessed through a plain text file,
+database, or DNS.
 The format of these file types is described below.
 .Nm
-itself creates the database maps used by keyed map lookups specified in
+creates the database maps used for lookups specified in
 .Xr smtpd.conf 5 .
 .Pp
 .Nm
 reads input from
 .Ar file
-and writes data to a file whose name is made by adding a
+and, by default, writes data to a file which is named by adding a
 .Dq .db
 suffix to
 .Ar file .
@@ -76,7 +77,7 @@ Specify the format of the resulting map 
 The default map format is suitable for storing simple, unstructured,
 key-to-value string associations.
 However, if the mapped value has special meaning,
-as in the case of the virtual domains file,
+as in the case of a virtual domains file,
 a suitable
 .Ar type
 must be provided.
@@ -94,15 +95,14 @@ This format can be used for building pri
 .It Fl U
 Instead of generating a database map from text input,
 dump the contents of a database map as text
-with the key and value separated with a tab.
+with the key and value separated by a tab.
 .El
 .Sh PRIMARY DOMAINS
-Primary domains can be kept in tables.
 To create a primary domain table, add each primary domain on a
 single line by itself.
 .Pp
 In addition to adding an entry to the primary domain map,
-one must add a filter rule that accepts mail for the domain
+one must add a match directive that accepts mail for the domain
 map, for example:
 .Bd -literal -offset indent
 table domains db:/etc/mail/domains.db
@@ -112,11 +112,10 @@ action "local" mbox
 match for domain  action "local"
 .Ed
 .Sh VIRTUAL DOMAINS
-Virtual domains may also be kept in tables.
 To create a virtual domain table, add each virtual domain on a
 single line by itself.
 .Pp
-Virtual domains expect a mapping of virtual users to real users
+Virtual domains expect a mapping of virtual to real users
 in order to determine if a recipient is accepted or not.
 The mapping format is an extension to
 .Xr aliases 5 ,
@@ -130,17 +129,17 @@ to provide a catch-all for the specified
 .Dq @
 to provide a global catch-all for all domains.
 .Xr smtpd 8
-will perform the lookups in that specific order.
+will perform lookups in that order.
 .Pp
-To create single virtual address, add
+To create a single virtual address, add
 .Dq u...@example.com user
-to the users map.
-To handle all mail destined to any user at example.com, add
+to the user map.
+To handle all mail addressed to any user at example.com, add
 .Dq @example.com user
 to the virtual map.
 .Pp
 In addition to adding an entry to the virtual map,
-one must add a filter rule that accepts mail for virtual domains,
+one must add a match directive that accepts mail for virtual domains,
 for example:
 .Bd -literal -offset indent
 table vdomains db:/etc/mail/vdomains.db



OpenSMTPD docs: newaliases.8

2021-02-12 Thread Larry Hynes


Index: newaliases.8
===
RCS file: /cvs/src/usr.sbin/smtpd/newaliases.8,v
retrieving revision 1.12
diff -u -p -r1.12 newaliases.8
--- newaliases.820 Jul 2018 15:35:33 -  1.12
+++ newaliases.812 Feb 2021 15:21:00 -
@@ -27,7 +27,7 @@
 .Sh DESCRIPTION
 The
 .Nm
-utility makes changes to the mail aliases file visible to
+utility updates a mail aliases file in use by
 .Xr smtpd 8 .
 It should be run every time the
 .Xr aliases 5



OpenSMTPD docs: mail.maildir.8

2021-02-12 Thread Larry Hynes


Index: mail.maildir.8
===
RCS file: /cvs/src/usr.sbin/smtpd/mail.maildir.8,v
retrieving revision 1.5
diff -u -p -r1.5 mail.maildir.8
--- mail.maildir.8  30 May 2018 12:37:57 -  1.5
+++ mail.maildir.8  12 Feb 2021 15:16:19 -
@@ -36,7 +36,8 @@ located in the user's home directory.
 The options are as follows:
 .Bl -tag -width Ds
 .It Fl j
-Scan message for X-Spam and move to Junk folder if result is positive.
+Scan message for an X-Spam header and move to Junk folder if the
+result is positive.
 .El
 .Sh EXIT STATUS
 .Ex -std mail.maildir



OpenSMTPD docs: forward.5

2021-02-12 Thread Larry Hynes


Index: forward.5
===
RCS file: /cvs/src/usr.sbin/smtpd/forward.5,v
retrieving revision 1.9
diff -u -p -r1.9 forward.5
--- forward.5   13 Mar 2015 22:41:54 -  1.9
+++ forward.5   12 Feb 2021 15:14:44 -
@@ -49,10 +49,10 @@ group or world-writable;
 if the home directory is group writeable;
 or if the file is not owned by the user.
 .Pp
-Users should avoid editing directly the
+Users should avoid editing the
 .Nm .forward
-file to prevent delivery failures from occurring if a message
-arrives while the file is not fully written.
+file directly, to prevent delivery failures from occurring if
+a message arrives while the file is not fully written.
 The best option is to use a temporary file and use the
 .Xr mv 1
 command to atomically overwrite the former



OpenSMTPD docs: aliases.5

2021-02-12 Thread Larry Hynes


Index: aliases.5
===
RCS file: /cvs/src/usr.sbin/smtpd/aliases.5,v
retrieving revision 1.16
diff -u -p -r1.16 aliases.5
--- aliases.5   23 Apr 2020 21:28:10 -  1.16
+++ aliases.5   12 Feb 2021 15:12:41 -
@@ -25,19 +25,18 @@ This manual page describes the format of
 .Nm
 file, as used by
 .Xr smtpd 8 .
-An alias in its simplest form is used to assign an arbitrary name
-to an email address, or a group of email addresses.
-This provides a convenient way to send mail.
-For example an alias could refer to all users of a group:
+An alias, in its simplest form, is used to assign an arbitrary name
+to an email address or a group of email addresses.
+For example, an alias can refer to all users in a group:
 email to that alias would be sent to all members of the group.
-Much more complex aliases can be defined however:
+More complex aliases can be defined, however:
 an alias can refer to other aliases,
-be used to send mail to a file instead of another person,
+be used to write mail to a file,
 or to execute various commands.
 .Pp
 Within the file,
 .Ql #
-is a comment delimiter; anything placed after it is discarded.
+is a comment delimiter; anything placed after it is ignored.
 The file consists of key/value mappings of the form:
 .Bd -filled -offset indent
 key: value1, value2, value3, ...



Re: random manual pages

2021-02-12 Thread Theo de Raadt
Nelson H. F. Beebe  wrote:

> Thanks for the comments, Theo.
> 
> I wasn't clear in my posting about distinguishing deterministic from
> nondeterministic generators.  The former are required for reproducible
> simulations

Perhaps.  Even then they often criminally abused, ie. Neil Ferguson --
sometimes the final results become decoupled from the inputs and become
biased towards the seed/sequence.

> the latter are needed for things like cryptographic key
> generation.

I disagree.

Outside of the deterministic cases, *all* other things should be strong
random.  There should be no such thing as "weak random".  Tying this to
"for cryptographic keys" is a mistake.

Every DNS packet contains a 16-bit ID.  Should it use weak random, since
it is not cryptographic key generation?



Re: random manual pages

2021-02-12 Thread Nelson H. F. Beebe
Thanks for the comments, Theo.

I wasn't clear in my posting about distinguishing deterministic from
nondeterministic generators.  The former are required for reproducible
simulations; the latter are needed for things like cryptographic key
generation.  Both have their place, and both are necessary.

---
- Nelson H. F. BeebeTel: +1 801 581 5254  -
- University of UtahFAX: +1 801 581 4148  -
- Department of Mathematics, 110 LCBInternet e-mail: be...@math.utah.edu  -
- 155 S 1400 E RM 233   be...@acm.org  be...@computer.org -
- Salt Lake City, UT 84112-0090, USAURL: http://www.math.utah.edu/~beebe/ -
---



Re: random manual pages

2021-02-12 Thread Theo de Raadt
Hi Nelson -


Nelson H. F. Beebe  wrote:

> Thanks, Theo, for this good advice about random-number generators:

In this discussion, I made no comments about "random-number generators".

Instead I commented on "deterministic generators", which have been
incorrectly placed inside subsystems claiming to be random, but which
are not random at all.  I am continuing my efforts to discourage use of
such generator throughout the entire software ecosystem.

My past advice about random-number generators can found here:

https://www.openbsd.org/papers/hackfest2014-arc4random/index.html

> >> - if you need determinism, write your own.
> >> - do not rely upon an external function which will make your seed
> >>   produce a different result approximately every 8 years.
> 
> I would add more points: 
> 
> * You MUST have portable source code for your generator (but you
>   probably don't need to, or should, write it yourself, unless
>   you are an expert in the field).

I have no interest in guiding programmers towards domain-specific
deterministic functions they may need for their shaders, Covid pandemic
bullshit simulations, or whatever.

> * If reproducibility of simulations matters (as it often does), set
>   the seed explicitly from your program source code or your input,
>   then print in your program output the first 50 to 100 random numbers
>   produced by your generator.  A similar list from a later run can
>   then be compared with the earlier one to verify that both use the
>   same sequence.

Wow, look what you just did.  You said a reproducible generator produces
random results.

Something cannot be random and reproduceable.  To be reproduceable, it
must be deterministic.  Deterministic is a term meaning the opposite of
random.

The very first sentence of "deterministic" in wikipedia hits the nail on
the head:

In mathematics, computer science and physics, a deterministic system is
a system in which no randomness is involved in the development of future
states of the system.[1]

I am here to slowly destroy the academic culture of blending the two
concepts, because it has misled two generations of programmers to
accidentally misuse deterministic routines where they are unsuitable,
because randomness is what they actually desire.

> * Avoid generators that use floating-point arithmetic: they are
>   difficult, perhaps impossible, to make platform independent.

Even the existing integer ones (rand.3 and random.3 ) were platform
dependent, since they perform wrapping operations on a mix of 64-bit and
32-bit types.

> * Beware of old algorithms: there have been huge improvements in
>   random-number generators in the last two to three decades

None of those things are _random_ number generators.

You are clearly part of the problem!  It seems there is an academic
industry that cannot use words correctly.

> See the preamble comments in
> 
>   http://www.math.utah.edu/pub/tex/bib/prng.bib
>   http://www.math.utah.edu/pub/tex/bib/prng.html
> 
> for pointers to papers that show the extreme subtlety of long-range
> correlations in many generators that can derail simulations.

I see studies of determinism, constantly abusing the word "random".

Generations of programmers have skipped use of proper randomization
because the conversation is muddled by this garbage.  Far less than 1%
of algorithms need deterministic sequences, but it has poisoned the well
for the other >99% of use cases.




Re: random manual pages

2021-02-12 Thread Nelson H. F. Beebe
Thanks, Theo, for this good advice about random-number generators:

>> - if you need determinism, write your own.
>> - do not rely upon an external function which will make your seed
>>   produce a different result approximately every 8 years.

I would add more points: 

* You MUST have portable source code for your generator (but you
  probably don't need to, or should, write it yourself, unless
  you are an expert in the field).

* If reproducibility of simulations matters (as it often does), set
  the seed explicitly from your program source code or your input,
  then print in your program output the first 50 to 100 random numbers
  produced by your generator.  A similar list from a later run can
  then be compared with the earlier one to verify that both use the
  same sequence.

* Avoid generators that use floating-point arithmetic: they are
  difficult, perhaps impossible, to make platform independent.

* Beware of old algorithms: there have been huge improvements in
  random-number generators in the last two to three decades

See the preamble comments in

http://www.math.utah.edu/pub/tex/bib/prng.bib
http://www.math.utah.edu/pub/tex/bib/prng.html

for pointers to papers that show the extreme subtlety of long-range
correlations in many generators that can derail simulations.

There are more recommendations, and programming subtleties, discussed
in chapter 7 of my book, The Mathematical-Function Computation
Handbook.  However, the extensive prng.bib file above can lead you to
the original research papers, if it matters to you.

---
- Nelson H. F. BeebeTel: +1 801 581 5254  -
- University of UtahFAX: +1 801 581 4148  -
- Department of Mathematics, 110 LCBInternet e-mail: be...@math.utah.edu  -
- 155 S 1400 E RM 233   be...@acm.org  be...@computer.org -
- Salt Lake City, UT 84112-0090, USAURL: http://www.math.utah.edu/~beebe/ -
---



Re: malloc junking tweaks

2021-02-12 Thread Otto Moerbeek
On Fri, Feb 12, 2021 at 02:18:08PM +0100, Otto Moerbeek wrote:

> Hi,
> 
> Curently, junking is done like this:
> 
> - for small chunk, the whole chunk junked on free
> 
> - for pages sized and larger, the first half a page is filled
> 
> - after a delayed free, the first 32 bytes of small chunks are
> validated to not be overwritten
> 
> - page sized and larger allocations are not validated at all, even if
> they end up in the cache.
> 
> This diff changes the following:
> 
> - I make use of the fact that I know how the chunks are aligned, and
> write 8 bytes at the time by using a uint64_t pointer. For an
> allocation a max of 4 such uint64_t's are written spread over the
> allocation. For pages sized and larger, the first page is junked in
> such a way.
> 
> - Delayed free of a small chunk checks the corresponiding way.
> 
> - Pages ending up in the cache are validated upon unmapping or re-use.
> 
> The last point is the real gain: we also check for write-after-free
> for large allocations, which we did not do before.
> 
> So we are catching more writes-after-frees. A price to pay is that
> larger chunks are not completely junked, but only a total of 32 bytes
> are. I chose this number after comparing performance with the current
> code: we still gain a bit in speed.
> 
> Junk mode 0 (j) and junk mode 2 (J) are not changed.
> 
> Please test and review,
> 
>   -Otto
> 

And now with correct version of diff

-Otto


Index: stdlib/malloc.3
===
RCS file: /cvs/src/lib/libc/stdlib/malloc.3,v
retrieving revision 1.126
diff -u -p -r1.126 malloc.3
--- stdlib/malloc.3 14 Sep 2019 13:16:50 -  1.126
+++ stdlib/malloc.3 12 Feb 2021 08:14:54 -
@@ -619,7 +619,7 @@ or
 reallocate an unallocated pointer was made.
 .It Dq chunk is already free
 There was an attempt to free a chunk that had already been freed.
-.It Dq use after free
+.It Dq write after free
 A chunk has been modified after it was freed.
 .It Dq modified chunk-pointer
 The pointer passed to
Index: stdlib/malloc.c
===
RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.267
diff -u -p -r1.267 malloc.c
--- stdlib/malloc.c 23 Nov 2020 15:42:11 -  1.267
+++ stdlib/malloc.c 12 Feb 2021 08:14:54 -
@@ -89,6 +89,7 @@
  */
 #define SOME_JUNK  0xdb/* deadbeef */
 #define SOME_FREEJUNK  0xdf/* dead, free */
+#define SOME_FREEJUNK_ULL  0xdfdfdfdfdfdfdfdfULL
 
 #define MMAP(sz,f) mmap(NULL, (sz), PROT_READ | PROT_WRITE, \
 MAP_ANON | MAP_PRIVATE | (f), -1, 0)
@@ -655,6 +656,49 @@ delete(struct dir_info *d, struct region
}
 }
 
+static inline void
+junk_free(int junk, void *p, size_t sz)
+{
+   size_t i, step = 1;
+   uint64_t *lp = p;
+
+   if (junk == 0 || sz == 0)
+   return;
+   sz /= sizeof(uint64_t);
+   if (junk == 1) {
+   if (sz > MALLOC_PAGESIZE / sizeof(uint64_t))
+   sz = MALLOC_PAGESIZE / sizeof(uint64_t);
+   step = sz / 4;
+   if (step == 0)
+   step = 1;
+   }
+   for (i = 0; i < sz; i += step)
+   lp[i] = SOME_FREEJUNK_ULL;
+}
+
+static inline void
+validate_junk(struct dir_info *pool, void *p, size_t sz)
+{
+   size_t i, step = 1;
+   uint64_t *lp = p;
+
+   if (pool->malloc_junk == 0 || sz == 0)
+   return;
+   sz /= sizeof(uint64_t);
+   if (pool->malloc_junk == 1) {
+   if (sz > MALLOC_PAGESIZE / sizeof(uint64_t))
+   sz = MALLOC_PAGESIZE / sizeof(uint64_t);
+   step = sz / 4;
+   if (step == 0)
+   step = 1;
+   }
+   for (i = 0; i < sz; i += step) {
+   if (lp[i] != SOME_FREEJUNK_ULL)
+   wrterror(pool, "write after free %p", p);
+   }
+}
+
+
 /*
  * Cache maintenance. We keep at most malloc_cache pages cached.
  * If the cache is becoming full, unmap pages in the cache for real,
@@ -663,7 +707,7 @@ delete(struct dir_info *d, struct region
  * cache are in MALLOC_PAGESIZE units.
  */
 static void
-unmap(struct dir_info *d, void *p, size_t sz, size_t clear, int junk)
+unmap(struct dir_info *d, void *p, size_t sz, size_t clear)
 {
size_t psz = sz >> MALLOC_PAGESHIFT;
size_t rsz;
@@ -695,6 +739,8 @@ unmap(struct dir_info *d, void *p, size_
r = >free_regions[(i + offset) & mask];
if (r->p != NULL) {
rsz = r->size << MALLOC_PAGESHIFT;
+   if (!mopts.malloc_freeunmap)
+   validate_junk(d, r->p, rsz);
if (munmap(r->p, rsz))
wrterror(d, "munmap %p", r->p);
r->p = NULL;
@@ -716,12 

malloc junking tweaks

2021-02-12 Thread Otto Moerbeek
Hi,

Curently, junking is done like this:

- for small chunk, the whole chunk junked on free

- for pages sized and larger, the first half a page is filled

- after a delayed free, the first 32 bytes of small chunks are
validated to not be overwritten

- page sized and larger allocations are not validated at all, even if
they end up in the cache.

This diff changes the following:

- I make use of the fact that I know how the chunks are aligned, and
write 8 bytes at the time by using a uint64_t pointer. For an
allocation a max of 4 such uint64_t's are written spread over the
allocation. For pages sized and larger, the first page is junked in
such a way.

- Delayed free of a small chunk checks the corresponiding way.

- Pages ending up in the cache are validated upon unmapping or re-use.

The last point is the real gain: we also check for write-after-free
for large allocations, which we did not do before.

So we are catching more writes-after-frees. A price to pay is that
larger chunks are not completely junked, but only a total of 32 bytes
are. I chose this number after comparing performance with the current
code: we still gain a bit in speed.

Junk mode 0 (j) and junk mode 2 (J) are not changed.

Please test and review,

-Otto

Index: stdlib/malloc.c
===
RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.267
diff -u -p -r1.267 malloc.c
--- stdlib/malloc.c 23 Nov 2020 15:42:11 -  1.267
+++ stdlib/malloc.c 6 Feb 2021 20:03:49 -
@@ -89,6 +89,7 @@
  */
 #define SOME_JUNK  0xdb/* deadbeef */
 #define SOME_FREEJUNK  0xdf/* dead, free */
+#define SOME_FREEJUNK_ULL  0xdfdfdfdfdfdfdfdfULL
 
 #define MMAP(sz,f) mmap(NULL, (sz), PROT_READ | PROT_WRITE, \
 MAP_ANON | MAP_PRIVATE | (f), -1, 0)
@@ -655,6 +656,46 @@ delete(struct dir_info *d, struct region
}
 }
 
+static inline void
+junkfree(int junk, void *p, size_t sz)
+{
+   size_t byte, step = 1;
+
+   if (junk == 0 || sz == 0)
+   return;
+   if (junk == 1) {
+   if (sz > MALLOC_PAGESIZE)
+   sz = MALLOC_PAGESIZE;
+   step = sz / 32;
+   if (step == 0)
+   step = 1;
+   }
+   for (byte = 0; byte < sz; byte++)
+   ((unsigned char *)p)[byte] = SOME_FREEJUNK;
+}
+
+static inline void
+validate_junk(struct dir_info *pool, void *p, size_t sz)
+{
+   size_t byte, step = 1;
+
+   if (pool->malloc_junk == 0 || sz == 0)
+   return;
+   if (pool->malloc_junk == 1) {
+   if (sz > MALLOC_PAGESIZE)
+   sz = MALLOC_PAGESIZE;
+   step = sz / 32;
+   if (step == 0)
+   step = 1;
+   }
+   for (byte = 0; byte < sz; byte += step) {
+   if (((unsigned char *)p)[byte] != SOME_FREEJUNK)
+   wrterror(pool, "write after free %p %#zx@%#zx", p,
+   byte, sz);
+   }
+}
+
+
 /*
  * Cache maintenance. We keep at most malloc_cache pages cached.
  * If the cache is becoming full, unmap pages in the cache for real,
@@ -663,7 +704,7 @@ delete(struct dir_info *d, struct region
  * cache are in MALLOC_PAGESIZE units.
  */
 static void
-unmap(struct dir_info *d, void *p, size_t sz, size_t clear, int junk)
+unmap(struct dir_info *d, void *p, size_t sz, size_t clear)
 {
size_t psz = sz >> MALLOC_PAGESHIFT;
size_t rsz;
@@ -716,12 +757,10 @@ unmap(struct dir_info *d, void *p, size_
if (r->p == NULL) {
if (clear > 0)
memset(p, 0, clear);
-   if (junk && !mopts.malloc_freeunmap) {
-   size_t amt = junk == 1 ?  MALLOC_MAXCHUNK : sz;
-   memset(p, SOME_FREEJUNK, amt);
-   }
if (mopts.malloc_freeunmap)
mprotect(p, sz, PROT_NONE);
+   else
+   junkfree(d->malloc_junk, p, psz << 
MALLOC_PAGESHIFT);
r->p = p;
r->size = psz;
d->free_regions_size += psz;
@@ -760,15 +799,16 @@ map(struct dir_info *d, size_t sz, int z
if (r->p != NULL) {
if (r->size == psz) {
p = r->p;
+   if (!mopts.malloc_freeunmap)
+   validate_junk(d, p, psz << 
MALLOC_PAGESHIFT);
r->p = NULL;
d->free_regions_size -= psz;
if (mopts.malloc_freeunmap)
mprotect(p, sz, PROT_READ | PROT_WRITE);
if (zero_fill)
memset(p, 0, 

Re: Possible null deref on pf.c

2021-02-12 Thread Claudio Jeker
On Fri, Feb 12, 2021 at 01:20:01PM +0100, Alexander Bluhm wrote:
> On Fri, Feb 12, 2021 at 01:11:24PM +0100, Claudio Jeker wrote:
> > On Fri, Feb 12, 2021 at 12:03:49PM +, Ricardo Mestre wrote:
> > > This was reported on CID 1501718, ifp starts as NULL and then might be 
> > > deref'ed.
> 
> 
> > This code is strange, the scope for the IPv6 address needs to be pulled
> > out of s (pf_state) somehow. Also is the state using embedded or
> > not-embedded scope addresses?
> 
> I was already discussung the issue with dlg@
> 
> We both think that the code is not necessary.  The address comes
> from pf configuration.  pf does nor work correctly with IPv6
> link-local anyway.  I think the only way to fix pf with link-local,
> is to embed the scope for all addresses within pf.
> 
> Current code is broken, embeding here cannot work, pf link-local
> needs rework, remove code makes rework easier.
> 
> ok?

I came to the same conclusion. I agree that this is currently unfixable
and so removing this bit of code is correct.
OK claudio@
 
> bluhm
> 
> Index: net/pf.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
> retrieving revision 1.1108
> diff -u -p -r1.1108 pf.c
> --- net/pf.c  4 Feb 2021 00:55:41 -   1.1108
> +++ net/pf.c  12 Feb 2021 12:06:47 -
> @@ -6156,8 +6156,6 @@ pf_route6(struct pf_pdesc *pd, struct pf
>   dst->sin6_addr = s->rt_addr.v6;
>   rtableid = m0->m_pkthdr.ph_rtableid;
>  
> - if (IN6_IS_SCOPE_EMBED(>sin6_addr))
> - dst->sin6_addr.s6_addr16[1] = htons(ifp->if_index);
>   rt = rtalloc(sin6tosa(dst), RT_RESOLVE, rtableid);
>   if (!rtisvalid(rt)) {
>   if (s->rt != PF_DUPTO) {
> 

-- 
:wq Claudio



Re: Possible null deref on pf.c

2021-02-12 Thread Alexander Bluhm
On Fri, Feb 12, 2021 at 01:11:24PM +0100, Claudio Jeker wrote:
> On Fri, Feb 12, 2021 at 12:03:49PM +, Ricardo Mestre wrote:
> > This was reported on CID 1501718, ifp starts as NULL and then might be 
> > deref'ed.


> This code is strange, the scope for the IPv6 address needs to be pulled
> out of s (pf_state) somehow. Also is the state using embedded or
> not-embedded scope addresses?

I was already discussung the issue with dlg@

We both think that the code is not necessary.  The address comes
from pf configuration.  pf does nor work correctly with IPv6
link-local anyway.  I think the only way to fix pf with link-local,
is to embed the scope for all addresses within pf.

Current code is broken, embeding here cannot work, pf link-local
needs rework, remove code makes rework easier.

ok?

bluhm

Index: net/pf.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.1108
diff -u -p -r1.1108 pf.c
--- net/pf.c4 Feb 2021 00:55:41 -   1.1108
+++ net/pf.c12 Feb 2021 12:06:47 -
@@ -6156,8 +6156,6 @@ pf_route6(struct pf_pdesc *pd, struct pf
dst->sin6_addr = s->rt_addr.v6;
rtableid = m0->m_pkthdr.ph_rtableid;
 
-   if (IN6_IS_SCOPE_EMBED(>sin6_addr))
-   dst->sin6_addr.s6_addr16[1] = htons(ifp->if_index);
rt = rtalloc(sin6tosa(dst), RT_RESOLVE, rtableid);
if (!rtisvalid(rt)) {
if (s->rt != PF_DUPTO) {



Re: Possible null deref on pf.c

2021-02-12 Thread Claudio Jeker
On Fri, Feb 12, 2021 at 12:03:49PM +, Ricardo Mestre wrote:
> Hi,
> 
> This was reported on CID 1501718, ifp starts as NULL and then might be 
> deref'ed.
> 
> The question is does the below make any sense to solve it since I don't know 
> what I'm doing? :)
> 
> What do you net gurus say?
> 
> Index: pf.c
> ===
> RCS file: /cvs/src/sys/net/pf.c,v
> retrieving revision 1.1108
> diff -u -p -u -r1.1108 pf.c
> --- pf.c  4 Feb 2021 00:55:41 -   1.1108
> +++ pf.c  12 Feb 2021 11:52:31 -
> @@ -6156,6 +6156,10 @@ pf_route6(struct pf_pdesc *pd, struct pf
>   dst->sin6_addr = s->rt_addr.v6;
>   rtableid = m0->m_pkthdr.ph_rtableid;
>  
> + ifp = if_get(rt->rt_ifidx);
> + if (ifp == NULL)
> + goto bad;
> +
>   if (IN6_IS_SCOPE_EMBED(>sin6_addr))
>   dst->sin6_addr.s6_addr16[1] = htons(ifp->if_index);
>   rt = rtalloc(sin6tosa(dst), RT_RESOLVE, rtableid);
> @@ -6168,10 +6172,6 @@ pf_route6(struct pf_pdesc *pd, struct pf
>   ip6stat_inc(ip6s_noroute);
>   goto bad;
>   }
> -
> - ifp = if_get(rt->rt_ifidx);
> - if (ifp == NULL)
> - goto bad;
>  
>   /* A locally generated packet may have invalid source address. */
>   if (IN6_IS_ADDR_LOOPBACK(>ip6_src) &&

This can't be right. rt is only valid after rtalloc() which is now called
afterwards.

This code is strange, the scope for the IPv6 address needs to be pulled
out of s (pf_state) somehow. Also is the state using embedded or
not-embedded scope addresses?

Last but not least, I hate IPv6 and their scoped addressing.
Worst feature ever.

-- 
:wq Claudio



Possible null deref on pf.c

2021-02-12 Thread Ricardo Mestre
Hi,

This was reported on CID 1501718, ifp starts as NULL and then might be deref'ed.

The question is does the below make any sense to solve it since I don't know 
what I'm doing? :)

What do you net gurus say?

Index: pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.1108
diff -u -p -u -r1.1108 pf.c
--- pf.c4 Feb 2021 00:55:41 -   1.1108
+++ pf.c12 Feb 2021 11:52:31 -
@@ -6156,6 +6156,10 @@ pf_route6(struct pf_pdesc *pd, struct pf
dst->sin6_addr = s->rt_addr.v6;
rtableid = m0->m_pkthdr.ph_rtableid;
 
+   ifp = if_get(rt->rt_ifidx);
+   if (ifp == NULL)
+   goto bad;
+
if (IN6_IS_SCOPE_EMBED(>sin6_addr))
dst->sin6_addr.s6_addr16[1] = htons(ifp->if_index);
rt = rtalloc(sin6tosa(dst), RT_RESOLVE, rtableid);
@@ -6168,10 +6172,6 @@ pf_route6(struct pf_pdesc *pd, struct pf
ip6stat_inc(ip6s_noroute);
goto bad;
}
-
-   ifp = if_get(rt->rt_ifidx);
-   if (ifp == NULL)
-   goto bad;
 
/* A locally generated packet may have invalid source address. */
if (IN6_IS_ADDR_LOOPBACK(>ip6_src) &&



Re: uhidpp(4): logitech hid++ device driver

2021-02-12 Thread Anindya Mukherjee
Hi,

Sorry for the delay. I am running the latest snapshot:
kern.version=OpenBSD 6.9-beta (GENERIC.MP) #331: Thu Feb 11 20:28:45 MST 2021
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
which seems to have the latest updates. However I still do not see battery
levels for my M570. Full dmesg below. Please let me know if I can test anything.

Regards,
Anindya

OpenBSD 6.9-beta (GENERIC.MP) #331: Thu Feb 11 20:28:45 MST 2021
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 34201006080 (32616MB)
avail mem = 33149145088 (31613MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.0 @ 0xe (94 entries)
bios0: vendor Dell Inc. version "1.5.6" date 12/07/2016
bios0: Dell Inc. OptiPlex 7040
acpi0 at bios0: ACPI 5.0
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP APIC FPDT FIDT MCFG HPET SSDT SSDT UEFI LPIT SSDT SSDT 
SSDT SSDT DBGP DBG2 SSDT SSDT MSDM SLIC DMAR TPM2 ASF! BGRT
acpi0: wakeup devices PEG0(S4) PEGP(S4) PEG1(S4) PEGP(S4) PEG2(S4) PEGP(S4) 
PS2K(S3) PS2M(S3) RP09(S4) PXSX(S4) RP10(S4) PXSX(S4) RP11(S4) PXSX(S4) 
RP12(S4) PXSX(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz, 3393.29 MHz, 06-5e-03
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SGX,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,SRBDS_CTRL,MD_CLEAR,TSXFA,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN
cpu0: 256KB 64b/line 8-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 24MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4.1, IBE
cpu1 at mainbus0: apid 2 (application processor)
cpu1: Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz, 3392.11 MHz, 06-5e-03
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SGX,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,SRBDS_CTRL,MD_CLEAR,TSXFA,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN
cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 0, core 1, package 0
cpu2 at mainbus0: apid 4 (application processor)
cpu2: Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz, 3392.11 MHz, 06-5e-03
cpu2: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SGX,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,SRBDS_CTRL,MD_CLEAR,TSXFA,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN
cpu2: 256KB 64b/line 8-way L2 cache
cpu2: smt 0, core 2, package 0
cpu3 at mainbus0: apid 6 (application processor)
cpu3: Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz, 3392.11 MHz, 06-5e-03
cpu3: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SGX,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,SRBDS_CTRL,MD_CLEAR,TSXFA,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN
cpu3: 256KB 64b/line 8-way L2 cache
cpu3: smt 0, core 3, package 0
cpu4 at mainbus0: apid 1 (application processor)
cpu4: Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz, 3392.10 MHz, 06-5e-03
cpu4: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SGX,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,SRBDS_CTRL,MD_CLEAR,TSXFA,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN
cpu4: 256KB 64b/line 8-way L2 cache

Re: snmpd: Add end of sequence tests

2021-02-12 Thread Martijn van Duren
On Fri, 2021-02-12 at 11:02 +0100, Claudio Jeker wrote:
> On Fri, Feb 12, 2021 at 10:03:21AM +0100, Martijn van Duren wrote:
> > ping
> > 
> > On Sun, 2021-01-31 at 11:57 +0100, Martijn van Duren wrote:
> > > Now that ober_scanf_elements supports '$' lets use it.
> > > 
> > > Here's a first stab by adding it to snmpd.
> > > Passing regress and a few manual checks.
> > > 
> > > 'e' still doesn't consume the element, but I've talked it over with
> > > rob@, who said that shouldn't get in the way of using this new feature.
> > > 
> > > OK?
> 
> Looks reasonable and I guess you verified the layout of all those ASN.1
> messages to ensure the $ is at the right place.

To the best of my knowledge in both reading and testing.
If I somehow did screw it up, it should be easy enough to fix and loud
enough to notice.
> 
> Side note: I wonder why does } not imply the $? At least now with S it
> would be possible to enforce this.

} does nothing more then dropping down from the current sequence level.
This comes in handy if for instance when parsing a PDU. The varbindlist
is of undefined length, so I wouldn't know how many Ss I'd need. See line
snmpe.c:380.
The parsing and verifying of the varbindlist happens on snmpe.c:439.

> I like that you closed a lot of open { format strings. What about these:
> 
> > > -   if (ober_scanf_elements(usm, "{xiixpxx", , ,
> > > +   if (ober_scanf_elements(usm, "{xiixpxx$", , ,
> 
> Wouldn't it be better to use "{xiixpxx$}" here?
> 
I treat } as dropping down from the current sequence level. In that case
it doesn't matter. It might be more estatically pleasing. I can add it if
you want, but doesn't add anything.

Updated diff, since I just now noticed how mangled that thing was.

Index: snmpe.c
===
RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
retrieving revision 1.69
diff -u -p -r1.69 snmpe.c
--- snmpe.c 5 Feb 2021 10:30:45 -   1.69
+++ snmpe.c 12 Feb 2021 10:34:32 -
@@ -227,7 +227,7 @@ snmpe_parse(struct snmp_message *msg)
case SNMP_V2:
if (env->sc_min_seclevel != 0)
goto badversion;
-   if (ober_scanf_elements(a, "se", , >sm_pdu) != 0)
+   if (ober_scanf_elements(a, "seS$", , >sm_pdu) != 0)
goto parsefail;
if (strlcpy(msg->sm_community, comn,
sizeof(msg->sm_community)) >= sizeof(msg->sm_community)) {
@@ -237,7 +237,7 @@ snmpe_parse(struct snmp_message *msg)
}
break;
case SNMP_V3:
-   if (ober_scanf_elements(a, "{iisi}e",
+   if (ober_scanf_elements(a, "{iisi$}e",
>sm_msgid, >sm_max_msg_size, ,
>sm_secmodel, ) != 0)
goto parsefail;
@@ -255,7 +255,7 @@ snmpe_parse(struct snmp_message *msg)
goto parsefail;
}
 
-   if (ober_scanf_elements(a, "{xxe",
+   if (ober_scanf_elements(a, "{xxeS$}$",
>sm_ctxengineid, >sm_ctxengineid_len,
, , >sm_pdu) != 0)
goto parsefail;
@@ -377,7 +377,7 @@ snmpe_parse(struct snmp_message *msg)
}
 
/* SNMP PDU */
-   if (ober_scanf_elements(a, "iiie{et",
+   if (ober_scanf_elements(a, "iiie{et}$",
, , , >sm_pduend,
>sm_varbind, , ) != 0) {
stats->snmp_silentdrops++;
@@ -436,7 +436,7 @@ snmpe_parsevarbinds(struct snmp_message 
 
for (i = 1; varbind != NULL && i < SNMPD_MAXVARBIND;
varbind = varbind->be_next, i++) {
-   if (ober_scanf_elements(varbind, "{oe}", , ) == -1) {
+   if (ober_scanf_elements(varbind, "{oeS$}", , ) == -1) {
stats->snmp_inasnparseerrs++;
msg->sm_errstr = "invalid varbind";
goto varfail;
Index: traphandler.c
===
RCS file: /cvs/src/usr.sbin/snmpd/traphandler.c,v
retrieving revision 1.20
diff -u -p -r1.20 traphandler.c
--- traphandler.c   22 Jan 2021 06:33:27 -  1.20
+++ traphandler.c   12 Feb 2021 10:34:32 -
@@ -67,7 +67,7 @@ traphandler_parse(struct snmp_message *m
struct privsep  *ps = _env->sc_ps;
struct snmp_stats   *stats = _env->sc_stats;
struct ber   ber = {0};
-   struct ber_element  *vblist = NULL, *elm, *elm2;
+   struct ber_element  *vblist = NULL, *elm;
struct ber_oid   o1, o2, snmpTrapOIDOID;
struct ber_oid   snmpTrapOID, sysUpTimeOID;
int  sysUpTime;
@@ -82,7 +82,7 @@ traphandler_parse(struct snmp_message *m
goto done;
break;
case SNMP_C_TRAPV2:
-   if (ober_scanf_elements(msg->sm_pdu, "{SSe}", ) == -1) {
+  

Re: snmpd: Add end of sequence tests

2021-02-12 Thread Claudio Jeker
On Fri, Feb 12, 2021 at 10:03:21AM +0100, Martijn van Duren wrote:
> ping
> 
> On Sun, 2021-01-31 at 11:57 +0100, Martijn van Duren wrote:
> > Now that ober_scanf_elements supports '$' lets use it.
> > 
> > Here's a first stab by adding it to snmpd.
> > Passing regress and a few manual checks.
> > 
> > 'e' still doesn't consume the element, but I've talked it over with
> > rob@, who said that shouldn't get in the way of using this new feature.
> > 
> > OK?

Looks reasonable and I guess you verified the layout of all those ASN.1
messages to ensure the $ is at the right place.

Side note: I wonder why does } not imply the $? At least now with S it
would be possible to enforce this.
I like that you closed a lot of open { format strings. What about these:

> > -   if (ober_scanf_elements(usm, "{xiixpxx", , ,
> > +   if (ober_scanf_elements(usm, "{xiixpxx$", , ,

Wouldn't it be better to use "{xiixpxx$}" here?

> > Index: snmpe.c
> > ===
> > RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
> > retrieving revision 1.68
> > diff -u -p -r1.68 snmpe.c
> > --- snmpe.c 22 Jan 2021 06:33:27 -  1.68
> > +++ snmpe.c 31 Jan 2021 10:55:49 -
> > @@ -220,7 +220,7 @@ snmpe_parse(struct snmp_message *msg)
> > case SNMP_V2:
> > if (env->sc_min_seclevel != 0)
> > goto badversion;
> > -   if (ober_scanf_elements(a, "se", , >sm_pdu) != 0)
> > +   if (ober_scanf_elements(a, "seS$", , >sm_pdu) != 
> > 0)
> > goto parsefail;
> > if (strlcpy(msg->sm_community, comn,
> >     sizeof(msg->sm_community)) >= 
> > sizeof(msg->sm_community)) {
> > @@ -230,7 +230,7 @@ snmpe_parse(st? tm_udp.c
> > Index: snmpe.c
> > ===
> > RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
> > retrieving revision 1.68
> > diff -u -p -r1.68 snmpe.c
> > --- snmpe.c 22 Jan 2021 06:33:27 -  1.68
> > +++ snmpe.c 31 Jan 2021 10:55:49 -
> > @@ -220,7 +220,7 @@ snmpe_parse(struct snmp_message *msg)
> > case SNMP_V2:
> > if (env->sc_min_seclevel != 0)
> > goto badversion;
> > -   if (ober_scanf_elements(a, "se", , >sm_pdu) != 0)
> > +   if (ober_scanf_elements(a, "seS$", , >sm_pdu) != 
> > 0)
> > goto parsefail;
> > if (strlcpy(msg->sm_community, comn,
> >     sizeof(msg->sm_community)) >= 
> > sizeof(msg->sm_community)) {
> > @@ -230,7 +230,7 @@ snmpe_parse(struct snmp_message *msg)
> > }
> > break;
> > case SNMP_V3:
> > -   if (ober_scanf_elements(a, "{iisi}e",
> > +   if (ober_scanf_elements(a, "{iisi$}e",
> >     >sm_msgid, >sm_max_msg_size, ,
> >     >sm_secmodel, ) != 0)
> > goto parsefail;
> > @@ -248,7 +248,7 @@ snmpe_parse(struct snmp_message *msg)
> > goto parsefail;
> > }
> >  
> > -   if (ober_scanf_elements(a, "{xxe",
> > +   if (ober_scanf_elements(a, "{xxeS$}$",
> >     >sm_ctxengineid, >sm_ctxengineid_len,
> >     , , >sm_pdu) != 0)
> > goto parsefail;
> > @@ -370,7 +370,7 @@ snmpe_parse(struct snmp_message *msg)
> > }
> >  
> > /* SNMP PDU */
> > -   if (ober_scanf_elements(a, "iiie{et",
> > +   if (ober_scanf_elements(a, "iiie{etS$}$",
> >     , , , >sm_pduend,
> >     >sm_varbind, , ) != 0) {
> > stats->snmp_silentdrops++;
> > @@ -429,7 +429,7 @@ snmpe_parsevarbinds(struct snmp_message 
> >  
> > for (i = 1; varbind != NULL && i < SNMPD_MAXVARBIND;
> >     varbind = varbind->be_next, i++) {
> > -   if (ober_scanf_elements(varbind, "{oe}", , ) == -1) 
> > {
> > +   if (ober_scanf_elements(varbind, "{oeS$}", , ) == 
> > -1) {
> > stats->snmp_inasnparseerrs++;
> > msg->sm_errstr = "invalid varbind";
> > goto varfail;
> > Index: traphandler.c
> > ===
> > RCS file: /cvs/src/usr.sbin/snmpd/traphandler.c,v
> > retrieving revision 1.20
> > diff -u -p -r1.20 traphandler.c
> > --- traphandler.c   22 Jan 2021 06:33:27 -  1.20
> > +++ traphandler.c   31 Jan 2021 10:55:49 -
> > @@ -67,7 +67,7 @@ traphandler_parse(struct snmp_message *m
> > struct privsep  *ps = _env->sc_ps;
> > struct snmp_stats   *stats = _env->sc_stats;
> > struct ber   ber = {0};
> > -   struct ber_element  *vblist = NULL, *elm, *elm2;
> > +   struct ber_element  *vblist = NULL, *elm;
> > struct ber_oid   o1, o2, 

Re: snmpd: Add end of sequence tests

2021-02-12 Thread Martijn van Duren
ping

On Sun, 2021-01-31 at 11:57 +0100, Martijn van Duren wrote:
> Now that ober_scanf_elements supports '$' lets use it.
> 
> Here's a first stab by adding it to snmpd.
> Passing regress and a few manual checks.
> 
> 'e' still doesn't consume the element, but I've talked it over with
> rob@, who said that shouldn't get in the way of using this new feature.
> 
> OK?
> 
> martijn@
> 
> Index: snmpe.c
> ===
> RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
> retrieving revision 1.68
> diff -u -p -r1.68 snmpe.c
> --- snmpe.c 22 Jan 2021 06:33:27 -  1.68
> +++ snmpe.c 31 Jan 2021 10:55:49 -
> @@ -220,7 +220,7 @@ snmpe_parse(struct snmp_message *msg)
> case SNMP_V2:
> if (env->sc_min_seclevel != 0)
> goto badversion;
> -   if (ober_scanf_elements(a, "se", , >sm_pdu) != 0)
> +   if (ober_scanf_elements(a, "seS$", , >sm_pdu) != 0)
> goto parsefail;
> if (strlcpy(msg->sm_community, comn,
>     sizeof(msg->sm_community)) >= sizeof(msg->sm_community)) {
> @@ -230,7 +230,7 @@ snmpe_parse(st? tm_udp.c
> Index: snmpe.c
> ===
> RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
> retrieving revision 1.68
> diff -u -p -r1.68 snmpe.c
> --- snmpe.c 22 Jan 2021 06:33:27 -  1.68
> +++ snmpe.c 31 Jan 2021 10:55:49 -
> @@ -220,7 +220,7 @@ snmpe_parse(struct snmp_message *msg)
> case SNMP_V2:
> if (env->sc_min_seclevel != 0)
> goto badversion;
> -   if (ober_scanf_elements(a, "se", , >sm_pdu) != 0)
> +   if (ober_scanf_elements(a, "seS$", , >sm_pdu) != 0)
> goto parsefail;
> if (strlcpy(msg->sm_community, comn,
>     sizeof(msg->sm_community)) >= sizeof(msg->sm_community)) {
> @@ -230,7 +230,7 @@ snmpe_parse(struct snmp_message *msg)
> }
> break;
> case SNMP_V3:
> -   if (ober_scanf_elements(a, "{iisi}e",
> +   if (ober_scanf_elements(a, "{iisi$}e",
>     >sm_msgid, >sm_max_msg_size, ,
>     >sm_secmodel, ) != 0)
> goto parsefail;
> @@ -248,7 +248,7 @@ snmpe_parse(struct snmp_message *msg)
> goto parsefail;
> }
>  
> -   if (ober_scanf_elements(a, "{xxe",
> +   if (ober_scanf_elements(a, "{xxeS$}$",
>     >sm_ctxengineid, >sm_ctxengineid_len,
>     , , >sm_pdu) != 0)
> goto parsefail;
> @@ -370,7 +370,7 @@ snmpe_parse(struct snmp_message *msg)
> }
>  
> /* SNMP PDU */
> -   if (ober_scanf_elements(a, "iiie{et",
> +   if (ober_scanf_elements(a, "iiie{etS$}$",
>     , , , >sm_pduend,
>     >sm_varbind, , ) != 0) {
> stats->snmp_silentdrops++;
> @@ -429,7 +429,7 @@ snmpe_parsevarbinds(struct snmp_message 
>  
> for (i = 1; varbind != NULL && i < SNMPD_MAXVARBIND;
>     varbind = varbind->be_next, i++) {
> -   if (ober_scanf_elements(varbind, "{oe}", , ) == -1) {
> +   if (ober_scanf_elements(varbind, "{oeS$}", , ) == -1) 
> {
> stats->snmp_inasnparseerrs++;
> msg->sm_errstr = "invalid varbind";
> goto varfail;
> Index: traphandler.c
> ===
> RCS file: /cvs/src/usr.sbin/snmpd/traphandler.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 traphandler.c
> --- traphandler.c   22 Jan 2021 06:33:27 -  1.20
> +++ traphandler.c   31 Jan 2021 10:55:49 -
> @@ -67,7 +67,7 @@ traphandler_parse(struct snmp_message *m
> struct privsep  *ps = _env->sc_ps;
> struct snmp_stats   *stats = _env->sc_stats;
> struct ber   ber = {0};
> -   struct ber_element  *vblist = NULL, *elm, *elm2;
> +   struct ber_element  *vblist = NULL, *elm;
> struct ber_oid   o1, o2, snmpTrapOIDOID;
> struct ber_oid   snmpTrapOID, sysUpTimeOID;
> int  sysUpTime;
> @@ -82,7 +82,7 @@ traphandler_parse(struct snmp_message *m
> goto done;
> break;
> case SNMP_C_TRAPV2:
> -   if (ober_scanf_elements(msg->sm_pdu, "{SSe}", ) == -1) {
> +   if (ober_scanf_elements(msg->sm_pdu, "{SSe}$", ) == -1) {
> stats->snmp_inasnparseerrs++;
> goto done;
> }
> @@ -98,7 +98,7 @@ traphandler_parse(struct snmp_message *m
>  
> (void)ober_string2oid("1.3.6.1.2.1.1.3.0", );
> (void)ober_string2oid("1.3.6.1.6.3.1.1.4.1.0", );
> -   if 

Re: change rpki-client repository code

2021-02-12 Thread Claudio Jeker
On Mon, Feb 08, 2021 at 05:15:40PM +0100, Claudio Jeker wrote:
> Split the repository code into two parts:
> 
> - fetch of the trust anchors (the certs referenced by TAL files)
> - fetch of the MFT files of a repository
> 
> While the two things kind of look similar there are some differences.
> 
> - TA files are loaded via rsync or https URI (only one file needs to be
>   loaded)
> - MFT files need everything inside the repository to be loaded since they
>   reference to other files (.roa, .cer, .crl). These repositories are
>   synced once with rsync and many mft may be part of a repo. Also these
>   repositories can be synced via rsync or RRDP
> 
> To simplify these diverse options it is time to split the code up.
> Introduce a ta_lookup() along with repo_lookup(). Refactor the repo_lookup
> code into subfunctions repo_alloc() and repo_fetch() (both are also used
> by ta_lookup()). Use the caRepository URI to figure out the base URI.
> Simplify rsync_uri_parse() into rsync_base_uri() which clips of excess
> directories from the URI (else thousends of individual rsync calls would
> be made against the RIR's CA repos).
> 
> The big change is that the layout of the cache directory is changed.
> The cache will now have two base directories:
> - ta/ (for all trust anchors)
> - rsync/ (for all other repositories)
> 

My plan at the moment is that rpki-client will split the cache directory
into three parts. ta/, rsync/, and rrdp/. This is done to ensure that data
does not get mixed up. Once this is in then my next step is to support
https:// links in TAL files and fetch the trust anchor via https instead
of rsync. Later RRDP will follow.

-- 
:wq Claudio

Index: extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.42
diff -u -p -r1.42 extern.h
--- extern.h8 Feb 2021 09:22:53 -   1.42
+++ extern.h8 Feb 2021 13:44:22 -
@@ -392,9 +392,7 @@ void proc_parser(int) __attribute__((n
 
 /* Rsync-specific. */
 
-int rsync_uri_parse(const char **, size_t *,
-   const char **, size_t *, const char **, size_t *,
-   enum rtype *, const char *);
+char   *rsync_base_uri(const char *);
 voidproc_rsync(char *, char *, int) __attribute__((noreturn));
 
 /* Logging (though really used for OpenSSL errors). */
Index: main.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
retrieving revision 1.98
diff -u -p -r1.98 main.c
--- main.c  5 Feb 2021 12:26:52 -   1.98
+++ main.c  8 Feb 2021 13:50:20 -
@@ -78,11 +78,12 @@
  * An rsync repository.
  */
 struct repo {
-   char*repo;  /* repository rsync URI */
-   char*local; /* local path name */
-   char*notify; /* RRDB notify URI if available */
-   size_t   id; /* identifier (array index) */
-   int  loaded; /* whether loaded or not */
+   char*repouri;   /* CA repository base URI */
+   char*local; /* local path name */
+   char*uris[2];   /* URIs to fetch from */
+   size_t   id;/* identifier (array index) */
+   int  uriidx;/* which URI is fetched */
+   int  loaded;/* whether loaded or not */
 };
 
 size_t entity_queue;
@@ -284,33 +285,12 @@ entityq_add(struct entityq *q, char *fil
 }
 
 /*
- * Look up a repository, queueing it for discovery if not found.
+ * Allocat a new repository be extending the repotable.
  */
-static const struct repo *
-repo_lookup(const char *uri)
+static struct repo *
+repo_alloc(void)
 {
-   const char  *host, *mod;
-   size_t   hostsz, modsz, i;
-   char*local;
-   struct repo *rp;
-   struct ibuf *b;
-
-   if (!rsync_uri_parse(, ,
-   , , NULL, NULL, NULL, uri))
-   errx(1, "%s: malformed", uri);
-
-   if (asprintf(, "%.*s/%.*s", (int)hostsz, host,
-   (int)modsz, mod) == -1)
-   err(1, "asprintf");
-
-   /* Look up in repository table. */
-
-   for (i = 0; i < rt.reposz; i++) {
-   if (strcmp(rt.repos[i].local, local))
-   continue;
-   free(local);
-   return [i];
-   }
+   struct repo *rp;
 
rt.repos = reallocarray(rt.repos,
rt.reposz + 1, sizeof(struct repo));
@@ -320,28 +300,99 @@ repo_lookup(const char *uri)
rp = [rt.reposz++];
memset(rp, 0, sizeof(struct repo));
rp->id = rt.reposz - 1;
-   rp->local = local;
 
-   if ((rp->repo = strndup(uri, mod + modsz - uri)) == NULL)
-   err(1, "strdup");
+   return rp;
+}
 
-   if (!noop) {
-   if (asprintf(, "%s", rp->local) == -1)
-