Re: www/errata74.html does not exist [CORRECTED]

[Ross L Richardson]

On Wed, Oct 04, 2023 at 12:37:03PM +1100, Ross L Richardson wrote:
> if the OpenBSD home page "Errata" still links to those of the old/current

...
  if the OpenBSD home page "Patches" still links to those of the old/current
...

> 
> Anyway, here's a 74.html version in case it's wanted...

Now with links fixed.






OpenBSD 7.4 Errata



https://www.openbsd.org/errata74.html;>





OpenBSD
7.4 Errata



For errata on a certain release, click below:
2.0,
2.1,
2.2,
2.3,
2.4,
2.5,
2.6,
2.7,
2.8,
2.9,
3.0,
3.1,
3.2,
3.3,
3.4,
3.5,

3.6,
3.7,
3.8,
3.9,
4.0,
4.1,
4.2,
4.3,
4.4,
4.5,
4.6,
4.7,
4.8,
4.9,
5.0,
5.1,

5.2,
5.3,
5.4,
5.5,
5.6,
5.7,
5.8,
5.9,
6.0,
6.1,
6.2,
6.3,
6.4,
6.5,
6.6,
6.7,

6.8,
6.9,
7.0,
7.1,
7.2,
7.3.



Patches for the OpenBSD base system are distributed as unified diffs.
Each patch is cryptographically signed with the
https://man.openbsd.org/OpenBSD-7.4/signify.1;>signify(1) tool and 
contains
usage instructions.
All the following patches are also available in one
https://ftp.openbsd.org/pub/OpenBSD/patches/7.4.tar.gz;>tar.gz file
for convenience.


Alternatively, the https://man.openbsd.org/syspatch;>syspatch(8)
utility can be used to apply binary updates on the following architectures:
amd64, i386, arm64.


Patches for supported releases are also incorporated into the
-stable branch, which is maintained for one year
after release.





None yet.

www/errata74.html does not exist

[Ross L Richardson]

errata.html has a link to errata74.html as the latest, but the latter
does not exist.

[FWIW, this confuses Undeadly's errata list/RSS, so that it bails out
and fails to update for 7.3's patch 017.  That's been fixed manually.]

IIRC, it's normal for the errata file [errata74.html] to be created when
the web site errata have a version roll [prior to the new release], even
if the OpenBSD home page "Errata" still links to those of the old/current
release [7.3].

Anyway, here's a 74.html version in case it's wanted...

Ross





OpenBSD 7.4 Errata



https://www.openbsd.org/errata74.html;>





OpenBSD
7.4 Errata



For errata on a certain release, click below:
2.0,
2.1,
2.2,
2.3,
2.4,
2.5,
2.6,
2.7,
2.8,
2.9,
3.0,
3.1,
3.2,
3.3,
3.4,
3.5,

3.6,
3.7,
3.8,
3.9,
4.0,
4.1,
4.2,
4.3,
4.4,
4.5,
4.6,
4.7,
4.8,
4.9,
5.0,
5.1,

5.2,
5.3,
5.4,
5.5,
5.6,
5.7,
5.8,
5.9,
6.0,
6.1,
6.2,
6.3,
6.4,
6.5,
6.6,
6.7,

6.8,
6.9,
7.0,
7.1,
7.1,
7.2.



Patches for the OpenBSD base system are distributed as unified diffs.
Each patch is cryptographically signed with the
https://man.openbsd.org/OpenBSD-7.4/signify.1;>signify(1) tool and 
contains
usage instructions.
All the following patches are also available in one
https://ftp.openbsd.org/pub/OpenBSD/patches/7.4.tar.gz;>tar.gz file
for convenience.


Alternatively, the https://man.openbsd.org/syspatch;>syspatch(8)
utility can be used to apply binary updates on the following architectures:
amd64, i386, arm64.


Patches for supported releases are also incorporated into the
-stable branch, which is maintained for one year
after release.





None yet.

[Patch] Inconsistent markup in 73.html

[Ross L Richardson]

There's a glaring inconsistency in the markup used for the various .tar.gz
files.  Should be all-or-nothing; patch for the former below...

Ross
--

Index: 73.html
===
RCS file: /cvs/www/73.html,v
retrieving revision 1.33
diff -u -p -r1.33 73.html
--- 73.html 4 Apr 2023 23:37:32 -   1.33
+++ 73.html 5 Apr 2023 05:03:43 -
@@ -60,8 +60,9 @@ RWShXqVD7hfbBpWb1B5EGr1DUX8kkjkTueCsa243
 
 
 
-All applicable copyrights and credits are in the src.tar.gz,
-sys.tar.gz, xenocara.tar.gz, ports.tar.gz files, or in the
+All applicable copyrights and credits are in the src.tar.gz,
+sys.tar.gz, xenocara.tar.gz,
+ports.tar.gz files, or in the
 files fetched via ports.tar.gz.
 
 

[Patch] Probable error in sh.1

[Ross L Richardson]

The word "array" occurs only once in sh.1.  Therefore, either it deserves
more explanation, or removal with something like the patch below.

Ross
==

--- sh.1.orig   Thu Sep  1 10:07:22 2022
+++ sh.1Sun Dec 18 20:47:53 2022
@@ -1390,7 +1390,7 @@
 .Pp
 Where
 .Ar expression
-is an integer, parameter name, or array reference,
+is an integer or parameter name,
 optionally combined with any of the operators described below,
 listed and grouped according to precedence:
 .Bl -tag -width Ds

[Patch] Delete extra word in 72.html

[Ross L Richardson]

Simple fix for typo/edito...

Ross

Index: 72.html
===
RCS file: /cvs/www/72.html,v
retrieving revision 1.54
diff -u -p -r1.54 72.html
--- 72.html 4 Oct 2022 01:38:08 -   1.54
+++ 72.html 4 Oct 2022 06:10:12 -
@@ -576,7 +576,7 @@ to 7.2.
   Added the -b option to https://man.openbsd.org/sysupgrade.8;>sysupgrade(8) to set
an alternative base directory to which the installation files will be
-   downloaded to.
+   downloaded.
   Increased the https://man.openbsd.org/disklabel.8;>disklabel(8) auto
partitioner's maximum size for /usr to 30G.

[Patch] Make errata70.html consistent with earlier errata*.html

[Ross L Richardson]

Hitherto, errata IDs have been "p" rather than "".

Ross

Index: errata70.html
===
RCS file: /cvs/www/errata70.html,v
retrieving revision 1.6
diff -u -p -r1.6 errata70.html
--- errata70.html   31 Oct 2021 00:14:46 -  1.6
+++ errata70.html   31 Oct 2021 04:10:17 -
@@ -99,7 +99,7 @@ after release.
 
 
 
-
+
 001: RELIABILITY FIX: October 31, 2021
  All architectures
 
@@ -109,7 +109,7 @@ In certain configurations, nsd(8) can be
 A source code patch exists which remedies this problem.
 
 
-
+
 002: RELIABILITY FIX: October 31, 2021
  All architectures
 
@@ -119,7 +119,7 @@ Opening /dev/bpf too often could lead to
 A source code patch exists which remedies this problem.
 
 
-
+
 002: SECURITY FIX: October 31, 2021
  All architectures
 

[Patch] Fix a couple of minor errors in OpenSSH section of 70.html

[Ross L Richardson]

Missing space after "RFC" and a typo...
Ross

Index: 70.html
===
RCS file: /cvs/www/70.html,v
retrieving revision 1.87
diff -u -p -r1.87 70.html
--- 70.html 10 Oct 2021 06:32:45 -  1.87
+++ 70.html 10 Oct 2021 09:07:43 -
@@ -895,7 +895,7 @@ to 7.0.
sshd(8): remove
references to ChallengeResponseAuthentication in favour of
KbdInteractiveAuthentication. The former is what was in SSHv1,
-   the latter is what is in SSHv2 (RFC4256)
+   the latter is what is in SSHv2 (RFC 4256)
and they were treated as somewhat but not entirely equivalent. We
retain the old name as a deprecated alias so configuration
files continue to work as well as a reference in the man page
@@ -938,7 +938,7 @@ to 7.0.
which allows RSA/SHA2 signatures for public key authentication but
fails to advertise this correctly via SSH2_MSG_EXT_INFO. This
causes clients of these server to incorrectly match
-   PubkeyAcceptedAlgorithmse and potentially refuse to offer
+   PubkeyAcceptedAlgorithms and potentially refuse to offer
valid keys.
 sftp(1)/
scp(1): degrade

[Patch] Capitalise "DHCP" and "DNS" in 70.html

[Ross L Richardson]

Think I got them all.
Ross

Index: 70.html
===
RCS file: /cvs/www/70.html,v
retrieving revision 1.85
diff -u -p -r1.85 70.html
--- 70.html 9 Oct 2021 18:27:18 -   1.85
+++ 70.html 10 Oct 2021 05:02:35 -
@@ -204,7 +204,7 @@ to 7.0.
in https://man.openbsd.org/vmm.4;>vmm(4).
Fixed https://man.openbsd.org/vmm.4;>vmm(4) vcpu 
locking issues.
Added https://man.openbsd.org/vmd.8;>vmd(8) support 
for variable length vionet rx descriptor chains.
-   Prevented stack overflow in https://man.openbsd.org/vmd.8;>vmd(8) due to large dhcp packets on 
local interfaces.
+   Prevented stack overflow in https://man.openbsd.org/vmd.8;>vmd(8) due to large DHCP packets on 
local interfaces.
Allowed locking of a randomly assigned lladdr in https://man.openbsd.org/vmd.8;>vmd(8).
Skipped inspecting non-udp packets on local interfaces for https://man.openbsd.org/vmd.8;>vmd(8).
Prevented guest virtio drivers from causing stack and buffer 
overflows in https://man.openbsd.org/vmd.8;>vmd(8).
@@ -589,7 +589,7 @@ to 7.0.
 
Changed https://man.openbsd.org/dhcpleased.8;>dhcpleased(8) 
client
-   identifier transmission to match other dhcp client 
implementations.
+   identifier transmission to match other DHCP client 
implementations.
Simplified https://man.openbsd.org/dhcpleasectl.8;>dhcpleasectl(8) and
added syntax to match Retried broadcast with https://man.openbsd.org/dhcpleased.8;>dhcpleased(8) 
when the
-   dhcp server is unreachable via unicast UDP.
+   DHCP server is unreachable via unicast UDP.
Made https://man.openbsd.org/resolvd.8;>resolvd(8)
-   accept dns proposals for the loopback addresses.
+   accept DNS proposals for the loopback addresses.
Added to https://man.openbsd.org/dhcpleased.conf.5;>dhcpleased.conf(5)
the ability to ignore routes or nameservers from a lease and to 
ignore
@@ -624,12 +624,12 @@ to 7.0.
href="https://man.openbsd.org/resolvd.8;>resolvd(8), https://man.openbsd.org/slaacd.8;>slaacd(8) and https://man.openbsd.org/dhcpleased.8;>dhcpleased(8).
-   Implemented classless static routes dhcp option in Implemented classless static routes DHCP option in https://man.openbsd.org/dhcpleased.8;>dhcpleased(8).
Added a new "nameserver" command to https://man.openbsd.org/route.8;>route(8), sending
nameserver proposals to https://man.openbsd.org/resolvd.8;>resolvd(8) using 
the dns
+   href="https://man.openbsd.org/resolvd.8;>resolvd(8) using 
the DNS
proposal protocol over the route socket. This command is 
intended be
used to integrate userland triggered nameserver changes, for 
example
by VPN software.

[Patch] Fix a few typos (etc.) in 70.html

[Ross L Richardson]

Hope these are right!

Ross

Index: 70.html
===
RCS file: /cvs/www/70.html,v
retrieving revision 1.83
diff -u -p -r1.83 70.html
--- 70.html 8 Oct 2021 16:31:36 -   1.83
+++ 70.html 9 Oct 2021 11:04:41 -
@@ -483,7 +483,7 @@ to 7.0.
In https://man.openbsd.org/bgpctl.8;>bgpctl(8) print 
out both the sent "Neighbor capabilities" and the
"Negotiated capabilities" for a session. 
Print timestamps both as a formatted and a pure time in seconds
-   filed in various JSON objects.
+   field in various JSON objects.
Fixed a bug, where during https://man.openbsd.org/bgpd.8;>bgpd(8) config reloads prefixes of the
wrong address family could leak to peers resulting in session 
resets.
Added support for RFC 7313 - Enhanced Route Refresh
@@ -501,7 +501,7 @@ to 7.0.
Implemented receive side of RFC 7911 - Advertisement of Multiple 
Paths
in BGP. OpenBGPD is currently not able to send multiple paths out.
Improved checks of VRPs loaded via RTR or from the roa-set table.
-   Allowed to optionally specify an expiry time for roa-set entries to
+   Allowed optionally specifying an expiry time for roa-set entries to
mitigate BGP route decision making based on outdated RPKI data.
OpenBGPD's companion rpki-client(8) produces roa-sets with the
new 'expires' property
@@ -542,10 +542,10 @@ to 7.0.
   and CRL validity times. The 'expires' value can be used to avoid 
route
   selection based on stale data when generating VRP sets, when faced
   with loss of communication between consumer and validator, or
-  validator and CA repository,
+  validator and CA repository.
Made the runtime timeout (-s option) also trigger in
-  child proecesses.
-   Improved RRDP support and make RRDP as default protocol for
+  child processes.
+   Improved RRDP support and make RRDP the default protocol for
   synchronizing the RPKI repository data, with https://man.openbsd.org/openrsync.1;>openrsync(1) used as 
secondary.
At startup, warn if the filesystem containing the cache directory

[Possible patch] httpd and HEAD requests to CGI scripts

[Ross L Richardson]

This relates to the earlier messages I sent to bugs@ in:
https://marc.info/?t=16330937691=1=2

RFC 7231 [HTTP/1.1] section 4.3.2. "HEAD" states:
The HEAD method is identical to GET except that the server MUST NOT
send a message body in the response (i.e., the response terminates at
the end of the header section).

RFC 3875 [The Common Gateway Interface (CGI) Version 1.1] in
section 4.3.2 HEAD states:
The HEAD method requests the script to do sufficient processing to
return the response header fields, without providing a response
message-body.  The script MUST NOT provide a response message-body
for a HEAD request.  If it does, then the server MUST discard the
message-body when reading the response from the script.

Therefore, a CGI script which sends a message body is violation of the CGI
specification, but so is the server if it fails to elide the body.


With httpd, we see (for example):

$ printf "HEAD /cgi-bin/ftplist.cgi?dbversion=1 
HTTP/1.0\r\nHost:ftp.openbsd.org\r\n\r\n" \
| nc -c ftp.openbsd.org https
HTTP/1.0 200 OK
Connection: close
Content-type: text/plain
Date: Fri, 01 Oct 2021 12:50:59 GMT
Server: OpenBSD httpd

https://mirror.aarnet.edu.au/pub/OpenBSD  Canberra, Australia
https://cdn.openbsd.org/pub/OpenBSD  Fastly (CDN)
https://cloudflare.cdn.openbsd.org/pub/OpenBSD   Cloudflare (CDN)
...
RND_BYTES=0xfe9832a3...


So httpd isn't behaving correctly.

The patch below is offered in the hope that it is a starting point for
a proper solution.  Whilst it solves the problem in a simple test case,
I'm insufficiently familiar with the httpd code to know whether this is
correct or sufficient!

Ross

Index: server_fcgi.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
retrieving revision 1.88
diff -u -p -r1.88 server_fcgi.c
--- server_fcgi.c   20 May 2021 15:12:10 -  1.88
+++ server_fcgi.c   9 Oct 2021 10:18:55 -
@@ -559,6 +559,11 @@ server_fcgi_read(struct bufferevent *bev
return;
}
}
+   if (clt->clt_fcgi.headerssent &&
+   ((struct http_descriptor *)
+   clt->clt_descreq)->http_method
+   == HTTP_METHOD_HEAD)
+   return;
if (server_fcgi_writechunk(clt) == -1) {
server_abort_http(clt, 500,
"encoding error");

[Patch] httpd handling of HEAD requests to CGI w.r.t. Content-Length

[Ross L Richardson]

This is a rewording of my later posts to bugs@ in:
https://marc.info/?t=16330937691=1=2


RFC 7231 [HTTP 1.1] states that, for a HEAD request, the server SHOULD
send the same header fields in response to HEAD as it would for GET,
except that payload headers MAY be omitted.  Content-Length is such
a header field.

The CGI on beta.undeadly.org has been updated (to be RFC-compliant)
such that it does not send a body in response to HEAD requests.
We now how:

 www.undeadly.org ##
$ printf "HEAD /cgi?action=front HTTP/1.0\r\nHost: www.undeadly.org\r\n\r\n" \
| nc -c www.undeadly.org https \
| head
HTTP/1.0 200 OK
Connection: close
Content-Type: text/html
Date: Wed, 06 Oct 2021 10:24:59 GMT
Server: OpenBSD httpd
Strict-Transport-Security: max-age=31536000; preload





 beta.undeadly.org #
$ printf "HEAD /cgi?action=front HTTP/1.0\r\nHost: beta.undeadly.org\r\n\r\n" \
| nc -c beta.undeadly.org https
HTTP/1.0 200 OK
Connection: close
Content-Length: 0
Content-Type: text/html
Date: Wed, 06 Oct 2021 10:25:01 GMT
Server: OpenBSD httpd
Strict-Transport-Security: max-age=31536000; preload

So, if the CGI treats HEAD the same way as GET [and returns a body],
httpd does not add a Content-Length header.  However, if the CGI behaves
correctly, and does not include the body in the response, httpd _does_
add a Content-Type header.  That's clearly wrong/unhelpful.

I'm insufficiently familiar with the httpd code to be certain that the
patch below is correct, but it fixes the problem in light testing.

Ross

Index: server_fcgi.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
retrieving revision 1.88
diff -u -p -r1.88 server_fcgi.c
--- server_fcgi.c   20 May 2021 15:12:10 -  1.88
+++ server_fcgi.c   7 Oct 2021 02:56:07 -
@@ -621,12 +621,14 @@ server_fcgi_header(struct client *clt, u
/* Can't chunk encode an empty body. */
clt->clt_fcgi.chunked = 0;
 
-   /* But then we need a Content-Length... */
-   key.kv_key = "Content-Length";
-   if ((kv = kv_find(>http_headers, )) == NULL) {
-   if (kv_add(>http_headers,
-   "Content-Length", "0") == NULL)
-   return (-1);
+   /* But then we need a Content-Length unless method is HEAD... */
+   if (desc->http_method != HTTP_METHOD_HEAD) {
+   key.kv_key = "Content-Length";
+   if ((kv = kv_find(>http_headers, )) == NULL) {
+   if (kv_add(>http_headers,
+   "Content-Length", "0") == NULL)
+   return (-1);
+   }
}
}
 

[Patch] Typo in www/events.html

[Ross L Richardson]

Just a stray '>'.

Ross
--
Index: events.html
===
RCS file: /cvs/www/events.html,v
retrieving revision 1.1205
diff -u -p -r1.1205 events.html
--- events.html 18 Sep 2021 13:05:34 -  1.1205
+++ events.html 19 Sep 2021 12:36:48 -
@@ -50,7 +50,7 @@ like-minded people.
 
 Past events:
 
-2021>
+2021
 
 
 

[Patch] Typo in errata69.html

[Ross L Richardson]

Broke the parsing on Undeadly :-(

Ross


Index: errata69.html
===
RCS file: /cvs/www/errata69.html,v
retrieving revision 1.3
diff -u -p -r1.3 errata69.html
--- errata69.html   3 May 2021 20:12:53 -   1.3
+++ errata69.html   4 May 2021 05:36:41 -
@@ -98,7 +98,7 @@ after release.
 
 
 
-
+
 001: RELIABILITY FIX: May 4, 2021
  amd64
 

[Patch] Delete extra word in OpenSSH section of 69.html

[Ross L Richardson]

(Probably the last report from me for 69.html)


Index: 69.html
===
RCS file: /cvs/www/69.html,v
retrieving revision 1.63
diff -u -p -r1.63 69.html
--- 69.html 19 Apr 2021 20:06:24 -  1.63
+++ 69.html 20 Apr 2021 05:20:54 -
@@ -1674,7 +1674,7 @@ to 6.9.
 
https://man.openbsd.org/ssh.1;>ssh(1), https://man.openbsd.org/sshd.8;>sshd(8): add a new
-   LogVerbose configuration directive for that allows forcing maximum
+   LogVerbose configuration directive that allows forcing maximum
debug logging by file/function/line pattern-lists.
 
https://man.openbsd.org/ssh.1;>ssh(1): when

[Patch] Typos in OpenSSH section of 69.html

[Ross L Richardson]

(Haven't finished reading it yet...)


Index: 69.html
===
RCS file: /cvs/www/69.html,v
retrieving revision 1.61
diff -u -p -r1.61 69.html
--- 69.html 19 Apr 2021 11:22:08 -  1.61
+++ 69.html 19 Apr 2021 12:11:28 -
@@ -1621,7 +1621,7 @@ to 6.9.
agent either to an account shared with a malicious user or to a host
with an attacker holding root access.
 
-Potentially incompatible changes.
+Potentially incompatible changes
 
https://man.openbsd.org/ssh.1;>ssh(1), https://man.openbsd.org/sshd.8;>sshd(8): this release
@@ -1651,8 +1651,8 @@ to 6.9.
replaced with sntrup761x25519-sha...@openssh.com. Per its designers,
the sntrup4591761 algorithm was superseded almost two years ago by
sntrup761.
-   (note this both the updated method and the one that it replaced are
-   disabled by default)
+   (Note that both the updated method and the one that it replaced are
+   disabled by default.)
 
https://man.openbsd.org/ssh.1;>ssh(1): disable
CheckHostIP by default. It provides insignificant benefits while

[Patch] Possible redundant item in 69.html

[Ross L Richardson]

The last item under OpenSMTPD seems to be covered by the second last item
(or the difference is really subtle).

Deleting the final item may be appropriate.

Ross


Index: 69.html
===
RCS file: /cvs/www/69.html,v
retrieving revision 1.60
diff -u -p -r1.60 69.html
--- 69.html 19 Apr 2021 07:05:46 -  1.60
+++ 69.html 19 Apr 2021 11:22:09 -
@@ -1125,9 +1125,6 @@ to 6.9.
TLS listeners may be configured with multiple certificates,
the matching is based on the names included in these 
certificates.
Allow to specify tls protocols and ciphers per listener and relay 
action.
-   Allowed https://man.openbsd.org/smtpd.conf.5;>smtpd.conf(5)
-   specification of tls protocols and ciphers on relay actions.
 
   
 

[Patch] Spell ...initializ... consistently in 69.html

[Ross L Richardson]

It may be pedantic, but the only offending case has different spellings
in the same sentence!

Ross


Index: 69.html
===
RCS file: /cvs/www/69.html,v
retrieving revision 1.60
diff -u -p -r1.60 69.html
--- 69.html 19 Apr 2021 07:05:46 -  1.60
+++ 69.html 19 Apr 2021 11:16:49 -
@@ -1468,7 +1468,7 @@ to 6.9.
Correct the return value type from ERR_peek_error() to a long.
 
Avoid use of uninitialized in ASN1_time_parse() which could happen
-   on parsing UTCTime if the caller did not initialise the passed
+   on parsing UTCTime if the caller did not initialize the passed
struct tm.
 
Destroy the mutex in a tls_config object on tls_config_free().

[Patch] Relocate a vmctl entry in 69.html

[Ross L Richardson]

It probably belongs with the other vmctl entry rather than under
userland networking changes...

Ross


Index: 69.html
===
RCS file: /cvs/www/69.html,v
retrieving revision 1.53
diff -u -p -r1.53 69.html
--- 69.html 18 Apr 2021 12:08:06 -  1.53
+++ 69.html 19 Apr 2021 05:06:03 -
@@ -307,6 +307,11 @@ to 6.9.
Made https://man.openbsd.org/vmctl.8;>vmctl(8)
properly indicate VMs are stopping instead of "running" with 
"vmctl
status".
+   Simplify argument parsing of
+   https://man.openbsd.org/vmctl.8;>vmctl(8) 
stop
+   thereby avoiding a
+   https://man.openbsd.org/printf.3;>printf(3) "%s" 
NULL,
+   a use of uninitialized and a dead else branch.
Cleaned up events on https://man.openbsd.org/vmd.8;>vmd(8) pause or resume 
and
fixed an issue leading to broken serial console by cleanly 
tearing
@@ -1094,11 +1099,6 @@ to 6.9.
analysis of TCP connections.
Avoid leaking the help text in
https://man.openbsd.org/tcpbench.1;>systat(8).
-   Simplify argument parsing of
-   https://man.openbsd.org/vmctl.8;>vmctl(8) 
stop
-   thereby avoiding a
-   https://man.openbsd.org/printf.3;>printf(3) "%s" 
NULL,
-   a use of uninitialized and a dead else branch.
Increased the maximum length for CHAP challenges to 96 octets to
ensure https://man.openbsd.org/npppd.8;>npppd(8) 
can
handle longer challenges, such as those sent by Juniper.

[Patch] Typo ("it's" should be "its") in 69.html

[Ross L Richardson]

Just an incorrect possessive form...

Ross


Index: 69.html
===
RCS file: /cvs/www/69.html,v
retrieving revision 1.53
diff -u -p -r1.53 69.html
--- 69.html 18 Apr 2021 12:08:06 -  1.53
+++ 69.html 19 Apr 2021 04:25:56 -
@@ -757,7 +757,7 @@ to 6.9.
href="https://man.openbsd.org/ospfd.8;>ospfd(8) for interfaces
that share the same IP.
 
-The https://man.openbsd.org/pf.4;>pf(4) packet filter and 
it's userland utility:
+The https://man.openbsd.org/pf.4;>pf(4) packet filter and 
its userland utility:
 
Relaxed checks in https://man.openbsd.org/pfctl.8;>pfctl(8) and 

Incomplete sentence in 69.html

[Ross L Richardson]

Under "IEEE 802.11 wireless stack improvements and bugfixes:", an item
appears to have been truncated...
"Fixed automatic selection of the 11a/b/g/n/ac operating mode when"

Ross

[Patch] "usb" ==> "USB" for consistency

[Ross L Richardson]

Probably!

Ross



Index: 69.html
===
RCS file: /cvs/www/69.html,v
retrieving revision 1.52
diff -u -p -r1.52 69.html
--- 69.html 17 Apr 2021 20:45:22 -  1.52
+++ 69.html 18 Apr 2021 11:19:14 -
@@ -424,7 +424,7 @@ to 6.9.
defaulting offset to the beginning of the largest free space and
preventing the creation of overlapping partitions.
Fixed a crash that could occur in https://man.openbsd.org/sndiod.8;>sndiod(8) when a usb
+   href="https://man.openbsd.org/sndiod.8;>sndiod(8) when a USB
device is unplugged.
Append .html suffixes to temporary files in https://man.openbsd.org/mandoc.1;>mandoc(1) to allow

[Patch] Fix mangled sentence in 69.html (apmd control socket)

[Ross L Richardson]

Simplest fix below.

Ross


Index: 69.html
===
RCS file: /cvs/www/69.html,v
retrieving revision 1.52
diff -u -p -r1.52 69.html
--- 69.html 17 Apr 2021 20:45:22 -  1.52
+++ 69.html 18 Apr 2021 11:05:33 -
@@ -450,9 +450,9 @@ to 6.9.
Removed the 30s minimum delay for https://man.openbsd.org/xlock.1;>xlock(1) timeouts.
Stopped deleting the control socket on exit in https://man.openbsd.org/apmd.8;>apmd(8) exit, as 
deleting
+   href="https://man.openbsd.org/apmd.8;>apmd(8), as deleting
the socket in process after calling https://man.openbsd.org/unveil.2;>unveil(2) would 
cause a
+   href="https://man.openbsd.org/unveil.2;>unveil(2) would 
cause an
unveil restriction violation,
   
 

Re: random manual pages

[Ross L Richardson]

On Thu, Feb 11, 2021 at 10:40:07PM -0700, Theo de Raadt wrote:
>[...]
> +original development, is underspecified, and should not be relied up to

"relied up to" should probably be "relied upon to" (twice).

Ross

Re: login_passwd.c (etc.) and auth_mkvalue(3) returning NULL

[Ross L Richardson]

On Wed, Dec 30, 2020 at 09:08:53AM -0700, Todd C. Miller wrote:
>[...]
> Yes, that should be checked.  In the case of login_passwd.c there
> is really no reason to use auth_mkvalue(3) at all as there is nothing
> that needs to be escaped.  I think the simplest approach is to send
> a reject message if there is a memory allocation error.
> 
>  - todd
> 
> Index: login_passwd/login_passwd.c
> ===
> RCS file: /cvs/src/libexec/login_passwd/login_passwd.c,v
> retrieving revision 1.18
> diff -u -p -u -r1.18 login_passwd.c
> --- login_passwd/login_passwd.c   15 May 2020 17:25:39 -  1.18
> +++ login_passwd/login_passwd.c   30 Dec 2020 16:05:30 -
> @@ -121,7 +121,7 @@ main(int argc, char *argv[])
>   }
>   if (wheel != NULL && strcmp(wheel, "yes") != 0) {
>   fprintf(back, BI_VALUE " errormsg %s\n",
> - auth_mkvalue("you are not in group wheel"));
> + "you are not in group wheel");
>   fprintf(back, BI_REJECT "\n");
>   exit(1);
>   }
>[...]

It could, of course, just use a fixed string rather than the "%s" format,
although the latter is certainly clear(er) and consistent.

With auth_mkvalue() not being used, I don't think it needs to include
 any more.

Thanks,
Ross

login_passwd.c (etc.) and auth_mkvalue(3) returning NULL

[Ross L Richardson]

auth_mkvalue(3) may return NULL (if no memory is available), but
login_passwd.c and friends use the return value without checking.

$ cd /usr/src/libexec/
$ egrep -B 1 auth_mkval login_*/*.c
login_passwd/login_passwd.c-fprintf(back, BI_VALUE " errormsg %s\n",
login_passwd/login_passwd.c:auth_mkvalue("you are not in group 
wheel"));
--
login_radius/login_radius.c-(void)fprintf(back, BI_VALUE " 
challenge %s\n",
login_radius/login_radius.c:auth_mkvalue(challenge));
--
login_radius/login_radius.c-else if (emsg)
login_radius/login_radius.c:(void)fprintf(back, "value errormsg 
%s\n", auth_mkvalue(emsg));
--
login_skey/login_skey.c-fprintf(back, BI_VALUE " challenge 
%s\n",
login_skey/login_skey.c:auth_mkvalue(challenge));
--
login_token/login_token.c-  fprintf(back, BI_VALUE " 
challenge %s\n",
login_token/login_token.c:  auth_mkvalue(challenge));


Is that considered acceptable in the context?

Ross

Re: [Patch] Change httpd's handling of request "Host:" headers

[Ross L Richardson]

On Mon, Aug 10, 2020 at 10:37:16PM +0200, Sebastian Benoit wrote:
>[...]
> 
> In which way can the current behaviour cause problems?

Whilst a client sending bizarre requests perhaps deserves what they get,
I thought an error status would be better than OK.

If odd hostnames are accepted, they may end up in $SERVER_NAME in
redirects.

>[...]
> As jca@ shows the first host can be a dummy.

That's certainly a nice workaround (although, of course, it requires
configuration and prevents requests without Host headers from getting
anything other than 404).

> 
> I kind of think that this is a documentation problem, we should docuemnt
> this in the manpage and maybe example config:
> 
> diff --git etc/examples/httpd.conf etc/examples/httpd.conf
>[...]

The diff looks good to me.

Thanks to you both.

Ross

Re: [Patch] Change httpd's handling of request "Host:" headers

[Ross L Richardson]

Leo,

On Mon, Aug 10, 2020 at 08:46:19AM +0200, Leo Unglaub wrote:
> Hey,
> i love your patch. The current behavour always bothered me because it caused
> servers to display "wrong" sites as defaults for all requests missing the
> Host header. I really like your patch and it works fine for me on my
> servers.
>[...]

Thanks for testing.

I forgot to mention that, by requiring an exact match, the patch should
fix the port problem [see below] too.

The problem:
$ printf "HEAD / HTTP/1.0\r\nHost: www.openbsd.org:22\r\n\r\n" \
| nc www.openbsd.org 80 | sed 1q
HTTP/1.0 200 OK

Ross

[Patch] Change httpd's handling of request "Host:" headers

[Ross L Richardson]

At present, if a request contains no "Host:" header [HTTP pre-1.1] or
if the supplied header does not match any of the servers configured
in httpd.conf, the request is directed to the first server.  This
isn't documented, AFAICT.

For example, if httpd.conf has just one server
server "www.example.com"
then we currently get
$ printf "HEAD / HTTP/1.0\r\nHost: www.openbsd.org\r\n\r\n" \
| nc www.example.com www | sed 1q 
HTTP/1.0 200 OK

This behaviour strikes me as wrong (or at least sub-optimal) in the
case of non-matching "Host:" headers.  The simplistic patch below
changes things to return a 404 status if no matching server is found.

[If status code 400 (bad request) is preferred, "goto fail;"
could be used.]

Justification:
- This seems more correct, and is consistent with the "fail closed"
  approach.
- There is a net gain in functionality, as use of glob/patterns
  wildcards can easily re-establish the current behaviour.  In
  contrast, there's no way at present to disable the implicit
  match-anything behaviour.

If this is adopted, it should be document in current.html
A followup patch could merge this if statement with the one above it.

Several other issues exist in "Host:" header handling.

Ross
--

Index: server_http.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
retrieving revision 1.140
diff -u -p -r1.140 server_http.c
--- server_http.c   3 Aug 2020 10:59:53 -   1.140
+++ server_http.c   9 Aug 2020 04:37:08 -
@@ -1200,7 +1200,7 @@ server_response(struct httpd *httpd, str
struct server_config*srv_conf = >srv_conf;
struct kv   *kv, key, *host;
struct str_find  sm;
-   int  portval = -1, ret;
+   int  hostmatch = 0, portval = -1, ret;
char*hostval, *query;
const char  *errstr = NULL;
 
@@ -1277,16 +1277,20 @@ server_response(struct httpd *httpd, str
/* Replace host configuration */
clt->clt_srv_conf = srv_conf;
srv_conf = NULL;
+   hostmatch = 1;
break;
}
}
}
 
-   if (srv_conf != NULL) {
+   if (host == NULL) {
/* Use the actual server IP address */
if (server_http_host(>clt_srv_ss, hostname,
sizeof(hostname)) == NULL)
goto fail;
+   } else if (!hostmatch) {
+   server_abort_http(clt, 404, "not found");
+   return (-1);
} else {
/* Host header was valid and found */
if (strlcpy(hostname, host->kv_value, sizeof(hostname)) >=

[Patch] Remove unused functions in httpd[.ch]

[Ross L Richardson]

cppcheck reports that kv_inherit(), kv_log(), and print_time() are unused.

The patch below deletes them.

Ross
--

Index: httpd.c
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.c,v
retrieving revision 1.69
diff -u -p -r1.69 httpd.c
--- httpd.c 30 Jul 2020 21:06:19 -  1.69
+++ httpd.c 3 Aug 2020 05:35:09 -
@@ -1059,50 +1059,6 @@ kv_free(struct kv *kv)
 }
 
 struct kv *
-kv_inherit(struct kv *dst, struct kv *src)
-{
-   memset(dst, 0, sizeof(*dst));
-   memcpy(dst, src, sizeof(*dst));
-   TAILQ_INIT(>kv_children);
-
-   if (src->kv_key != NULL) {
-   if ((dst->kv_key = strdup(src->kv_key)) == NULL) {
-   kv_free(dst);
-   return (NULL);
-   }
-   }
-   if (src->kv_value != NULL) {
-   if ((dst->kv_value = strdup(src->kv_value)) == NULL) {
-   kv_free(dst);
-   return (NULL);
-   }
-   }
-
-   return (dst);
-}
-
-int
-kv_log(struct evbuffer *log, struct kv *kv)
-{
-   char*msg;
-
-   if (log == NULL)
-   return (0);
-   if (asprintf(, " [%s%s%s]",
-   kv->kv_key == NULL ? "(unknown)" : kv->kv_key,
-   kv->kv_value == NULL ? "" : ": ",
-   kv->kv_value == NULL ? "" : kv->kv_value) == -1)
-   return (-1);
-   if (evbuffer_add(log, msg, strlen(msg)) == -1) {
-   free(msg);
-   return (-1);
-   }
-   free(msg);
-
-   return (0);
-}
-
-struct kv *
 kv_find(struct kvtree *keys, struct kv *kv)
 {
struct kv   *match;
@@ -1270,22 +1226,6 @@ print_host(struct sockaddr_storage *ss, 
buf[0] = '\0';
return (NULL);
}
-   return (buf);
-}
-
-const char *
-print_time(struct timeval *a, struct timeval *b, char *buf, size_t len)
-{
-   struct timeval  tv;
-   unsigned long   h, sec, min;
-
-   timerclear();
-   timersub(a, b, );
-   sec = tv.tv_sec % 60;
-   min = tv.tv_sec / 60 % 60;
-   h = tv.tv_sec / 60 / 60;
-
-   snprintf(buf, len, "%.2lu:%.2lu:%.2lu", h, min, sec);
return (buf);
 }
 
Index: httpd.h
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
retrieving revision 1.148
diff -u -p -r1.148 httpd.h
--- httpd.h 30 Jul 2020 21:06:19 -  1.148
+++ httpd.h 3 Aug 2020 05:35:09 -
@@ -731,8 +731,6 @@ void kv_delete(struct kvtree *, struct
 struct kv  *kv_extend(struct kvtree *, struct kv *, char *);
 voidkv_purge(struct kvtree *);
 voidkv_free(struct kv *);
-struct kv  *kv_inherit(struct kv *, struct kv *);
-int kv_log(struct evbuffer *, struct kv *);
 struct kv  *kv_find(struct kvtree *, struct kv *);
 int kv_cmp(struct kv *, struct kv *);
 struct media_type
@@ -751,7 +749,6 @@ struct auth *auth_add(struct serverauth 
 struct auth*auth_byid(struct serverauth *, uint32_t);
 voidauth_free(struct serverauth *, struct auth *);
 const char *print_host(struct sockaddr_storage *, char *, size_t);
-const char *print_time(struct timeval *, struct timeval *, char *, size_t);
 const char *printb_flags(const uint32_t, const char *);
 voidgetmonotime(struct timeval *);
 

[Patch] Remove unused functions in httpd's proc(?)

[Ross L Richardson]

cppcheck reports that proc_iev and proc_ispeer are unused.

Unless they are wanted for consistency with other versions of proc.c,
tbey can be removed.

Ross


Index: httpd.h
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
retrieving revision 1.148
diff -u -p -r1.148 httpd.h
--- httpd.h 30 Jul 2020 21:06:19 -  1.148
+++ httpd.h 3 Aug 2020 05:21:39 -
@@ -803,8 +803,6 @@ int  proc_forward_imsg(struct privsep *,
enum privsep_procid, int);
 struct imsgbuf *
 proc_ibuf(struct privsep *, enum privsep_procid, int);
-struct imsgev *
-proc_iev(struct privsep *, enum privsep_procid, int);
 int proc_flush_imsg(struct privsep *, enum privsep_procid, int);
 voidimsg_event_add(struct imsgev *);
 int imsg_compose_event(struct imsgev *, uint16_t, uint32_t,
Index: proc.c
===
RCS file: /cvs/src/usr.sbin/httpd/proc.c,v
retrieving revision 1.38
diff -u -p -r1.38 proc.c
--- proc.c  9 Sep 2018 21:06:51 -   1.38
+++ proc.c  3 Aug 2020 05:21:39 -
@@ -43,24 +43,11 @@ void proc_open(struct privsep *, int, i
 voidproc_accept(struct privsep *, int, enum privsep_procid,
unsigned int);
 voidproc_close(struct privsep *);
-int proc_ispeer(struct privsep_proc *, unsigned int, enum privsep_procid);
 voidproc_shutdown(struct privsep_proc *);
 voidproc_sig_handler(int, short, void *);
 voidproc_range(struct privsep *, enum privsep_procid, int *, int *);
 int proc_dispatch_null(int, struct privsep_proc *, struct imsg *);
 
-int
-proc_ispeer(struct privsep_proc *procs, unsigned int nproc,
-enum privsep_procid type)
-{
-   unsigned inti;
-
-   for (i = 0; i < nproc; i++)
-   if (procs[i].p_id == type)
-   return (1);
-   return (0);
-}
-
 enum privsep_procid
 proc_getid(struct privsep_proc *procs, unsigned int nproc,
 const char *proc_name)
@@ -819,15 +806,6 @@ proc_ibuf(struct privsep *ps, enum privs
 
proc_range(ps, id, , );
return (>ps_ievs[id][n].ibuf);
-}
-
-struct imsgev *
-proc_iev(struct privsep *ps, enum privsep_procid id, int n)
-{
-   int  m;
-
-   proc_range(ps, id, , );
-   return (>ps_ievs[id][n]);
 }
 
 /* This function should only be called with care as it breaks async I/O */

[Patch] Remove redundant condition in httpd's server_http.c

[Ross L Richardson]

cppcheck reports this [and other less simple things].

Ross

Index: server_http.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
retrieving revision 1.139
diff -u -p -r1.139 server_http.c
--- server_http.c   22 May 2020 07:18:17 -  1.139
+++ server_http.c   3 Aug 2020 05:01:26 -
@@ -1273,8 +1273,7 @@ server_response(struct httpd *httpd, str
hostname, FNM_CASEFOLD);
}
if (ret == 0 &&
-   (portval == -1 ||
-   (portval != -1 && portval == srv_conf->port))) {
+   (portval == -1 || portval == srv_conf->port)) {
/* Replace host configuration */
clt->clt_srv_conf = srv_conf;
srv_conf = NULL;

[Patch] Remove unnecessary assignment in httpd's server_fcgi.c

[Ross L Richardson]

cppcheck finds an unnecessary assignment.

The patch below deletes it.

Ross

Index: server_fcgi.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
retrieving revision 1.81
diff -u -p -r1.81 server_fcgi.c
--- server_fcgi.c   9 Feb 2020 09:44:04 -   1.81
+++ server_fcgi.c   2 Aug 2020 11:49:44 -
@@ -684,7 +684,7 @@ server_fcgi_header(struct client *clt, u
 
/* Date header is mandatory and should be added as late as possible */
key.kv_key = "Date";
-   if ((kv = kv_find(>http_headers, )) == NULL &&
+   if (kv_find(>http_headers, ) == NULL &&
(server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
kv_add(>http_headers, "Date", tmbuf) == NULL))
return (-1);

[Patch] Delete redundant condition in httpd's proc.c

[Ross L Richardson]

cppcheck finds a redundant condition.

The patch below deletes it.

Ross

Index: proc.c
===
RCS file: /cvs/src/usr.sbin/httpd/proc.c,v
retrieving revision 1.38
diff -u -p -r1.38 proc.c
--- proc.c  9 Sep 2018 21:06:51 -   1.38
+++ proc.c  2 Aug 2020 11:38:50 -
@@ -401,7 +401,7 @@ proc_kill(struct privsep *ps)
free(cause);
} else
log_warnx("lost child: pid %u", pid);
-   } while (pid != -1 || (pid == -1 && errno == EINTR));
+   } while (pid != -1 || errno == EINTR);
 }
 
 void

[Patch] Delete unread assignments in httpd's config.c

[Ross L Richardson]

cppcheck finds some unread assignments in httpd's config.c

The patch below deletes them and the resulting unused variables.  By way of
[a bit more] context, the last of these is:

   682  if (srv->srv_conf.return_uri_len != 0) {
   683  if ((srv->srv_conf.return_uri = get_data(p + s,
   684  srv->srv_conf.return_uri_len)) == NULL)
   685  goto fail;
   686  s += srv->srv_conf.return_uri_len;
   687  }
   688  
   689  return (0);
   690  
   691   fail:

Ross

===
RCS file: /cvs/src/usr.sbin/httpd/config.c,v
retrieving revision 1.57
diff -u -p -r1.57 config.c
--- config.c8 May 2019 19:57:45 -   1.57
+++ config.c2 Aug 2020 11:12:28 -
@@ -135,9 +135,7 @@ config_getreset(struct httpd *env, struc
 int
 config_getcfg(struct httpd *env, struct imsg *imsg)
 {
-   struct privsep  *ps = env->sc_ps;
struct ctl_flags cf;
-   unsigned int what;
 
if (IMSG_DATA_SIZE(imsg) != sizeof(cf))
return (0); /* ignore */
@@ -148,8 +146,6 @@ config_getcfg(struct httpd *env, struct 
env->sc_flags = cf.cf_flags;
memcpy(env->sc_tls_sid, cf.cf_tls_sid, sizeof(env->sc_tls_sid));
 
-   what = ps->ps_what[privsep_process];
-
if (privsep_process != PROC_PARENT)
proc_compose(env->sc_ps, PROC_PARENT,
IMSG_CFG_DONE, NULL, 0);
@@ -683,7 +679,6 @@ config_getserver(struct httpd *env, stru
if ((srv->srv_conf.return_uri = get_data(p + s,
srv->srv_conf.return_uri_len)) == NULL)
goto fail;
-   s += srv->srv_conf.return_uri_len;
}
 
return (0);

[Patch] httpd - delete unused function canonicalize_host()

[Ross L Richardson]

Function is unused and can go.

Ross
--

Index: httpd.c
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.c,v
retrieving revision 1.68
diff -u -p -r1.68 httpd.c
--- httpd.c 9 Sep 2018 21:06:51 -   1.68
+++ httpd.c 26 Jul 2020 13:07:26 -
@@ -552,59 +552,6 @@ expand_string(char *label, size_t len, c
 }
 
 const char *
-canonicalize_host(const char *host, char *name, size_t len)
-{
-   struct sockaddr_in   sin4;
-   struct sockaddr_in6  sin6;
-   size_t   i, j;
-   size_t   plen;
-   char c;
-
-   if (len < 2)
-   goto fail;
-
-   /*
-* Canonicalize an IPv4/6 address
-*/
-   if (inet_pton(AF_INET, host, ) == 1)
-   return (inet_ntop(AF_INET, , name, len));
-   if (inet_pton(AF_INET6, host, ) == 1)
-   return (inet_ntop(AF_INET6, , name, len));
-
-   /*
-* Canonicalize a hostname
-*/
-
-   /* 1. remove repeated dots and convert upper case to lower case */
-   plen = strlen(host);
-   memset(name, 0, len);
-   for (i = j = 0; i < plen; i++) {
-   if (j >= (len - 1))
-   goto fail;
-   c = tolower((unsigned char)host[i]);
-   if ((c == '.') && (j == 0 || name[j - 1] == '.'))
-   continue;
-   name[j++] = c;
-   }
-
-   /* 2. remove trailing dots */
-   for (i = j; i > 0; i--) {
-   if (name[i - 1] != '.')
-   break;
-   name[i - 1] = '\0';
-   j--;
-   }
-   if (j <= 0)
-   goto fail;
-
-   return (name);
-
- fail:
-   errno = EINVAL;
-   return (NULL);
-}
-
-const char *
 url_decode(char *url)
 {
char*p, *q;
Index: httpd.h
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
retrieving revision 1.147
diff -u -p -r1.147 httpd.h
--- httpd.h 25 Jul 2020 21:12:49 -  1.147
+++ httpd.h 26 Jul 2020 13:07:26 -
@@ -710,7 +710,6 @@ void event_again(struct event *, int, 
 int expand_string(char *, size_t, const char *, const char *);
 const char *url_decode(char *);
 char   *url_encode(const char *);
-const char *canonicalize_host(const char *, char *, size_t);
 const char *canonicalize_path(const char *, char *, size_t);
 size_t  path_info(char *);
 char   *escape_html(const char *);

[Patch] httpd.h - delete unused field and enum

[Ross L Richardson]

Field kv_type in struct kv is not used.  As that's the only use of
enum key_type, delete them both.

Ross


Index: httpd.h
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
retrieving revision 1.146
diff -u -p -r1.146 httpd.h
--- httpd.h 9 Feb 2020 09:44:04 -   1.146
+++ httpd.h 15 Jul 2020 04:45:16 -
@@ -121,24 +121,12 @@ struct ctl_flags {
uint8_t  cf_tls_sid[TLS_MAX_SESSION_ID_LENGTH];
 };
 
-enum key_type {
-   KEY_TYPE_NONE   = 0,
-   KEY_TYPE_COOKIE,
-   KEY_TYPE_HEADER,
-   KEY_TYPE_PATH,
-   KEY_TYPE_QUERY,
-   KEY_TYPE_URL,
-   KEY_TYPE_MAX
-};
-
 TAILQ_HEAD(kvlist, kv);
 RB_HEAD(kvtree, kv);
 
 struct kv {
char*kv_key;
char*kv_value;
-
-   enum key_typekv_type;
 
 #define KV_FLAG_INVALID 0x01
 #define KV_FLAG_GLOBBING0x02

[Patch] Be strict with fastcgi Status header

[Ross L Richardson]

The current code hides gross errors in the Status header generated by
fastcgi applications:
- Bogus codes are converted to 200.
- The presence of multple Status headers is not reported as an error.

IMHO, this doesn't really help anyone.

The patch below attempts to address both these concerns.

Ross


Index: server_fcgi.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
retrieving revision 1.81
diff -u -p -r1.81 server_fcgi.c
--- server_fcgi.c   9 Feb 2020 09:44:04 -   1.81
+++ server_fcgi.c   11 Jul 2020 11:08:53 -
@@ -141,7 +141,7 @@ server_fcgi(struct httpd *env, struct cl
memset(hbuf, 0, sizeof(hbuf));
clt->clt_fcgi.state = FCGI_READ_HEADER;
clt->clt_fcgi.toread = sizeof(struct fcgi_record_header);
-   clt->clt_fcgi.status = 200;
+   clt->clt_fcgi.status = 0; /* -1 = err; 0 = not set; ... */
clt->clt_fcgi.headersdone = 0;
 
if (clt->clt_srvevb != NULL)
@@ -574,7 +574,11 @@ server_fcgi_read(struct bufferevent *bev
clt->clt_fcgi.headersdone =
server_fcgi_getheaders(clt);
if (clt->clt_fcgi.headersdone) {
-   if (server_fcgi_header(clt,
+   if (clt->clt_fcgi.status == 0)
+   clt->clt_fcgi.status
+   = 200;
+   if (clt->clt_fcgi.status == -1
+   || server_fcgi_header(clt,
clt->clt_fcgi.status)
== -1) {
server_abort_http(clt,
@@ -797,12 +801,17 @@ server_fcgi_getheaders(struct client *cl
DPRINTF("%s: %s: %s", __func__, key, value);
 
if (strcasecmp("Status", key) == 0) {
-   value[strcspn(value, " \t")] = '\0';
-   code = (int)strtonum(value, 100, 600, );
-   if (errstr != NULL || server_httperror_byid(
-   code) == NULL)
-   code = 200;
-   clt->clt_fcgi.status = code;
+   if (clt->clt_fcgi.status != 0)
+   /* This is not the first status header. */
+   clt->clt_fcgi.status = -1;
+   else {
+   value[strcspn(value, " \t")] = '\0';
+   code = (int)strtonum(value, 100, 600, );
+   if (errstr != NULL || server_httperror_byid(
+   code) == NULL)
+   code = -1;
+   clt->clt_fcgi.status = code;
+   }
} else {
(void)kv_add(>http_headers, key, value);
}

[Patch] Make some functions static in httpd's server_fcgi.c

[Ross L Richardson]

Noticed when working on my next [more interesting] patch...

Various functions could/should be static.

Ross


Index: server_fcgi.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
retrieving revision 1.81
diff -u -p -r1.81 server_fcgi.c
--- server_fcgi.c   9 Feb 2020 09:44:04 -   1.81
+++ server_fcgi.c   11 Jul 2020 11:18:07 -
@@ -76,13 +76,13 @@ struct server_fcgi_param {
uint8_t buf[FCGI_RECORD_SIZE];
 };
 
-intserver_fcgi_header(struct client *, unsigned int);
-void   server_fcgi_read(struct bufferevent *, void *);
-intserver_fcgi_writeheader(struct client *, struct kv *, void *);
-intserver_fcgi_writechunk(struct client *);
-intserver_fcgi_getheaders(struct client *);
-intfcgi_add_param(struct server_fcgi_param *, const char *, const char *,
-   struct client *);
+static int server_fcgi_header(struct client *, unsigned int);
+static voidserver_fcgi_read(struct bufferevent *, void *);
+static int server_fcgi_writeheader(struct client *, struct kv *, void *);
+static int server_fcgi_writechunk(struct client *);
+static int server_fcgi_getheaders(struct client *);
+static int fcgi_add_param(struct server_fcgi_param *, const char *,
+   const char *, struct client *);
 
 int
 server_fcgi(struct httpd *env, struct client *clt)
@@ -453,7 +453,7 @@ fcgi_add_stdin(struct client *clt, struc
return (0);
 }
 
-int
+static int
 fcgi_add_param(struct server_fcgi_param *p, const char *key,
 const char *val, struct client *clt)
 {
@@ -509,7 +509,7 @@ fcgi_add_param(struct server_fcgi_param 
return (0);
 }
 
-void
+static void
 server_fcgi_read(struct bufferevent *bev, void *arg)
 {
uint8_t  buf[FCGI_RECORD_SIZE];
@@ -619,7 +619,7 @@ server_fcgi_read(struct bufferevent *bev
} while (len > 0);
 }
 
-int
+static int
 server_fcgi_header(struct client *clt, unsigned int code)
 {
struct server_config*srv_conf = clt->clt_srv_conf;
@@ -699,7 +699,7 @@ server_fcgi_header(struct client *clt, u
return (0);
 }
 
-int
+static int
 server_fcgi_writeheader(struct client *clt, struct kv *hdr, void *arg)
 {
struct server_fcgi_param*param = arg;
@@ -751,7 +751,7 @@ server_fcgi_writeheader(struct client *c
return (ret);
 }
 
-int
+static int
 server_fcgi_writechunk(struct client *clt)
 {
struct evbuffer *evb = clt->clt_srvevb;
@@ -776,7 +776,7 @@ server_fcgi_writechunk(struct client *cl
return (0);
 }
 
-int
+static int
 server_fcgi_getheaders(struct client *clt)
 {
struct http_descriptor  *resp = clt->clt_descresp;

[Patch] Fix mangled language in x509v3.cnf.5

[Ross L Richardson]

Fix a horribly mangled sentence.   Would "may" be more appropriate
than "can"?

Also, in the list of usages: for serverAuth and clientAuth, shouldn't
the word "web" be elided?

Ross


Index: x509v3.cnf.5
===
RCS file: /cvs/src/lib/libcrypto/man/x509v3.cnf.5,v
retrieving revision 1.6
diff -u -p -r1.6 x509v3.cnf.5
--- x509v3.cnf.56 Jun 2019 01:06:59 -   1.6
+++ x509v3.cnf.511 Jun 2020 11:48:34 -
@@ -186,8 +186,8 @@ keyUsage=digitalSignature, nonRepudiatio
 keyUsage=critical, keyCertSign
 .Ed
 .Ss Extended key usage
-This extensions consists of a list of usages indicating purposes for
-which the certificate public key can be used for.
+This extension consists of a list of purposes for
+which the certificate public key can be used.
 .Pp
 These can either be object short names or the dotted numerical form of OIDs.
 While any OID can be used, only certain values make sense.

[Patch] Correct the version of OpenSSH in 67.html

[Ross L Richardson]

Should be 8.3, shouldn't it?

Ross


Index: 67.html
===
RCS file: /cvs/www/67.html,v
retrieving revision 1.65
diff -u -p -r1.65 67.html
--- 67.html 11 May 2020 19:24:58 -  1.65
+++ 67.html 12 May 2020 04:45:33 -
@@ -1178,7 +1178,7 @@ and https://www.openbsd.org/arm
 
   
 
-OpenSSH 8.1
+OpenSSH 8.3
   
 New Features
   

[Patch 1 of 2] Update list of ssh key files checked in Xsession

[Ross L Richardson]

Noticed when testing a U2F key...

SSH1 is no longer supported, so checking for an "identity" private key
file is unhelpful.

[Has this been overlooked, or is there some reason it has been preserved?]

Ross



Index: Xsession.in
===
RCS file: /cvs/xenocara/app/xenodm/config/Xsession.in,v
retrieving revision 1.1
diff -u -p -r1.1 Xsession.in
--- Xsession.in 26 Jul 2017 21:14:54 -  1.1
+++ Xsession.in 25 Nov 2019 10:30:45 -
@@ -23,14 +23,13 @@ else
 fi
 
 # if we have private ssh key(s), start ssh-agent and add the key(s)
-id1=$HOME/.ssh/identity
-id2=$HOME/.ssh/id_dsa
-id3=$HOME/.ssh/id_rsa
-id4=$HOME/.ssh/id_ecdsa
-id5=$HOME/.ssh/id_ed25519
+id1=$HOME/.ssh/id_dsa
+id2=$HOME/.ssh/id_rsa
+id3=$HOME/.ssh/id_ecdsa
+id4=$HOME/.ssh/id_ed25519
 if [ -z "$SSH_AGENT_PID" ];
 then
-   if [ -x /usr/bin/ssh-agent ] && [ -f $id1 -o -f $id2 -o -f $id3 -o -f 
$id4 -o -f $id5 ];
+   if [ -x /usr/bin/ssh-agent ] && [ -f $id1 -o -f $id2 -o -f $id3 -o -f 
$id4 ];
then
eval `ssh-agent -s`
ssh-add < /dev/null

[Patch 2 of 2] Update list of ssh key files checked in Xsession

[Ross L Richardson]

Add checks for the (new) security key files...

Patch assumes previous patch has been applied.

Ross



--- Xsession.in.prevMon Nov 25 21:29:42 2019
+++ Xsession.in Mon Nov 25 21:35:11 2019
@@ -26,10 +26,12 @@ fi
 id1=$HOME/.ssh/id_dsa
 id2=$HOME/.ssh/id_rsa
 id3=$HOME/.ssh/id_ecdsa
-id4=$HOME/.ssh/id_ed25519
+id4=$HOME/.ssh/id_ecdsa_sk
+id5=$HOME/.ssh/id_ed25519
+id6=$HOME/.ssh/id_ed25519_sk
 if [ -z "$SSH_AGENT_PID" ];
 then
-   if [ -x /usr/bin/ssh-agent ] && [ -f $id1 -o -f $id2 -o -f $id3 -o -f 
$id4 ];
+   if [ -x /usr/bin/ssh-agent ] && [ -f $id1 -o -f $id2 -o -f $id3 -o -f 
$id4 -o -f $id5 -o -f $id6 ];
then
eval `ssh-agent -s`
ssh-add < /dev/null

Re: probable error in 66.html regarding rpki-client

[Ross L Richardson]

On Wed, Oct 16, 2019 at 09:34:26PM -0600, Theo de Raadt wrote:
> Ross L Richardson  wrote:
> 
> > 66.html claims that rpki-client is included, but:
> > - <https://man.openbsd.org/rpki-client.8> gives no result.
> > - The test system I just sysupgraded doesn't have it.
> > 
> > It's not actually linked to the build (in src/usr.sbin/Makefile), is it?
> 
> It isn't.  It wasn't ready.

Here's a patch, then...

Ross


Index: 66.html
===
RCS file: /cvs/www/66.html,v
retrieving revision 1.72
diff -u -p -r1.72 66.html
--- 66.html 17 Oct 2019 13:49:28 -  1.72
+++ 66.html 18 Oct 2019 06:48:48 -
@@ -500,8 +500,6 @@ to 6.6.
   always check for namespace collisions on table
   commands. Introduced 'pfctl -FR' to reset pfctl(8) settings to
   defaults.
-Imported Kristaps Dzonsons' RPKI
-  validator, https://man.openbsd.org/rpki-client.8;>rpki-client(8).
  https://man.openbsd.org/relayd.8;>relayd(8) now supports
   binary protocol health checking. See
   https://man.openbsd.org/relayd.conf.5;>relayd.conf(5).

probable error in 66.html regarding rpki-client

[Ross L Richardson]

66.html claims that rpki-client is included, but:
-  gives no result.
- The test system I just sysupgraded doesn't have it.

It's not actually linked to the build (in src/usr.sbin/Makefile), is it?

Ross

Re: smtp(1) certificate validation

[Ross L Richardson]

Eric,

On Tue, Sep 10, 2019 at 05:38:30PM +0200, Eric Faurot wrote:
> On Fri, Sep 06, 2019 at 08:41:21AM +0200, Eric Faurot wrote:
> > Hi,
> > 
> > This patch adds the missing bits for verifying the server certificate
> > in smtp(1).
> 
> Take two: now check the name(s) of the server certificate.
>[...]

With that patch [plus sthen@'s], smtp now behaves correctly for
my test cases.

Thanks very much,
Ross

Re: smtp(1) certificate validation

[Ross L Richardson]

On Fri, Sep 06, 2019 at 10:58:43PM +1000, Ross L Richardson wrote:
>[...]
> 
> For invalid certificates, the result sometimes does not correspond with
> that from nc(1):
> 
>   $ ./smtp -nv -s smtps://devio.us:https
>   trying host 74.81.181.124 port 443...
>   certificate validation error 10
>   connection error: Invalid server certificate
>   done...
>   $ nc -cvz devio.us https
>   Connection to devio.us 443 port [tcp/https] succeeded!
>   nc: tls handshake failed (certificate verification failed: certificate 
> has expired)
> 
>   $ ./smtp -nv -s smtps://www.nokia.com.au:https
>   trying host 180.150.2.201 port 443...
>   valid certificate
>   ^C
>   $ nc -cvz www.nokia.com.au https
>   Connection to www.nokia.com.au 443 port [tcp/https] succeeded!
>   nc: tls handshake failed (name `www.nokia.com.au' not present in server 
> certificate)
> 
>[...]

To be clear...

For proper verification, smtp(1) needs to check the name(s) of the
server certificate.


Ross

Re: smtp(1) certificate validation

[Ross L Richardson]

On Fri, Sep 06, 2019 at 08:41:21AM +0200, Eric Faurot wrote:
> Hi,
> 
> This patch adds the missing bits for verifying the server certificate
> in smtp(1).
>[...]

Whilst this is a great improvement, it doesn't seem to be quite right...


For sites with valid certificates, things seem OK:

$ ./smtp -nv -s smtps://smtp.fastmail.com
trying host 66.111.4.139 port 465...
valid certificate
connection ready...
connection closed...
trying host 66.111.4.140 port 465...
valid certificate
connection ready...
connection closed...
done...

$ ./smtp -nv -s smtps://mail.aussiebroadband.com.au
trying host 202.142.142.238 port 465...
valid certificate
connection ready...
connection closed...
trying host 202.142.142.239 port 465...
valid certificate
connection ready...
connection closed...
trying host 2403:5800:1:3::25 port 465...
valid certificate
connection ready...
connection closed...
trying host 2403:5800:1:4::25 port 465...
valid certificate
connection ready...
connection closed...
done...

$ ./smtp -nv -s smtps://mail.internode.on.net
trying host 203.16.214.182 port 465...
valid certificate
connection ready...
connection closed...
done...


[I don't know of any mail servers with invalid certs against which
I can test, so I've tried some web servers.]

For invalid certificates, the result sometimes does not correspond with
that from nc(1):

$ ./smtp -nv -s smtps://devio.us:https
trying host 74.81.181.124 port 443...
certificate validation error 10
connection error: Invalid server certificate
done...
$ nc -cvz devio.us https
Connection to devio.us 443 port [tcp/https] succeeded!
nc: tls handshake failed (certificate verification failed: certificate 
has expired)

$ ./smtp -nv -s smtps://www.nokia.com.au:https
trying host 180.150.2.201 port 443...
valid certificate
^C
$ nc -cvz www.nokia.com.au https
Connection to www.nokia.com.au 443 port [tcp/https] succeeded!
nc: tls handshake failed (name `www.nokia.com.au' not present in server 
certificate)

$ ./smtp -nv -s smtps://www.on.net:https
trying host 150.101.140.197 port 443...
valid certificate
^C
$ nc -cvz www.on.net https
Connection to www.on.net 443 port [tcp/https] succeeded!
nc: tls handshake failed (handshake failed: error:14004410:SSL 
routines:CONNECT_CR_SRVR_HELLO:sslv3 alert handshake failure)


Thanks,
Ross

[Patch] smtp(1) with proto "smtps" should default to port smtps/465

[Ross L Richardson]

...unless I'm very mistaken!

Ross


Index: smtpc.c
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpc.c,v
retrieving revision 1.6
diff -u -p -r1.6 smtpc.c
--- smtpc.c 2 Jul 2019 09:36:20 -   1.6
+++ smtpc.c 31 Aug 2019 11:48:17 -
@@ -229,7 +229,7 @@ parse_server(char *server)
else if (!strcmp(scheme, "smtps")) {
params.tls_req = TLS_SMTPS;
if (port == NULL)
-   port = "submission";
+   port = "smtps";
}
else if (!strcmp(scheme, "smtp")) {
}

[diff] www/libressl/papers.html - add video link

[Ross L Richardson]

The following corresponds with the events.html patch...

Ross


Index: papers.html
===
RCS file: /cvs/www/libressl/papers.html,v
retrieving revision 1.16
diff -u -p -r1.16 papers.html
--- papers.html 13 Jun 2019 07:34:36 -  1.16
+++ papers.html 22 Jun 2019 04:20:59 -
@@ -19,7 +19,7 @@ Presentations and Papers
 
 https://www.openbsd.org/papers/bsdcan2019-tls13.pdf;>Design and 
verification of the TLS 1.3 handshake state machine in LibreSSL
 by Theo Buehler
-
+ (https://www.youtube.com/watch?v=gvmGfpMgny4;>video)
 
 
 Presentation: FSec 2015

[diff] events.html - add 2 more BSDCan 2019 videos

[Ross L Richardson]

Probably in acceptable form :-)

[Patch for www/libressl/papers.html should follow.]

Ross


Index: events.html
===
RCS file: /cvs/www/events.html,v
retrieving revision 1.1174
diff -u -p -r1.1174 events.html
--- events.html 18 Jun 2019 15:01:52 -  1.1174
+++ events.html 22 Jun 2019 04:09:17 -
@@ -68,7 +68,9 @@ May 15-18, 2019, Ottawa, Canada.
 (http://bhyvecon.org/;>bhyvecon Ottawa 2019)
 Bob Beck -
 https://github.com/bob-beck/libtls/blob/master/TUTORIAL.md;>libtls 
for beginners conference tutorial
-Theo Buehler - Design and verification of the TLS 1.3 handshake state 
machine in LibreSSL (slides)
+Theo Buehler - Design and verification of the TLS 1.3 handshake state
+  machine in LibreSSL (slides,
+  https://www.youtube.com/watch?v=MCVIBwGOwNY;>video)
 Florian Obser - https://man.openbsd.org/unwind.8;>unwind(8)
   a privilege-separated, validating DNS recursive nameserver for every laptop
   (slides,
@@ -82,7 +84,8 @@ May 15-18, 2019, Ottawa, Canada.
   (slides,
   https://www.youtube.com/watch?v=s6rAXaHylFM;>video)
 Bob Beck
-Unveil in OpenBSD
+  Unveil in OpenBSD
+  (https://www.youtube.com/watch?v=gvmGfpMgny4;>video)
 Jan Klemkow - Network booted OpenBSD Workstations
   (slides,
   https://www.youtube.com/watch?v=kFqHXfWEB4o;>video)

[diff] events.html - video links for BSDCan 2019

[Ross L Richardson]

The patch below just adds links for the videos currently available.

Notes:
- Lines were already > 80 characters, so I haven't folded.
- Links are to the context of the BSDCan 2019 playlist (which may not
  be what is wanted).

Ross


Index: events.html
===
RCS file: /cvs/www/events.html,v
retrieving revision 1.1173
diff -u -p -r1.1173 events.html
--- events.html 1 Jun 2019 23:12:47 -   1.1173
+++ events.html 18 Jun 2019 10:25:47 -
@@ -69,13 +69,13 @@ May 15-18, 2019, Ottawa, Canada.
 Bob Beck -
 https://github.com/bob-beck/libtls/blob/master/TUTORIAL.md;>libtls 
for beginners conference tutorial
 Theo Buehler - Design and verification of the TLS 1.3 handshake state 
machine in LibreSSL (slides)
-Florian Obser - https://man.openbsd.org/unwind.8;>unwind(8) a 
privilege-separated, validating DNS recursive nameserver for every laptop (slides)
-Stefan Sperling - Building an accessible OpenBSD laptop (Enabling secure 
and functional computing for a person with severe disabilities) (slides)
+Florian Obser - https://man.openbsd.org/unwind.8;>unwind(8) a 
privilege-separated, validating DNS recursive nameserver for every laptop (slides, https://www.youtube.com/watch?v=88SoI49nO4olist=PLeF8ZihVdpFegPoAKppaDSoYmsBvpnSZvindex=8t=0s;>video)
+Stefan Sperling - Building an accessible OpenBSD laptop (Enabling secure 
and functional computing for a person with severe disabilities) (slides, https://www.youtube.com/watch?v=Ma_Y1hVmK8olist=PLeF8ZihVdpFegPoAKppaDSoYmsBvpnSZvindex=6t=0s;>video)
 Antoine Jacoutot - https://man.openbsd.org/syspatch.8;>syspatch(8), The Boring Healing 
Potion (slides)
-Alexander Bluhm - Measuring Performance on OpenBSD (slides)
+Alexander Bluhm - Measuring Performance on OpenBSD (slides, https://www.youtube.com/watch?v=s6rAXaHylFMlist=PLeF8ZihVdpFegPoAKppaDSoYmsBvpnSZvindex=15t=0s;>video)
 Bob Beck
 Unveil in OpenBSD
-Jan Klemkow - Network booted OpenBSD Workstations (slides)
+Jan Klemkow - Network booted OpenBSD Workstations (slides, https://www.youtube.com/watch?v=kFqHXfWEB4olist=PLeF8ZihVdpFegPoAKppaDSoYmsBvpnSZvindex=19t=0s;>video)
 
 
 

Re: [Patch] Specify unit for 'every' in ifstated.conf.5

[Ross L Richardson]

On Fri, Dec 07, 2018 at 06:42:51AM +, Jason McIntyre wrote:
>[...]
> 
> morning.
> 
> i'm afraid i think this reads quite poorly - it is hard to actually
> understand what is meant.
> 
> here's the thing - someone has already written the text. if we want to
> change the author's text, i think we need to demonstrate that their text
> is wrong, inaccurate, or somehow misleading.
> 
> so far, i don;t believe anyone has done this. so i am reluctant to
> change the author's words.
>[...]

Fair enough.

The reason I couldn't make it work with "frequency" is that, as Otto
indicated, the second is not a unit of measurement for a frequency.

"tests / second" would be an appropriate unit for a frequency, but that
is _not_ what we are specifying when using "every".  Rather, we are
specifying a "period" - the interval between occurrences of tests.


I'm not trying to complicate this, just trying to explain why "frequency"
is an incorrect term.

Having stated my case, I'm happy to leave the wording to others (if that's
what's preferred).

Thanks,
Ross

Re: [Patch] Specify unit for 'every' in ifstated.conf.5

[Ross L Richardson]

Whilst I agree with Otto (and others), how about making it really simple
by avoiding technical terms?

Ross


Index: ifstated.conf.5
===
RCS file: /cvs/src/usr.sbin/ifstated/ifstated.conf.5,v
retrieving revision 1.13
diff -u -p -r1.13 ifstated.conf.5
--- ifstated.conf.5 18 Jun 2018 06:04:25 -  1.13
+++ ifstated.conf.5 7 Dec 2018 02:59:43 -
@@ -90,7 +90,8 @@ interfaces this equals the init state.
 .Pp
 In contrast to link state tests, external tests must be run periodically to
 evaluate their status.
-The frequency at which an external test is run has to be set with the
+The interval in seconds between (starts of) runs of an external test is set
+with the
 .Ar every
 keyword.
 .Pp

[Patch] Specify unit for 'every' in ifstated.conf.5

[Ross L Richardson]

The number is in seconds, but that's currently not specified.

Wording which preserved "frequency" but made sense with "seconds"
eluded me, so I changed things to refer to "interval".


Ross



Index: ifstated.conf.5
===
RCS file: /cvs/src/usr.sbin/ifstated/ifstated.conf.5,v
retrieving revision 1.13
diff -u -p -r1.13 ifstated.conf.5
--- ifstated.conf.5 18 Jun 2018 06:04:25 -  1.13
+++ ifstated.conf.5 6 Dec 2018 11:12:44 -
@@ -90,7 +90,8 @@ interfaces this equals the init state.
 .Pp
 In contrast to link state tests, external tests must be run periodically to
 evaluate their status.
-The frequency at which an external test is run has to be set with the
+The interval in seconds between invocations of an external test has to
+be set with the
 .Ar every
 keyword.
 .Pp

[3/3] acme-client - style(9) cleanup in parse.y

[Ross L Richardson]

This diff contains only style(9) cleanup:
- mainly "return (x);" => "return x;"
- a few "if (x)" ==> "if (x != something)"

No change to parse.o


Ross


Index: parse.y
===
RCS file: /cvs/src/usr.sbin/acme-client/parse.y,v
retrieving revision 1.26
diff -u -p -r1.26 parse.y
--- parse.y 29 Jul 2018 12:46:31 -  1.26
+++ parse.y 3 Aug 2018 12:11:30 -
@@ -402,7 +402,7 @@ yyerror(const char *fmt, ...)
 int
 kw_cmp(const void *k, const void *e)
 {
-   return (strcmp(k, ((const struct keywords *)e)->k_name));
+   return strcmp(k, ((const struct keywords *)e)->k_name);
 }
 
 int
@@ -431,10 +431,10 @@ lookup(char *s)
p = bsearch(s, keywords, sizeof(keywords)/sizeof(keywords[0]),
sizeof(keywords[0]), kw_cmp);
 
-   if (p)
-   return (p->k_val);
+   if (p != NULL)
+   return p->k_val;
else
-   return (STRING);
+   return STRING;
 }
 
 #defineSTART_EXPAND1
@@ -460,7 +460,7 @@ igetc(void)
else
break;
}
-   return (c);
+   return c;
 }
 
 int
@@ -474,9 +474,9 @@ lgetc(int quotec)
"quoted string");
if (file == topfile || popfile() == EOF)
return (EOF);
-   return (quotec);
+   return quotec;
}
-   return (c);
+   return c;
}
 
while ((c = igetc()) == '\\') {
@@ -497,7 +497,7 @@ lgetc(int quotec)
 */
if (file->eof_reached == 0) {
file->eof_reached = 1;
-   return ('\n');
+   return '\n';
}
while (c == EOF) {
if (file == topfile || popfile() == EOF)
@@ -505,7 +505,7 @@ lgetc(int quotec)
c = igetc();
}
}
-   return (c);
+   return c;
 }
 
 void
@@ -539,7 +539,7 @@ findeol(void)
if (c == EOF)
break;
}
-   return (ERROR);
+   return ERROR;
 }
 
 int
@@ -562,11 +562,11 @@ top:
if (c == '$' && !expanding) {
while (1) {
if ((c = lgetc(0)) == EOF)
-   return (0);
+   return 0;
 
if (p + 1 >= buf + sizeof(buf) - 1) {
yyerror("string too long");
-   return (findeol());
+   return findeol();
}
if (isalnum(c) || c == '_') {
*p++ = c;
@@ -579,7 +579,7 @@ top:
val = symget(buf);
if (val == NULL) {
yyerror("macro '%s' not defined", buf);
-   return (findeol());
+   return findeol();
}
p = val + strlen(val) - 1;
lungetc(DONE_EXPAND);
@@ -597,13 +597,13 @@ top:
quotec = c;
while (1) {
if ((c = lgetc(quotec)) == EOF)
-   return (0);
+   return 0;
if (c == '\n') {
file->lineno++;
continue;
} else if (c == '\\') {
if ((next = lgetc(quotec)) == EOF)
-   return (0);
+   return 0;
if (next == quotec || c == ' ' || c == '\t')
c = next;
else if (next == '\n') {
@@ -616,18 +616,18 @@ top:
break;
} else if (c == '\0') {
yyerror("syntax error");
-   return (findeol());
+   return findeol();
}
if (p + 1 >= buf + sizeof(buf) - 1) {
yyerror("string too long");
-   return (findeol());
+   return findeol();
}
*p++ = c;
}
yylval.v.string = strdup(buf);
if (yylval.v.string == NULL)
err(EXIT_FAILURE, "%s", __func__);
-   return (STRING);
+   return STRING;
}
 
 #define allowed_to_end_number(x) \
@@ -638,7 +638,7 @@ top:
*p++ = c;
if ((unsigned)(p-buf) >= sizeof(buf)) {
yyerror("string too long");
-   

[2/3] acme-client - correct "sign with" error messages in parse.y

[Ross L Richardson]

It appears that, sometime in the early development of the config
file stuff, there was a different syntax for what is now "sign with".
A couple of error messages refer to "use" [but should not].

Ross



Index: parse.y
===
RCS file: /cvs/src/usr.sbin/acme-client/parse.y,v
retrieving revision 1.26
diff -u -p -r1.26 parse.y
--- parse.y 29 Jul 2018 12:46:31 -  1.26
+++ parse.y 3 Aug 2018 12:05:16 -
@@ -326,13 +326,13 @@ domainoptsl   : ALTERNATIVE NAMES '{' altn
| SIGN WITH STRING {
char *s;
if (domain->auth != NULL) {
-   yyerror("duplicate use");
+   yyerror("duplicate sign with");
YYERROR;
}
if ((s = strdup($3)) == NULL)
err(EXIT_FAILURE, "strdup");
if (authority_find(conf, s) == NULL) {
-   yyerror("use: unknown authority");
+   yyerror("sign with: unknown authority");
free(s);
YYERROR;
}

[1/3] acme-client - correct error message in parse.y

[Ross L Richardson]

[Previously submitted large diff now being sent in three more-readily
digestible pieces...]

The error message looks to have been copied/pasted from the earlier
DOMAIN CHAIN CERT STRING case, but not edited.


Ross



Index: parse.y
===
RCS file: /cvs/src/usr.sbin/acme-client/parse.y,v
retrieving revision 1.26
diff -u -p -r1.26 parse.y
--- parse.y 29 Jul 2018 12:46:31 -  1.26
+++ parse.y 3 Aug 2018 12:03:00 -
@@ -311,7 +311,7 @@ domainoptsl : ALTERNATIVE NAMES '{' altn
| DOMAIN FULL CHAIN CERT STRING {
char *s;
if (domain->fullchain != NULL) {
-   yyerror("duplicate chain");
+   yyerror("duplicate full chain");
YYERROR;
}
if ((s = strdup($5)) == NULL)

acme-client.conf.5 - "sign with" setting is (also) optional

[Ross L Richardson]

sign with 
is the final optional setting not yet documented

If absent, defaults to the first authority in the config.

Ross


Index: acme-client.conf.5
===
RCS file: /cvs/src/usr.sbin/acme-client/acme-client.conf.5,v
retrieving revision 1.14
diff -u -p -r1.14 acme-client.conf.5
--- acme-client.conf.5  2 Aug 2018 14:40:38 -   1.14
+++ acme-client.conf.5  3 Aug 2018 11:47:49 -
@@ -147,6 +147,7 @@ is specified.
 The certificate authority (as declared above in the
 .Sx AUTHORITIES
 section) to use.
+If this setting is absent, the first authority specified is used.
 .It Ic challengedir Ar path
 The directory in which the challenge file will be stored.
 If it is not specified, a default of

acme-client.conf.5 - domain chain certificate is (also) optional

[Ross L Richardson]

domain chain certificate 
is one of the two remaining optional settings not yet documented

Ross


Index: acme-client.conf.5
===
RCS file: /cvs/src/usr.sbin/acme-client/acme-client.conf.5,v
retrieving revision 1.14
diff -u -p -r1.14 acme-client.conf.5
--- acme-client.conf.5  2 Aug 2018 14:40:38 -   1.14
+++ acme-client.conf.5  3 Aug 2018 11:45:16 -
@@ -129,6 +129,7 @@ that will be returned by the certificate
 It needs to be in the same directory as the
 .Ar domain certificate
 (or in a subdirectory) and can be specified as a relative or absolute path.
+This setting is optional.
 .It Ic domain full chain certificate Ar file
 The filename in which to store the full certificate chain
 that will be returned by the certificate authority.

acme-client.1 - Change "TLS certificate" to "X.509 certificate"

[Ross L Richardson]

It's an "X.509 certificate" rather than a "TLS certificate".
As pointed out by sthen@, TLS isn't the only possible use.



Index: acme-client.1
===
RCS file: /cvs/src/usr.sbin/acme-client/acme-client.1,v
retrieving revision 1.24
diff -u -p -r1.24 acme-client.1
--- acme-client.1   13 Jun 2018 15:08:24 -  1.24
+++ acme-client.1   2 Aug 2018 04:41:05 -
@@ -56,7 +56,7 @@ The domain name.
 looks in its configuration for a
 .Ar domain
 section corresponding to the domain given as command line argument.
-It then uses that configuration to retrieve a TLS certificate.
+It then uses that configuration to retrieve an X.509 certificate.
 If the certificate already exists and is less than 30 days from expiry,
 .Nm
 will attempt to refresh the signature.

acme-client.1 - fix mangled sentence and stray plural

[Ross L Richardson]

This is the first of several diffs containing separate bits of the
earlier combined diff.

"X509" to "X.509" for correctness and consistency with acme-client.conf.5


Index: acme-client.1
===
RCS file: /cvs/src/usr.sbin/acme-client/acme-client.1,v
retrieving revision 1.24
diff -u -p -r1.24 acme-client.1
--- acme-client.1   13 Jun 2018 15:08:24 -  1.24
+++ acme-client.1   2 Aug 2018 04:29:36 -
@@ -44,7 +44,7 @@ Specify an alternative configuration fil
 .It Fl n
 No operation: check and print configuration.
 .It Fl r
-Revoke the X509 certificate found in the certificates.
+Revoke the X.509 certificate.
 .It Fl v
 Verbose operation.
 Specify twice to also trace communication and data transfers.
@@ -110,7 +110,7 @@ as above:
 .Pp
 A daily
 .Xr cron 8
-job can renew the certificates:
+job can renew the certificate:
 .Pp
 .Dl acme-client example.com && rcctl reload httpd
 .Sh SEE ALSO

[diff] Improvements for acme-client.1, acme-client.conf.5

[Ross L Richardson]

acme-client.1 changes are largely similar to my previous attempt
[off-list] with some feedback from jsing@ [thanks!]
The example has been changed from "example.com" to "www.example.com"
in an attempt to make it clear that "domain" is a misnomer.

acme-client.conf.5 stuff is mostly documenting which cert settings
are optional.  I've removed "(CAs)" as the abbreviation is unused
later.  "(CN)" has been added because, although unused elsewhere
in the man page, it is likely to be seen be anyone examining the
subject of a certificate.

Ross



Index: acme-client.1
===
RCS file: /cvs/src/usr.sbin/acme-client/acme-client.1,v
retrieving revision 1.24
diff -u -p -r1.24 acme-client.1
--- acme-client.1   13 Jun 2018 15:08:24 -  1.24
+++ acme-client.1   1 Aug 2018 11:44:27 -
@@ -34,9 +34,9 @@ Automatic Certificate Management Environ
 The options are as follows:
 .Bl -tag -width Ds
 .It Fl A
-Create a new RSA account key if one does not already exist.
+Create a new RSA account key pair if one does not already exist.
 .It Fl D
-Create a new RSA domain key if one does not already exist.
+Create a new RSA certificate key pair if one does not already exist.
 .It Fl F
 Force updating the certificate signature even if it's too soon.
 .It Fl f Ar configfile
@@ -44,27 +44,28 @@ Specify an alternative configuration fil
 .It Fl n
 No operation: check and print configuration.
 .It Fl r
-Revoke the X509 certificate found in the certificates.
+Revoke the certificate.
 .It Fl v
 Verbose operation.
 Specify twice to also trace communication and data transfers.
 .It Ar domain
-The domain name.
+The primary DNS name.
 .El
 .Pp
 .Nm
 looks in its configuration for a
 .Ar domain
-section corresponding to the domain given as command line argument.
-It then uses that configuration to retrieve a TLS certificate.
+section corresponding to the name given as command line argument.
+It then uses that configuration to retrieve an X509 certificate.
 If the certificate already exists and is less than 30 days from expiry,
 .Nm
 will attempt to refresh the signature.
-Before a certificate can be requested, an account key needs to be
+Before a certificate can be requested, an account key pair needs to be
 created using the
 .Fl A
 argument.
-The first time a certificate is requested, the RSA key needs to be created with
+The first time a certificate is requested, an RSA key pair needs to be
+created for it with
 .Fl D .
 .Pp
 Challenges are used to verify that the submitter has access to the
@@ -98,21 +99,21 @@ Default challengedir.
 returns 1 on failure, 2 if the certificates didn't change (up to date),
 or 0 if certificates were changed (revoked or updated).
 .Sh EXAMPLES
-To initialize a new account and Domain key:
+To initialize new account and certificate key pairs:
 .Pp
-.Dl # acme-client -vAD example.com
+.Dl # acme-client -vAD www.example.com
 .Pp
-To create and submit a new key for a single domain, assuming that the
+To create and submit a key pair for a certificate, assuming that the
 web server has already been configured to map the challenge directory
 as above:
 .Pp
-.Dl # acme-client -vD example.com
+.Dl # acme-client -vD www.example.com
 .Pp
 A daily
 .Xr cron 8
-job can renew the certificates:
+job can renew the certificate:
 .Pp
-.Dl acme-client example.com && rcctl reload httpd
+.Dl acme-client www.example.com && rcctl reload httpd
 .Sh SEE ALSO
 .Xr openssl 1 ,
 .Xr acme-client.conf 5 ,
Index: acme-client.conf.5
===
RCS file: /cvs/src/usr.sbin/acme-client/acme-client.conf.5,v
retrieving revision 1.13
diff -u -p -r1.13 acme-client.conf.5
--- acme-client.conf.5  8 Jul 2018 15:41:17 -   1.13
+++ acme-client.conf.5  1 Aug 2018 11:44:27 -
@@ -32,7 +32,7 @@ file is divided into the following main 
 User-defined variables may be defined and used later, simplifying the
 configuration file.
 .It Sy Authorities
-Certificate authorities (CAs) that can be contacted via ACME.
+Certificate authorities that can be contacted via ACME.
 .It Sy Domains
 Certificate specifications.
 .El
@@ -105,27 +105,33 @@ The certificates to be obtained through 
 .It Ic domain Ar name Brq ...
 Each domain section begins with the
 .Ic domain
-keyword followed by the name to be used as the common name component
+keyword followed by the primary DNS name for the certificate.
+This name is used as the Common Name (CN) component
 of the subject of the X.509 certificate.
 .El
 .Pp
 It is followed by a block of options enclosed in curly brackets:
 .Bl -tag -width Ds
 .It Ic alternative names Brq ...
-Specify a list of alternative names for which the certificate will be valid.
-The common name is included automatically if this option is present,
+Specify a list of alternative DNS names for which the certificate will
+be valid.
+The primary DNS name is included automatically if this option is present,
 but there 

[diff] acme-client - fix err messages and style in parse.y

[Ross L Richardson]

[The non-style(9) parts of this were sent previously off-list to a few.]

The diff:
- fixes error messages
- one copy/pasto
- two with old(?) syntax "use" rather than "sign with"
- applies some style(9)
- lots of "return (x);" ==> "return x;"
- a few "if (x)" ==> "if (x != something)"

Ross


Index: parse.y
===
RCS file: /cvs/src/usr.sbin/acme-client/parse.y,v
retrieving revision 1.26
diff -u -p -r1.26 parse.y
--- parse.y 29 Jul 2018 12:46:31 -  1.26
+++ parse.y 30 Jul 2018 11:56:47 -
@@ -311,7 +311,7 @@ domainoptsl : ALTERNATIVE NAMES '{' altn
| DOMAIN FULL CHAIN CERT STRING {
char *s;
if (domain->fullchain != NULL) {
-   yyerror("duplicate chain");
+   yyerror("duplicate full chain");
YYERROR;
}
if ((s = strdup($5)) == NULL)
@@ -326,13 +326,13 @@ domainoptsl   : ALTERNATIVE NAMES '{' altn
| SIGN WITH STRING {
char *s;
if (domain->auth != NULL) {
-   yyerror("duplicate use");
+   yyerror("duplicate sign with");
YYERROR;
}
if ((s = strdup($3)) == NULL)
err(EXIT_FAILURE, "strdup");
if (authority_find(conf, s) == NULL) {
-   yyerror("use: unknown authority");
+   yyerror("sign with: unknown authority");
free(s);
YYERROR;
}
@@ -402,7 +402,7 @@ yyerror(const char *fmt, ...)
 int
 kw_cmp(const void *k, const void *e)
 {
-   return (strcmp(k, ((const struct keywords *)e)->k_name));
+   return strcmp(k, ((const struct keywords *)e)->k_name);
 }
 
 int
@@ -431,10 +431,10 @@ lookup(char *s)
p = bsearch(s, keywords, sizeof(keywords)/sizeof(keywords[0]),
sizeof(keywords[0]), kw_cmp);
 
-   if (p)
-   return (p->k_val);
+   if (p != NULL)
+   return p->k_val;
else
-   return (STRING);
+   return STRING;
 }
 
 #defineSTART_EXPAND1
@@ -460,7 +460,7 @@ igetc(void)
else
break;
}
-   return (c);
+   return c;
 }
 
 int
@@ -474,9 +474,9 @@ lgetc(int quotec)
"quoted string");
if (file == topfile || popfile() == EOF)
return (EOF);
-   return (quotec);
+   return quotec;
}
-   return (c);
+   return c;
}
 
while ((c = igetc()) == '\\') {
@@ -497,7 +497,7 @@ lgetc(int quotec)
 */
if (file->eof_reached == 0) {
file->eof_reached = 1;
-   return ('\n');
+   return '\n';
}
while (c == EOF) {
if (file == topfile || popfile() == EOF)
@@ -505,7 +505,7 @@ lgetc(int quotec)
c = igetc();
}
}
-   return (c);
+   return c;
 }
 
 void
@@ -539,7 +539,7 @@ findeol(void)
if (c == EOF)
break;
}
-   return (ERROR);
+   return ERROR;
 }
 
 int
@@ -562,11 +562,11 @@ top:
if (c == '$' && !expanding) {
while (1) {
if ((c = lgetc(0)) == EOF)
-   return (0);
+   return 0;
 
if (p + 1 >= buf + sizeof(buf) - 1) {
yyerror("string too long");
-   return (findeol());
+   return findeol();
}
if (isalnum(c) || c == '_') {
*p++ = c;
@@ -579,7 +579,7 @@ top:
val = symget(buf);
if (val == NULL) {
yyerror("macro '%s' not defined", buf);
-   return (findeol());
+   return findeol();
}
p = val + strlen(val) - 1;
lungetc(DONE_EXPAND);
@@ -597,13 +597,13 @@ top:
quotec = c;
while (1) {
if ((c = lgetc(quotec)) == EOF)
-   return (0);
+   return 0;
if (c == '\n') {
file->lineno++;
continue;
} else if (c == '\\') {

[diff] acme-client - clean up main.c

[Ross L Richardson]

[This diff is very similar to one I sent previously off-list to a few.]

Just some style(9) and simple cleanup:
- order getopt string and switch cases
- add space between "if" and "("
- wrap a long line
- return rather than exit() from main()
- move chngdir == NULL test to where it belongs


Questions:

- is there any reason
   404  /* Jail: sandbox, file-system, user. */
   405  
   406  if (pledge("stdio", NULL) == -1) {
   407  warn("pledge");
   408  exit(EXIT_FAILURE);
   409  }
   410  
  shouldn't just use err() ?
  If so, then the exit() should still be return, shouldn't it?

- should the exit() calls from the various forked child processes
  be _exit()?



Index: main.c
===
RCS file: /cvs/src/usr.sbin/acme-client/main.c,v
retrieving revision 1.36
diff -u -p -r1.36 main.c
--- main.c  27 Nov 2017 01:58:52 -  1.36
+++ main.c  30 Jul 2018 01:12:45 -
@@ -56,21 +56,24 @@ main(int argc, char *argv[])
struct domain_c *domain = NULL;
struct altname_c*ac;
 
-   while ((c = getopt(argc, argv, "FADrvnf:")) != -1)
+   while ((c = getopt(argc, argv, "ADFf:nrv")) != -1)
switch (c) {
-   case 'f':
-   if ((conffile = strdup(optarg)) == NULL)
-   err(EXIT_FAILURE, "strdup");
-   break;
-   case 'F':
-   force = 1;
-   break;
case 'A':
popts |= ACME_OPT_NEWACCT;
break;
case 'D':
popts |= ACME_OPT_NEWDKEY;
break;
+   case 'F':
+   force = 1;
+   break;
+   case 'f':
+   if ((conffile = strdup(optarg)) == NULL)
+   err(EXIT_FAILURE, "strdup");
+   break;
+   case 'n':
+   popts |= ACME_OPT_CHECK;
+   break;
case 'r':
revocate = 1;
break;
@@ -78,9 +81,6 @@ main(int argc, char *argv[])
verbose = verbose ? 2 : 1;
popts |= ACME_OPT_VERBOSE;
break;
-   case 'n':
-   popts |= ACME_OPT_CHECK;
-   break;
default:
goto usage;
}
@@ -90,7 +90,7 @@ main(int argc, char *argv[])
 
/* parse config file */
if ((conf = parse_config(conffile, popts)) == NULL)
-   exit(EXIT_FAILURE);
+   return EXIT_FAILURE;
 
argc -= optind;
argv += optind;
@@ -127,7 +127,7 @@ main(int argc, char *argv[])
err(EXIT_FAILURE, "basename");
}
 
-   if(domain->chain != NULL) {
+   if (domain->chain != NULL) {
if ((chainfile = basename(domain->chain)) != NULL) {
if ((chainfile = strdup(chainfile)) == NULL)
err(EXIT_FAILURE, "strdup");
@@ -135,7 +135,7 @@ main(int argc, char *argv[])
err(EXIT_FAILURE, "basename");
}
 
-   if(domain->fullchain != NULL) {
+   if (domain->fullchain != NULL) {
if ((fullchainfile = basename(domain->fullchain)) != NULL) {
if ((fullchainfile = strdup(fullchainfile)) == NULL)
err(EXIT_FAILURE, "strdup");
@@ -160,14 +160,12 @@ main(int argc, char *argv[])
/* XXX replace with existance check in parse.y */
err(EXIT_FAILURE, "no account key in config?");
}
-   if (domain->challengedir == NULL)
-   chngdir = strdup(WWW_DIR);
-   else
+   if (domain->challengedir == NULL) {
+   if ((chngdir = strdup(WWW_DIR)) == NULL)
+   err(EXIT_FAILURE, "strdup");
+   } else
chngdir = domain->challengedir;
 
-   if (chngdir == NULL)
-   err(EXIT_FAILURE, "strdup");
-
/*
 * Do some quick checks to see if our paths exist.
 * This will be done in the children, but we might as well check
@@ -185,7 +183,8 @@ main(int argc, char *argv[])
if (!(popts & ACME_OPT_NEWDKEY) && access(domain->key, R_OK) == -1) {
warnx("%s: domain key file must exist", domain->key);
ne++;
-   } else if ((popts & ACME_OPT_NEWDKEY) && access(domain->key, R_OK) != 
-1) {
+   } else if ((popts & ACME_OPT_NEWDKEY) && access(domain->key, R_OK)
+   != -1) {
dodbg("%s: domain key exists (not creating)", domain->key);
popts &= ~ACME_OPT_NEWDKEY;
}
@@ -204,10 +203,10 @@ main(int argc, char *argv[])
   

[diff] acme-client - improve printing of alt names (with -n)

[Ross L Richardson]

Simple diff so that instead of printing
...
alternative names {  httpd.example.com, secure.example.com}
...
we get
...
alternative names { httpd.example.com, secure.example.com }
...

Ross


Index: parse.y
===
RCS file: /cvs/src/usr.sbin/acme-client/parse.y,v
retrieving revision 1.25
diff -u -p -r1.25 parse.y
--- parse.y 11 Jul 2018 07:39:22 -  1.25
+++ parse.y 29 Jul 2018 05:31:57 -
@@ -998,14 +998,14 @@ print_config(struct acme_conf *xconf)
printf("domain %s {\n", d->domain);
TAILQ_FOREACH(ac, >altname_list, entry) {
if (!f)
-   printf("\talternative names { ");
+   printf("\talternative names {");
if (ac->domain != NULL) {
printf("%s%s", f ? ", " : " ", ac->domain);
f = 1;
}
}
if (f)
-   printf("}\n");
+   printf(" }\n");
if (d->key != NULL)
printf("\tdomain key \"%s\"\n", d->key);
if (d->cert != NULL)

Re: [diff] acme-client - remove NULL checks before _free() calls

[Ross L Richardson]

On Sat, Jul 28, 2018 at 05:09:50PM +0200, Theo Buehler wrote:
>[...]
> I'll commit this tomorrow if no-one commits it earlier with my ok.
> 
> > [Note that the behaviour of X509_REQ_free(NULL) and X509_NAME_free(NULL)
> > is not documented in the man pages, so those calls in keyproc.c are not
> > modified in this diff.]
> 
> They are NULL safe as they're just wrappers around ASN1_item_free().
> (see also the regression test in /usr/src/regress/lib/libcrypto/free/)
> 
> So both, a diff that fixes the manuals, and a diff to remove the NULL
> checks in keyproc.c would be great.
> 

diffs below:
- fix the manuals [as suggested, plus
  for X509_REQ_INFO_free() whilst there]
- remove the NULL checks from keyproc.c [plus add a style(9)
  fix whilst there]

Thanks,
Ross

Index: X509_NAME_new.3
===
RCS file: /cvs/src/lib/libcrypto/man/X509_NAME_new.3,v
retrieving revision 1.5
diff -u -p -r1.5 X509_NAME_new.3
--- X509_NAME_new.3 27 Mar 2018 17:35:50 -  1.5
+++ X509_NAME_new.3 29 Jul 2018 05:04:20 -
@@ -62,6 +62,11 @@ frees
 and all the
 .Vt X509_NAME_ENTRY
 objects contained in it.
+If
+.Fa name
+is a
+.Dv NULL
+pointer, no action occurs.
 .Sh RETURN VALUES
 .Fn X509_NAME_new
 returns a new
Index: X509_REQ_new.3
===
RCS file: /cvs/src/lib/libcrypto/man/X509_REQ_new.3,v
retrieving revision 1.4
diff -u -p -r1.4 X509_REQ_new.3
--- X509_REQ_new.3  27 Mar 2018 17:35:50 -  1.4
+++ X509_REQ_new.3  29 Jul 2018 05:04:20 -
@@ -47,6 +47,11 @@ information about the signature algorith
 .Fn X509_REQ_free
 frees
 .Fa req .
+If
+.Fa req
+is a
+.Dv NULL
+pointer, no action occurs.
 .Pp
 .Fn X509_REQ_INFO_new
 allocates and initializes an empty
@@ -61,6 +66,11 @@ certificate and additional attributes.
 .Fn X509_REQ_INFO_free
 frees
 .Fa req_info .
+If
+.Fa req_info
+is a
+.Dv NULL
+pointer, no action occurs.
 .Sh RETURN VALUES
 .Fn X509_REQ_new
 and
Index: keyproc.c
===
RCS file: /cvs/src/usr.sbin/acme-client/keyproc.c,v
retrieving revision 1.10
diff -u -p -r1.10 keyproc.c
--- keyproc.c   28 Jul 2018 15:25:23 -  1.10
+++ keyproc.c   29 Jul 2018 05:09:40 -
@@ -252,12 +252,10 @@ out:
free(der64);
free(sans);
free(san);
-   if (x != NULL)
-   X509_REQ_free(x);
-   if (name != NULL)
-   X509_NAME_free(name);
+   X509_REQ_free(x);
+   X509_NAME_free(name);
EVP_PKEY_free(pkey);
ERR_print_errors_fp(stderr);
ERR_free_strings();
-   return (rc);
+   return rc;
 }

[diff] acme-client - remove NULL checks before _free() calls

[Ross L Richardson]

The diff below just removes NULL checks before various LibreSSL *_free()
calls, on the assumption that that's the preferred usage where it's
documented that *_free(NULL) is safe.

[Note that the behaviour of X509_REQ_free(NULL) and X509_NAME_free(NULL)
is not documented in the man pages, so those calls in keyproc.c are not
modified in this diff.]

Ross


Index: acctproc.c
===
RCS file: /cvs/src/usr.sbin/acme-client/acctproc.c,v
retrieving revision 1.11
diff -u -p -r1.11 acctproc.c
--- acctproc.c  24 Jan 2017 13:32:55 -  1.11
+++ acctproc.c  28 Jul 2018 12:39:19 -
@@ -393,8 +393,7 @@ out:
close(netsock);
if (f != NULL)
fclose(f);
-   if (pkey != NULL)
-   EVP_PKEY_free(pkey);
+   EVP_PKEY_free(pkey);
ERR_print_errors_fp(stderr);
ERR_free_strings();
return rc;
Index: certproc.c
===
RCS file: /cvs/src/usr.sbin/acme-client/certproc.c,v
retrieving revision 1.10
diff -u -p -r1.10 certproc.c
--- certproc.c  24 Jan 2017 13:32:55 -  1.10
+++ certproc.c  28 Jul 2018 12:39:19 -
@@ -231,10 +231,8 @@ certproc(int netsock, int filesock)
 out:
close(netsock);
close(filesock);
-   if (x != NULL)
-   X509_free(x);
-   if (chainx != NULL)
-   X509_free(chainx);
+   X509_free(x);
+   X509_free(chainx);
free(csr);
free(url);
free(chain);
Index: keyproc.c
===
RCS file: /cvs/src/usr.sbin/acme-client/keyproc.c,v
retrieving revision 1.9
diff -u -p -r1.9 keyproc.c
--- keyproc.c   26 Mar 2017 18:41:02 -  1.9
+++ keyproc.c   28 Jul 2018 12:39:19 -
@@ -256,8 +256,7 @@ out:
X509_REQ_free(x);
if (name != NULL)
X509_NAME_free(name);
-   if (pkey != NULL)
-   EVP_PKEY_free(pkey);
+   EVP_PKEY_free(pkey);
ERR_print_errors_fp(stderr);
ERR_free_strings();
return (rc);
Index: revokeproc.c
===
RCS file: /cvs/src/usr.sbin/acme-client/revokeproc.c,v
retrieving revision 1.13
diff -u -p -r1.13 revokeproc.c
--- revokeproc.c8 Jul 2017 13:37:23 -   1.13
+++ revokeproc.c28 Jul 2018 12:39:19 -
@@ -335,10 +335,8 @@ out:
close(fd);
if (f != NULL)
fclose(f);
-   if (x != NULL)
-   X509_free(x);
-   if (bio != NULL)
-   BIO_free(bio);
+   X509_free(x);
+   BIO_free(bio);
free(san);
free(path);
free(der);
Index: rsa.c
===
RCS file: /cvs/src/usr.sbin/acme-client/rsa.c,v
retrieving revision 1.6
diff -u -p -r1.6 rsa.c
--- rsa.c   24 Jan 2017 13:32:55 -  1.6
+++ rsa.c   28 Jul 2018 12:39:19 -
@@ -62,12 +62,10 @@ rsa_key_create(FILE *f, const char *fnam
 
warnx("%s: PEM_write_PrivateKey", fname);
 err:
-   if (pkey != NULL)
-   EVP_PKEY_free(pkey);
+   EVP_PKEY_free(pkey);
pkey = NULL;
 out:
-   if (ctx != NULL)
-   EVP_PKEY_CTX_free(ctx);
+   EVP_PKEY_CTX_free(ctx);
return pkey;
 }
 

Typo in utf8.7

[Ross L Richardson]

Two bytes plus a bonus bit :-)

Patch below.

Ross
--


Index: utf8.7
===
RCS file: /cvs/src/share/man/man7/utf8.7,v
retrieving revision 1.6
diff -u -p -r1.6 utf8.7
--- utf8.7  31 May 2017 17:58:56 -  1.6
+++ utf8.7  16 May 2018 10:31:09 -
@@ -29,7 +29,7 @@ Unicode codepoints are encoded as follow
 .It U+ \(en U+007F:
 One byte: 0... (compatible with ASCII)
 .It U+0080 \(en U+07FF:
-Two bytes: 110. 10...
+Two bytes: 110. 10..
 .It U+0800 \(en U+D7FF and U+E000 \(en U+:
 Three bytes: 1110 10.. 10..
 .It U+1 \(en U+10:

Re: [Patch] style(9) improvements for sample code in pf(4)

[Ross L Richardson]

Improved version, with thanks to anton@

[Should usage() be __dead?]

Ross

Index: pf.4
===
RCS file: /cvs/src/share/man/man4/pf.4,v
retrieving revision 1.88
diff -u -p -r1.88 pf.4
--- pf.429 Aug 2017 02:16:56 -  1.88
+++ pf.49 Oct 2017 00:51:30 -
@@ -1008,14 +1008,16 @@ command to show the hard limit of a memo
 #include 
 #include 
 #include 
+
 #include 
 #include 
 #include 
+
+#include 
 #include 
 #include 
 #include 
 #include 
-#include 
 
 static const struct {
const char  *name;
@@ -1032,10 +1034,9 @@ static const struct {
 void
 usage(void)
 {
-   extern char *__progname;
int i;
 
-   fprintf(stderr, "usage: %s [", __progname);
+   fprintf(stderr, "usage: %s [", getprogname());
for (i = 0; pf_limits[i].name; i++)
fprintf(stderr, "%s%s", (i > 0 ? "|" : ""), pf_limits[i].name);
fprintf(stderr, "]\en");
@@ -1053,7 +1054,7 @@ main(int argc, char *argv[])
usage();
 
for (i = 0; pf_limits[i].name; i++)
-   if (!strcmp(argv[1], pf_limits[i].name)) {
+   if (strcmp(argv[1], pf_limits[i].name) == 0) {
pool_index = pf_limits[i].index;
break;
}
@@ -1067,10 +1068,10 @@ main(int argc, char *argv[])
if (dev == -1)
err(1, "open(\e"/dev/pf\e") failed");
 
-   bzero(, sizeof(struct pfioc_limit));
+   memset(, 0, sizeof(struct pfioc_limit));
pl.index = pool_index;
 
-   if (ioctl(dev, DIOCGETLIMIT, ))
+   if (ioctl(dev, DIOCGETLIMIT, ) == -1)
err(1, "DIOCGETLIMIT");
 
printf("The %s memory pool has ", pf_limits[i].name);
@@ -1079,7 +1080,7 @@ main(int argc, char *argv[])
else
printf("a hard limit of %u entries.\en", pl.limit);
 
-   return (0);
+   return 0;
 }
 .Ed
 .Sh SEE ALSO

[Patch] style(9) improvements for sample code in pf(4)

[Ross L Richardson]

Just trying to make things a little more standard.

Ross


Index: pf.4
===
RCS file: /cvs/src/share/man/man4/pf.4,v
retrieving revision 1.88
diff -u -p -r1.88 pf.4
--- pf.429 Aug 2017 02:16:56 -  1.88
+++ pf.48 Oct 2017 09:14:29 -
@@ -1011,11 +1011,11 @@ command to show the hard limit of a memo
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
-#include 
 
 static const struct {
const char  *name;
@@ -1032,10 +1032,9 @@ static const struct {
 void
 usage(void)
 {
-   extern char *__progname;
int i;
 
-   fprintf(stderr, "usage: %s [", __progname);
+   fprintf(stderr, "usage: %s [", getprogname());
for (i = 0; pf_limits[i].name; i++)
fprintf(stderr, "%s%s", (i > 0 ? "|" : ""), pf_limits[i].name);
fprintf(stderr, "]\en");
@@ -1053,7 +1052,7 @@ main(int argc, char *argv[])
usage();
 
for (i = 0; pf_limits[i].name; i++)
-   if (!strcmp(argv[1], pf_limits[i].name)) {
+   if (strcmp(argv[1], pf_limits[i].name) == 0) {
pool_index = pf_limits[i].index;
break;
}
@@ -1067,10 +1066,10 @@ main(int argc, char *argv[])
if (dev == -1)
err(1, "open(\e"/dev/pf\e") failed");
 
-   bzero(, sizeof(struct pfioc_limit));
+   memset(, 0, sizeof(struct pfioc_limit));
pl.index = pool_index;
 
-   if (ioctl(dev, DIOCGETLIMIT, ))
+   if (ioctl(dev, DIOCGETLIMIT, ) != 0)
err(1, "DIOCGETLIMIT");
 
printf("The %s memory pool has ", pf_limits[i].name);
@@ -1079,7 +1078,7 @@ main(int argc, char *argv[])
else
printf("a hard limit of %u entries.\en", pl.limit);
 
-   return (0);
+   return 0;
 }
 .Ed
 .Sh SEE ALSO

[Patch] directory.3 should describe return value of readdir_r()

[Ross L Richardson]

directory.3 (has no "RETURN VALUES" section and) fails to
describe the return value of readdir_r().

The diff below adds a sentence about the return value.
It also converts "return (X)" to "return X" in the
sample code.

Ross

Index: directory.3
===
RCS file: /cvs/src/lib/libc/gen/directory.3,v
retrieving revision 1.24
diff -u -p -r1.24 directory.3
--- directory.3 10 Nov 2015 23:48:18 -  1.24
+++ directory.3 13 Sep 2017 10:46:29 -
@@ -129,6 +129,8 @@ must be large enough for a dirent with a
 array member containing at least
 .Dv NAME_MAX
 plus one elements.
+.Fn readdir_r
+returns 0 on success, or an error number indicating the error which occurred.
 On successful return, the pointer returned at
 .Fa "*result"
 will have the same value as the argument
@@ -201,11 +203,11 @@ if (dirp) {
if (dp->d_namlen == len &&
!strcmp(dp->d_name, name)) {
(void)closedir(dirp);
-   return (FOUND);
+   return FOUND;
}
(void)closedir(dirp);
 }
-return (NOT_FOUND);
+return NOT_FOUND;
 .Ed
 .Sh ERRORS
 The

Add $REQUEST_SCHEME (block) macro to httpd

[Ross L Richardson]

The patch below adds a "$REQUEST_SCHEME" macro to those available in
block rules in httpd.conf

Justification: when redirecting from a (virtual) server which supports
both http and https to a (virtual) server which also supports both
schemes, it make sense to be able to respect (preserve) the scheme of
the original request rather than having to hard-code "http" or "https".

Patch is against -current, but has been tested (only) against
5.9-stable.

Ross


Index: src/usr.sbin/httpd/httpd.conf.5
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
retrieving revision 1.73
diff -u -p -u -r1.73 httpd.conf.5
--- src/usr.sbin/httpd/httpd.conf.5 9 May 2016 19:36:54 -   1.73
+++ src/usr.sbin/httpd/httpd.conf.5 22 Jun 2016 11:16:01 -
@@ -202,7 +202,7 @@ The
 .Ar uri
 may contain predefined macros that will be expanded at runtime:
 .Pp
-.Bl -tag -width $DOCUMENT_URI -offset indent -compact
+.Bl -tag -width $REQUEST_SCHEME -offset indent -compact
 .It Ic $DOCUMENT_URI
 The request path.
 .It Ic $QUERY_STRING
@@ -213,6 +213,14 @@ The IP address of the connected client.
 The TCP source port of the connected client.
 .It Ic $REMOTE_USER
 The remote user for HTTP authentication.
+.It Ic $REQUEST_SCHEME
+The scheme
+.Po
+.Dq http
+or
+.Dq https
+.Pc
+of the request.
 .It Ic $REQUEST_URI
 The request path and optional query string.
 .It Ic $SERVER_ADDR
Index: src/usr.sbin/httpd/server_http.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
retrieving revision 1.108
diff -u -p -u -r1.108 server_http.c
--- src/usr.sbin/httpd/server_http.c27 May 2016 11:24:13 -  1.108
+++ src/usr.sbin/httpd/server_http.c22 Jun 2016 11:16:01 -
@@ -1011,6 +1011,12 @@ server_expand_http(struct client *clt, c
if (ret != 0)
return (NULL);
}
+   if (strstr(val, "$REQUEST_SCHEME") != NULL) {
+   ret = expand_string(buf, len, "$REQUEST_SCHEME",
+   clt->clt_tls_ctx != NULL ? "https" : "http");
+   if (ret != 0)
+   return (NULL);
+   }
if (strstr(val, "$SERVER_") != NULL) {
if (strstr(val, "$SERVER_ADDR") != NULL) {
if (print_host(_conf->ss,