usb: another bad hid quirk

2022-06-02 Thread Anton Lindqvist
Hi,
I recently got a Gembird EG-PMS2 power strip controllable over USB.
It currently attaches over uhidev which is problematic since it lacks an
interrupt endpoint causing uhidev to punt early during attachment.
Instead, letting it attach as ugen makes it possible to control the
device using third-party software like sispmctl.

Here's a diff adding a quirk, excluding the usbdevs regen bits.

Comments? OK?

diff --git sys/dev/usb/usb_quirks.c sys/dev/usb/usb_quirks.c
index be65ad08602..a85ea0cd510 100644
--- sys/dev/usb/usb_quirks.c
+++ sys/dev/usb/usb_quirks.c
@@ -126,6 +126,7 @@ const struct usbd_quirk_entry {
  { USB_VENDOR_CYPRESS, USB_PRODUCT_CYPRESS_SISPM_OLD,  ANY,{ UQ_BAD_HID }},
  { USB_VENDOR_CYPRESS, USB_PRODUCT_CYPRESS_SISPM,  ANY,{ UQ_BAD_HID }},
  { USB_VENDOR_CYPRESS, USB_PRODUCT_CYPRESS_SISPM_FLASH,ANY,{ 
UQ_BAD_HID }},
+ { USB_VENDOR_CYPRESS, USB_PRODUCT_CYPRESS_GEMBIRD_EG_PMS2,ANY,{ 
UQ_BAD_HID }},
  { USB_VENDOR_MICROCHIP, USB_PRODUCT_MICROCHIP_USBLCD20X2, ANY,{ 
UQ_BAD_HID }},
  { USB_VENDOR_MICROCHIP, USB_PRODUCT_MICROCHIP_USBLCD256X64,   ANY,{ 
UQ_BAD_HID }},
  { USB_VENDOR_MECANIQUE, USB_PRODUCT_MECANIQUE_WISPY,  ANY,{ UQ_BAD_HID }},
diff --git sys/dev/usb/usbdevs sys/dev/usb/usbdevs
index 1091754f576..6cbf022bba4 100644
--- sys/dev/usb/usbdevs
+++ sys/dev/usb/usbdevs
@@ -1508,6 +1508,7 @@ product CYPRESS LPRDK 0xe001  CY4636 LP RDK 
Bridge
 product CYPRESS SISPM_OLD  0xfd10  Sispm - old version
 product CYPRESS SISPM  0xfd11  Sispm
 product CYPRESS SISPM_FLASH0xfd12  Sispm - flash
+product CYPRESS GEMBIRD_EG_PMS20xfd15  Gembird EG-PMS2
 
 /* Daisy Technology products */
 product DAISY DMC  0x6901  PhotoClip



Re: httpd: add include_dir keyword

2022-06-02 Thread Theo de Raadt
I do not understand why it is believed that people will generate 
better configurations if they split the parts out into different
files.

Adding that kind of trick to an already established grammer rarely works
well.  It only works in narrowly constrained uses of the old grammer,
because now one must consider what is in the included files.  At that
point, why the extra files?  It does not require less brainpower, it
potentially requires more, when the included files start interfering
with the core.

This feels ripe for abuse, and of not much use.


mfre...@mulethew.com wrote:

> Coincidentally I have been working on adding globbing support to 
> include in the httpd config parser. I have only done light testing,
> nothing in production yet but the patch provided below has not given
> me any trouble in my test environment yet. Any feedback is welcome!
> -Matt
> 
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> retrieving revision 1.128
> diff -u -p -u -p -r1.128 parse.y
> --- parse.y   27 Feb 2022 20:30:30 -  1.128
> +++ parse.y   2 Jun 2022 20:29:46 -
> @@ -52,6 +52,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "httpd.h"
>  #include "http.h"
> @@ -165,16 +166,21 @@ grammar : /* empty */
>  
>  include  : INCLUDE STRING{
>   struct file *nfile;
> + glob_t g;
>  
> - if ((nfile = pushfile($2, 0)) == NULL) {
> - yyerror("failed to include file %s", $2);
> - free($2);
> - YYERROR;
> + memset(, 0, sizeof(g));
> + glob($2, GLOB_NOCHECK, NULL, );
> + for(int i = 0; i < g.gl_pathc; ++i) {
> + if ((nfile = pushfile(g.gl_pathv[i], 0)) == 
> NULL) {
> + yyerror("failed to include file %s", 
> g.gl_pathv[i]);
> + free(g.gl_pathv[i]);
> + YYERROR;
> + }
> + file = nfile;
> + lungetc('\n');
>   }
> + globfree();
>   free($2);
> -
> - file = nfile;
> - lungetc('\n');
>   }
>   ;
>  
> 



Re: obsolete dump options syntax

2022-06-02 Thread Christian Weisgerber
Jan Stary:

> http://cvsweb.netbsd.org/bsdweb.cgi/src/sbin/dump/main.c.diff?r1=1.4=1.5=h
> "Use getopt(3), with obsolete() from restore(8) for backward compatibility."
> 
> So it's restore(8); I write "restore rf" myself.
> Is this it? Does that still need to be supported by dump(8)?

The same "bundled flags" syntax is supported by dump(8), restore(8),
tar(1), and ar(1).

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: httpd: add include_dir keyword

2022-06-02 Thread mfrench
Coincidentally I have been working on adding globbing support to 
include in the httpd config parser. I have only done light testing,
nothing in production yet but the patch provided below has not given
me any trouble in my test environment yet. Any feedback is welcome!
-Matt

Index: parse.y
===
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
retrieving revision 1.128
diff -u -p -u -p -r1.128 parse.y
--- parse.y 27 Feb 2022 20:30:30 -  1.128
+++ parse.y 2 Jun 2022 20:29:46 -
@@ -52,6 +52,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "httpd.h"
 #include "http.h"
@@ -165,16 +166,21 @@ grammar   : /* empty */
 
 include: INCLUDE STRING{
struct file *nfile;
+   glob_t g;
 
-   if ((nfile = pushfile($2, 0)) == NULL) {
-   yyerror("failed to include file %s", $2);
-   free($2);
-   YYERROR;
+   memset(, 0, sizeof(g));
+   glob($2, GLOB_NOCHECK, NULL, );
+   for(int i = 0; i < g.gl_pathc; ++i) {
+   if ((nfile = pushfile(g.gl_pathv[i], 0)) == 
NULL) {
+   yyerror("failed to include file %s", 
g.gl_pathv[i]);
+   free(g.gl_pathv[i]);
+   YYERROR;
+   }
+   file = nfile;
+   lungetc('\n');
}
+   globfree();
free($2);
-
-   file = nfile;
-   lungetc('\n');
}
;
 



[patch] 802.11 printing akm and cipher suite lists in tcpdump

2022-06-02 Thread Mikhail
Recently I bought a router with WPA3 support and decided to investigate
wireless dump with WPA3 config, during the process I've found a small
bug in tcpdump - it doesn't print all akms, also the printing logic is
flawed if more than one akm or pairwise cipher is presented - there is
extra addition to the data index.

Tested with multiple akms, can't test with multiple ciphers, since my
router doesn't support such configuration.

diff --git usr.sbin/tcpdump/print-802_11.c usr.sbin/tcpdump/print-802_11.c
index b0641a29279..14ecbdc6cfc 100644
--- usr.sbin/tcpdump/print-802_11.c
+++ usr.sbin/tcpdump/print-802_11.c
@@ -860,6 +860,9 @@ ieee80211_print_akm(uint8_t selector[4])
case 6:
printf("SHA256-PSK");
break;
+   case 8:
+   printf("SAE");
+   break;
default:
printf("%d", selector[3]);
break;
@@ -910,7 +913,7 @@ ieee80211_print_rsn(u_int8_t *data, u_int len)
printf(",cipher%s ", nciphers > 1 ? "s" : "");
for (i = 0; i < nciphers; i++) {
for (j = 0; j < 4; j++)
-   selector[j] = data[i + j];
+   selector[j] = data[j];
ieee80211_print_rsncipher(selector);
if (i < nciphers - 1)
printf(" ");
@@ -931,11 +934,11 @@ ieee80211_print_rsn(u_int8_t *data, u_int len)
}
 
printf(",akm%s ", nakms > 1 ? "s" : "");
-   for (i = 0; i < nciphers; i++) {
+   for (i = 0; i < nakms; i++) {
for (j = 0; j < 4; j++)
-   selector[j] = data[i + j];
+   selector[j] = data[j];
ieee80211_print_akm(selector);
-   if (i < nciphers - 1)
+   if (i < nakms - 1)
printf(" ");
data += 4;
}



Re: vmm: remove vm teardown from vcpu run path (testers needed)

2022-06-02 Thread Dave Voutila


Dave Voutila  writes:

> tech@ et al.:
>
> Looking for testers of the following diff for vmm(4). In my efforts to
> fix some stability issues, I'm taking baby steps tweaking parts of the
> code to make my upcoming proposal (adding refcnts) easier to swallow.
>
> This change removes the calling of vm_teardown from the code path in
> vm_run after vmm has exited the vm/vcpu and is on its way back to
> userland/vmd(8).
>
> vm_teardown is currently called in 3 areas to destroy/free a vm:
>
>   - vm_create: as cleanup in an error path
>   - vm_terminate: on a vm the ioctl is killing
>   - vm_run: the run ioctl handler
>
> This diff removes that last bullet. It's not needed as vmd will cleanup
> the vm on child exit, calling vm_terminate. Any non-vmd user of vmm(4)
> can stop being lazy and use the VMM_IOC_TERM ioctl.
>
> Not included in the snippet is the existing final else block that still
> toggles the vcpu state:
>
>   } else {
>   vrp->vrp_exit_reason = VM_EXIT_TERMINATED;
>   vcpu->vc_state = VCPU_STATE_TERMINATED;
>   }
>
> If testing, please describe *any* difference in shutdown/reboot of vm
> guests. (n.b. there's a known issue for Linux guests running very recent
> Linux kernels not being able to reboot. That needs to be addressed in
> vmd.)
>

Bumping as the diff has been out for testing and looking for ok's.

-dv

>
>
> Index: sys/arch/amd64/amd64/vmm.c
> ===
> RCS file: /opt/cvs/src/sys/arch/amd64/amd64/vmm.c,v
> retrieving revision 1.311
> diff -u -p -r1.311 vmm.c
> --- sys/arch/amd64/amd64/vmm.c20 May 2022 22:42:09 -  1.311
> +++ sys/arch/amd64/amd64/vmm.c23 May 2022 11:57:49 -
> @@ -4495,22 +4495,8 @@ vm_run(struct vm_run_params *vrp)
>   ret = vcpu_run_svm(vcpu, vrp);
>   }
>
> - /*
> -  * We can set the VCPU states here without CAS because once
> -  * a VCPU is in state RUNNING or REQTERM, only the VCPU itself
> -  * can switch the state.
> -  */
>   atomic_dec_int(>vm_vcpus_running);
> - if (vcpu->vc_state == VCPU_STATE_REQTERM) {
> - vrp->vrp_exit_reason = VM_EXIT_TERMINATED;
> - vcpu->vc_state = VCPU_STATE_TERMINATED;
> - if (vm->vm_vcpus_running == 0) {
> - rw_enter_write(_softc->vm_lock);
> - vm_teardown(vm);
> - rw_exit_write(_softc->vm_lock);
> - }
> - ret = 0;
> - } else if (ret == 0 || ret == EAGAIN) {
> + if (ret == 0 || ret == EAGAIN) {
>   /* If we are exiting, populate exit data so vmd can help. */
>   vrp->vrp_exit_reason = (ret == 0) ? VM_EXIT_NONE
>   : vcpu->vc_gueststate.vg_exit_reason;


--
-Dave Voutila



Re: Change behaviour of vis(3) in syslogd concerning backslash escaping

2022-06-02 Thread Theo de Raadt
The purpose of the vis() addition was mostly to guard against later
"cat" views of the output files sending remote-controllable escape-codes
to terminals (especially in xterm, there are many unfortunately features
which should not be reachable from remote.  the nastiest features were
disabled over decades, and some bugs were fixed, but some nasty escape
codes remain).

But please consider this impact of the change you propose.

 There is one additional flag, VIS_NOSLASH, which inhibits the doubling of
 backslashes and the backslash before the default format (that is, control
 characters are represented by `^C' and meta characters as `M-C').  With
 this flag set, the encoding is ambiguous and non-invertible.


This means if syslog is used to send some 'binary data', and you later on
want to decode meaning "unvis" the block, that won't work.  Is that a usage
case to worry about?


Matthias Pitzl  wrote:

> Hi,
> 
> We're sending log data in JSON format to a SIEM system and noticed a special 
> behaviour of
> OpenBSD's syslogd concerning strings with backslashes that is unique to 
> OpenBSD:
> echo '{"msg": \"This is "a test\""}' | logger
> results in the following string logged:
> {"msg": "This is \\"a test\\""}
> 
> 
> As no other syslog daemon I tried (Linx and FreeBSD) behaves like this, the 
> SIEM
> system does not use something like unvis() to correctly remove the escaping.
> This leads to a wrong text in the SIEM system after parsing the JSON string:
> This is \"a test\"
> 
> This has been introduced about 21 years ago when vis(3) was added to syslogd.
> 
> The following diff changes the behaviour of syslogd so that it no longer 
> escapes
> backslashes and thus is more consistent with other syslog implementations.
> 
> Greetings,
> Matthias
> 
> diff --git a/usr.sbin/syslogd/syslogd.c b/usr.sbin/syslogd/syslogd.c
> index d44b311ae1..184e0d6089 100644
> --- a/usr.sbin/syslogd/syslogd.c
> +++ b/usr.sbin/syslogd/syslogd.c
> @@ -1571,7 +1571,7 @@ printline(char *hname, char *msgstr)
>   if (*p == '\n')
>   *q++ = ' ';
>   else
> - q = vis(q, *p, 0, 0);
> + q = vis(q, *p, VIS_NOSLASH, 0);
>   }
>   line[LOG_MAXLINE] = *q = '\0';
>  
> @@ -1627,7 +1627,7 @@ printsys(char *msgstr)
>   q = lp;
>   while (*p && (c = *p++) != '\n' &&
>   q < _msg[sizeof(msg.m_msg) - 4])
> - q = vis(q, c, 0, 0);
> + q = vis(q, c, VIS_NOSLASH, 0);
>  
>   logmsg(, flags, LocalHostName);
>   }



Re: obsolete dump options syntax

2022-06-02 Thread Theo de Raadt
Sure.  That solves the immediate problem, provides people with a
strong hint to use options, and does no harm to the legacy option
behaviour which people used for half a century and will use for
the next half a century

Todd C. Miller  wrote:

> On Thu, 02 Jun 2022 07:54:02 -0600, "Theo de Raadt" wrote:
> 
> > I'm fine with a / check, but it also needs documenting.  While there can't
> > we say at least one option must be supplied?
> 
> How about this?
> 
>  - todd
> 
> Index: sbin/dump/dump.8
> ===
> RCS file: /cvs/src/sbin/dump/dump.8,v
> retrieving revision 1.54
> diff -u -p -u -r1.54 dump.8
> --- sbin/dump/dump.8  19 Dec 2019 09:38:03 -  1.54
> +++ sbin/dump/dump.8  2 Jun 2022 14:39:09 -
> @@ -293,6 +293,13 @@ In the latter case, certain restrictions
>  is ignored, the only dump level that is supported is
>  .Fl 0 ,
>  and all of the files must reside on the same filesystem.
> +If no options are specified, the first of the
> +.Ar files-to-dump
> +must contain a
> +.Ql /
> +character to prevent it from being interpreted as a
> +.Bx 4.3
> +option string.
>  .Pp
>  .Nm
>  requires operator intervention on these conditions:
> Index: sbin/dump/main.c
> ===
> RCS file: /cvs/src/sbin/dump/main.c,v
> retrieving revision 1.62
> diff -u -p -u -r1.62 main.c
> --- sbin/dump/main.c  21 Jan 2021 00:16:36 -  1.62
> +++ sbin/dump/main.c  2 Jun 2022 13:32:15 -
> @@ -718,9 +718,9 @@ obsolete(int *argcp, char **argvp[])
>   argv = *argvp;
>   argc = *argcp;
>  
> - /* Return if no arguments or first argument has leading dash. */
> + /* Return if no args or first argument has leading dash or a slash. */
>   ap = argv[1];
> - if (argc == 1 || *ap == '-')
> + if (argc == 1 || *ap == '-' || strchr(ap, '/') != NULL)
>   return;
>  
>   /* Allocate space for new arguments. */



Re: obsolete dump options syntax

2022-06-02 Thread Todd C . Miller
On Thu, 02 Jun 2022 07:54:02 -0600, "Theo de Raadt" wrote:

> I'm fine with a / check, but it also needs documenting.  While there can't
> we say at least one option must be supplied?

How about this?

 - todd

Index: sbin/dump/dump.8
===
RCS file: /cvs/src/sbin/dump/dump.8,v
retrieving revision 1.54
diff -u -p -u -r1.54 dump.8
--- sbin/dump/dump.819 Dec 2019 09:38:03 -  1.54
+++ sbin/dump/dump.82 Jun 2022 14:39:09 -
@@ -293,6 +293,13 @@ In the latter case, certain restrictions
 is ignored, the only dump level that is supported is
 .Fl 0 ,
 and all of the files must reside on the same filesystem.
+If no options are specified, the first of the
+.Ar files-to-dump
+must contain a
+.Ql /
+character to prevent it from being interpreted as a
+.Bx 4.3
+option string.
 .Pp
 .Nm
 requires operator intervention on these conditions:
Index: sbin/dump/main.c
===
RCS file: /cvs/src/sbin/dump/main.c,v
retrieving revision 1.62
diff -u -p -u -r1.62 main.c
--- sbin/dump/main.c21 Jan 2021 00:16:36 -  1.62
+++ sbin/dump/main.c2 Jun 2022 13:32:15 -
@@ -718,9 +718,9 @@ obsolete(int *argcp, char **argvp[])
argv = *argvp;
argc = *argcp;
 
-   /* Return if no arguments or first argument has leading dash. */
+   /* Return if no args or first argument has leading dash or a slash. */
ap = argv[1];
-   if (argc == 1 || *ap == '-')
+   if (argc == 1 || *ap == '-' || strchr(ap, '/') != NULL)
return;
 
/* Allocate space for new arguments. */



Change behaviour of vis(3) in syslogd concerning backslash escaping

2022-06-02 Thread Matthias Pitzl
Hi,

We're sending log data in JSON format to a SIEM system and noticed a special 
behaviour of
OpenBSD's syslogd concerning strings with backslashes that is unique to OpenBSD:
echo '{"msg": \"This is "a test\""}' | logger
results in the following string logged:
{"msg": "This is \\"a test\\""}


As no other syslog daemon I tried (Linx and FreeBSD) behaves like this, the SIEM
system does not use something like unvis() to correctly remove the escaping.
This leads to a wrong text in the SIEM system after parsing the JSON string:
This is \"a test\"

This has been introduced about 21 years ago when vis(3) was added to syslogd.

The following diff changes the behaviour of syslogd so that it no longer escapes
backslashes and thus is more consistent with other syslog implementations.

Greetings,
Matthias

diff --git a/usr.sbin/syslogd/syslogd.c b/usr.sbin/syslogd/syslogd.c
index d44b311ae1..184e0d6089 100644
--- a/usr.sbin/syslogd/syslogd.c
+++ b/usr.sbin/syslogd/syslogd.c
@@ -1571,7 +1571,7 @@ printline(char *hname, char *msgstr)
if (*p == '\n')
*q++ = ' ';
else
-   q = vis(q, *p, 0, 0);
+   q = vis(q, *p, VIS_NOSLASH, 0);
}
line[LOG_MAXLINE] = *q = '\0';
 
@@ -1627,7 +1627,7 @@ printsys(char *msgstr)
q = lp;
while (*p && (c = *p++) != '\n' &&
q < _msg[sizeof(msg.m_msg) - 4])
-   q = vis(q, c, 0, 0);
+   q = vis(q, c, VIS_NOSLASH, 0);
 
logmsg(, flags, LocalHostName);
}


smime.p7s
Description: S/MIME cryptographic signature


Re: obsolete dump options syntax

2022-06-02 Thread Theo de Raadt
Todd C. Miller  wrote:

> True, those would not be handled but isn't the most common usage
> to pass a fully-qualified path or a device name?  The biggest problem
> I see is that this would not catch a disk uid being used but I don't
> think that is really fixable unless we check the string for a duid
> first.

Pattern matching the string to decide "oh it cannot be a path" is really
weird, besides the fact it is a TOCTOU.

I'm fine with a / check, but it also needs documenting.  While there can't
we say at least one option must be supplied?

Is using dump without an option realistic?



Re: obsolete dump options syntax

2022-06-02 Thread Todd C . Miller
On Thu, 02 Jun 2022 07:43:15 -0600, "Theo de Raadt" wrote:

> Hmm, but consider these cases
>
> dump home
>
> or
>
> mkdir 0af
> dump 0af
>
> or
>
> cd /dev && dump rsd0a

True, those would not be handled but isn't the most common usage
to pass a fully-qualified path or a device name?  The biggest problem
I see is that this would not catch a disk uid being used but I don't
think that is really fixable unless we check the string for a duid
first.

> Don't people always pass at least '0' (to ignore a stored level) and/or
> 'a' (to avoid the volume sizing code), on very large filesystems in
> particular, so it becomes good practice to always pass at least one
> option, so maybe we should just state the requirement is you must pass a
> flag?  Or are there people passing no flags?

I certainly always have, for example "dump 0f ...".
That said, I don't see a downside to avoiding interpreting what is
clearly a pathname as an obsolete argument.

 - todd



Re: obsolete dump options syntax

2022-06-02 Thread Theo de Raadt
Todd C. Miller  wrote:

> On Thu, 02 Jun 2022 14:36:16 +0200, Jan Stary wrote:
> 
> > That results in the above. What obsolete options format
> > is this trying to accomodate? The manpage doesn't say -
> > the options it describes are perfectly getopt()-likable.
> > Looking at the CVS log, this was already "obsolete"
> > in the original 1995 import. Does anyone still use
> > that undescribed obsolete syntax dump "supports"?
> 
> Yes, many people do, myself included.  These programs have worked
> this way for the past 42 years and you cannot just break that.
> 
> However, it may be reasonable to skip parsing in obsolete() if the
> first argument contains a '/' since that is cannot match the old
> syntax.  FreeBSD just checks for a leading slash.

Hmm, but consider these cases

dump home

or

mkdir 0af
dump 0af

or

cd /dev && dump rsd0a

Don't people always pass at least '0' (to ignore a stored level) and/or
'a' (to avoid the volume sizing code), on very large filesystems in
particular, so it becomes good practice to always pass at least one
option, so maybe we should just state the requirement is you must pass a
flag?  Or are there people passing no flags?

>  - todd
> 
> Index: main.c
> ===
> RCS file: /cvs/src/sbin/dump/main.c,v
> retrieving revision 1.62
> diff -u -p -u -r1.62 main.c
> --- main.c21 Jan 2021 00:16:36 -  1.62
> +++ main.c2 Jun 2022 13:32:15 -
> @@ -718,9 +718,9 @@ obsolete(int *argcp, char **argvp[])
>   argv = *argvp;
>   argc = *argcp;
>  
> - /* Return if no arguments or first argument has leading dash. */
> + /* Return if no args or first argument has leading dash or a slash. */
>   ap = argv[1];
> - if (argc == 1 || *ap == '-')
> + if (argc == 1 || *ap == '-' || strchr(ap, '/') != NULL)
>   return;
>  
>   /* Allocate space for new arguments. */
> 



Re: obsolete dump options syntax

2022-06-02 Thread Todd C . Miller
On Thu, 02 Jun 2022 14:36:16 +0200, Jan Stary wrote:

> That results in the above. What obsolete options format
> is this trying to accomodate? The manpage doesn't say -
> the options it describes are perfectly getopt()-likable.
> Looking at the CVS log, this was already "obsolete"
> in the original 1995 import. Does anyone still use
> that undescribed obsolete syntax dump "supports"?

Yes, many people do, myself included.  These programs have worked
this way for the past 42 years and you cannot just break that.

However, it may be reasonable to skip parsing in obsolete() if the
first argument contains a '/' since that is cannot match the old
syntax.  FreeBSD just checks for a leading slash.

 - todd

Index: main.c
===
RCS file: /cvs/src/sbin/dump/main.c,v
retrieving revision 1.62
diff -u -p -u -r1.62 main.c
--- main.c  21 Jan 2021 00:16:36 -  1.62
+++ main.c  2 Jun 2022 13:32:15 -
@@ -718,9 +718,9 @@ obsolete(int *argcp, char **argvp[])
argv = *argvp;
argc = *argcp;
 
-   /* Return if no arguments or first argument has leading dash. */
+   /* Return if no args or first argument has leading dash or a slash. */
ap = argv[1];
-   if (argc == 1 || *ap == '-')
+   if (argc == 1 || *ap == '-' || strchr(ap, '/') != NULL)
return;
 
/* Allocate space for new arguments. */



Re: obsolete dump options syntax

2022-06-02 Thread Jan Stary
On Jun 02 07:16:52, dera...@openbsd.org wrote:
> Your diff completely breaks a majority of the ways people use it.

Does that mean people mostly use
the undocumented obsolete syntax
that obsolete() keeps supported?


> Jan Stary  wrote:
> 
> > # dump /home   
> > dump: option requires an argument -- h
> > 
> > # dump /music
> > dump: option requires an argument -- s
> > 
> > # dump /media 
> > dump: option requires an argument -- d
> > 
> > What? Before passing its options to getopt(),
> > dump's main() processes them with obsolete():
> > 
> > Change set of key letters and ordered arguments
> > into something getopt(3) will like.
> > 
> > That results in the above. What obsolete options format
> > is this trying to accomodate? The manpage doesn't say -
> > the options it describes are perfectly getopt()-likable.
> > Looking at the CVS log, this was already "obsolete"
> > in the original 1995 import. Does anyone still use
> > that undescribed obsolete syntax dump "supports"?
> > 
> > The diff below simply removes the function.
> > With the diff, dump does the right thing,
> > i.e. just tries to dump the given fs to rst0.
> > 
> > # dump /home
> > DUMP: Date of this level 0 dump: Thu Jun  2 14:20:44 2022
> > DUMP: Date of last level 0 dump: the epoch
> > DUMP: Dumping /dev/rsd0a (/home) to /dev/rst0
> > DUMP: mapping (Pass I) [regular files]
> > DUMP: mapping (Pass II) [directories]
> > DUMP: estimated 47580129 tape blocks on 1222.00 tape(s).
> > DUMP: Cannot open output "/dev/rst0".
> > 
> > Jan
> > 
> > 
> > Index: main.c
> > ===
> > RCS file: /cvs/src/sbin/dump/main.c,v
> > retrieving revision 1.62
> > diff -u -p -r1.62 main.c
> > --- main.c  21 Jan 2021 00:16:36 -  1.62
> > +++ main.c  2 Jun 2022 12:23:56 -
> > @@ -99,7 +99,6 @@ struct disklabel lab;
> >  static int sblock_try[] = SBLOCKSEARCH;
> >  
> >  static long long numarg(char *, long long, long long);
> > -static void obsolete(int *, char **[]);
> >  static void usage(void);
> >  
> >  int
> > @@ -134,7 +133,6 @@ main(int argc, char *argv[])
> > if (argc < 2)
> > usage();
> >  
> > -   obsolete(, );
> > while ((ch = getopt(argc, argv, "0123456789aB:b:cd:f:h:ns:ST:uWw")) != 
> > -1)
> > switch (ch) {
> > /* dump level */
> > @@ -700,80 +698,4 @@ getduid(char *path)
> > }
> >  
> > return (NULL);
> > -}
> > -
> > -/*
> > - * obsolete --
> > - * Change set of key letters and ordered arguments into something
> > - * getopt(3) will like.
> > - */
> > -static void
> > -obsolete(int *argcp, char **argvp[])
> > -{
> > -   int argc, flags;
> > -   char *ap, **argv, *flagsp, **nargv, *p;
> > -   size_t len;
> > -
> > -   /* Setup. */
> > -   argv = *argvp;
> > -   argc = *argcp;
> > -
> > -   /* Return if no arguments or first argument has leading dash. */
> > -   ap = argv[1];
> > -   if (argc == 1 || *ap == '-')
> > -   return;
> > -
> > -   /* Allocate space for new arguments. */
> > -   if ((*argvp = nargv = calloc(argc + 1, sizeof(char *))) == NULL ||
> > -   (p = flagsp = malloc(strlen(ap) + 2)) == NULL)
> > -   err(1, NULL);
> > -
> > -   *nargv++ = *argv;
> > -   argv += 2;
> > -
> > -   for (flags = 0; *ap; ++ap) {
> > -   switch (*ap) {
> > -   case 'B':
> > -   case 'b':
> > -   case 'd':
> > -   case 'f':
> > -   case 'h':
> > -   case 's':
> > -   case 'T':
> > -   if (*argv == NULL) {
> > -   warnx("option requires an argument -- %c", *ap);
> > -   usage();
> > -   }
> > -   len = 2 + strlen(*argv) + 1;
> > -   if ((nargv[0] = malloc(len)) == NULL)
> > -   err(1, NULL);
> > -   nargv[0][0] = '-';
> > -   nargv[0][1] = *ap;
> > -   (void)strlcpy([0][2], *argv, len - 2);
> > -   ++argv;
> > -   ++nargv;
> > -   break;
> > -   default:
> > -   if (!flags) {
> > -   *p++ = '-';
> > -   flags = 1;
> > -   }
> > -   *p++ = *ap;
> > -   break;
> > -   }
> > -   }
> > -
> > -   /* Terminate flags, or toss the buffer we did not use. */
> > -   if (flags) {
> > -   *p = '\0';
> > -   *nargv++ = flagsp;
> > -   } else
> > -   free(flagsp);
> > -
> > -   /* Copy remaining arguments. */
> > -   while ((*nargv++ = *argv++))
> > -   continue;
> > -
> > -   /* Update argument count. */
> > -   *argcp = nargv - *argvp - 1;
> >  }
> > 
> 
> 



Re: obsolete dump options syntax

2022-06-02 Thread Theo de Raadt
Your diff completely breaks a majority of the ways people use it.


Jan Stary  wrote:

> # dump /home   
> dump: option requires an argument -- h
> 
> # dump /music
> dump: option requires an argument -- s
> 
> # dump /media 
> dump: option requires an argument -- d
> 
> What? Before passing its options to getopt(),
> dump's main() processes them with obsolete():
> 
>   Change set of key letters and ordered arguments
>   into something getopt(3) will like.
> 
> That results in the above. What obsolete options format
> is this trying to accomodate? The manpage doesn't say -
> the options it describes are perfectly getopt()-likable.
> Looking at the CVS log, this was already "obsolete"
> in the original 1995 import. Does anyone still use
> that undescribed obsolete syntax dump "supports"?
> 
> The diff below simply removes the function.
> With the diff, dump does the right thing,
> i.e. just tries to dump the given fs to rst0.
> 
> # dump /home
> DUMP: Date of this level 0 dump: Thu Jun  2 14:20:44 2022
> DUMP: Date of last level 0 dump: the epoch
> DUMP: Dumping /dev/rsd0a (/home) to /dev/rst0
> DUMP: mapping (Pass I) [regular files]
> DUMP: mapping (Pass II) [directories]
> DUMP: estimated 47580129 tape blocks on 1222.00 tape(s).
> DUMP: Cannot open output "/dev/rst0".
> 
>   Jan
> 
> 
> Index: main.c
> ===
> RCS file: /cvs/src/sbin/dump/main.c,v
> retrieving revision 1.62
> diff -u -p -r1.62 main.c
> --- main.c21 Jan 2021 00:16:36 -  1.62
> +++ main.c2 Jun 2022 12:23:56 -
> @@ -99,7 +99,6 @@ struct disklabel lab;
>  static int sblock_try[] = SBLOCKSEARCH;
>  
>  static long long numarg(char *, long long, long long);
> -static void obsolete(int *, char **[]);
>  static void usage(void);
>  
>  int
> @@ -134,7 +133,6 @@ main(int argc, char *argv[])
>   if (argc < 2)
>   usage();
>  
> - obsolete(, );
>   while ((ch = getopt(argc, argv, "0123456789aB:b:cd:f:h:ns:ST:uWw")) != 
> -1)
>   switch (ch) {
>   /* dump level */
> @@ -700,80 +698,4 @@ getduid(char *path)
>   }
>  
>   return (NULL);
> -}
> -
> -/*
> - * obsolete --
> - *   Change set of key letters and ordered arguments into something
> - *   getopt(3) will like.
> - */
> -static void
> -obsolete(int *argcp, char **argvp[])
> -{
> - int argc, flags;
> - char *ap, **argv, *flagsp, **nargv, *p;
> - size_t len;
> -
> - /* Setup. */
> - argv = *argvp;
> - argc = *argcp;
> -
> - /* Return if no arguments or first argument has leading dash. */
> - ap = argv[1];
> - if (argc == 1 || *ap == '-')
> - return;
> -
> - /* Allocate space for new arguments. */
> - if ((*argvp = nargv = calloc(argc + 1, sizeof(char *))) == NULL ||
> - (p = flagsp = malloc(strlen(ap) + 2)) == NULL)
> - err(1, NULL);
> -
> - *nargv++ = *argv;
> - argv += 2;
> -
> - for (flags = 0; *ap; ++ap) {
> - switch (*ap) {
> - case 'B':
> - case 'b':
> - case 'd':
> - case 'f':
> - case 'h':
> - case 's':
> - case 'T':
> - if (*argv == NULL) {
> - warnx("option requires an argument -- %c", *ap);
> - usage();
> - }
> - len = 2 + strlen(*argv) + 1;
> - if ((nargv[0] = malloc(len)) == NULL)
> - err(1, NULL);
> - nargv[0][0] = '-';
> - nargv[0][1] = *ap;
> - (void)strlcpy([0][2], *argv, len - 2);
> - ++argv;
> - ++nargv;
> - break;
> - default:
> - if (!flags) {
> - *p++ = '-';
> - flags = 1;
> - }
> - *p++ = *ap;
> - break;
> - }
> - }
> -
> - /* Terminate flags, or toss the buffer we did not use. */
> - if (flags) {
> - *p = '\0';
> - *nargv++ = flagsp;
> - } else
> - free(flagsp);
> -
> - /* Copy remaining arguments. */
> - while ((*nargv++ = *argv++))
> - continue;
> -
> - /* Update argument count. */
> - *argcp = nargv - *argvp - 1;
>  }
> 



Re: obsolete dump options syntax

2022-06-02 Thread Jan Stary
On Jun 02 14:36:16, h...@stare.cz wrote:
> # dump /home   
> dump: option requires an argument -- h
> 
> # dump /music
> dump: option requires an argument -- s
> 
> # dump /media 
> dump: option requires an argument -- d
> 
> What? Before passing its options to getopt(),
> dump's main() processes them with obsolete():
> 
>   Change set of key letters and ordered arguments
>   into something getopt(3) will like.
> 
> That results in the above. What obsolete options format
> is this trying to accomodate?

http://cvsweb.netbsd.org/bsdweb.cgi/src/sbin/dump/main.c.diff?r1=1.4=1.5=h
"Use getopt(3), with obsolete() from restore(8) for backward compatibility."

So it's restore(8); I write "restore rf" myself.
Is this it? Does that still need to be supported by dump(8)?
And by restore(8), while here?


> The manpage doesn't say -
> the options it describes are perfectly getopt()-likable.
> Looking at the CVS log, this was already "obsolete"
> in the original 1995 import. Does anyone still use
> that undescribed obsolete syntax dump "supports"?
> 
> The diff below simply removes the function.
> With the diff, dump does the right thing,
> i.e. just tries to dump the given fs to rst0.
> 
> # dump /home
> DUMP: Date of this level 0 dump: Thu Jun  2 14:20:44 2022
> DUMP: Date of last level 0 dump: the epoch
> DUMP: Dumping /dev/rsd0a (/home) to /dev/rst0
> DUMP: mapping (Pass I) [regular files]
> DUMP: mapping (Pass II) [directories]
> DUMP: estimated 47580129 tape blocks on 1222.00 tape(s).
> DUMP: Cannot open output "/dev/rst0".
> 
>   Jan
> 
> 
> Index: main.c
> ===
> RCS file: /cvs/src/sbin/dump/main.c,v
> retrieving revision 1.62
> diff -u -p -r1.62 main.c
> --- main.c21 Jan 2021 00:16:36 -  1.62
> +++ main.c2 Jun 2022 12:23:56 -
> @@ -99,7 +99,6 @@ struct disklabel lab;
>  static int sblock_try[] = SBLOCKSEARCH;
>  
>  static long long numarg(char *, long long, long long);
> -static void obsolete(int *, char **[]);
>  static void usage(void);
>  
>  int
> @@ -134,7 +133,6 @@ main(int argc, char *argv[])
>   if (argc < 2)
>   usage();
>  
> - obsolete(, );
>   while ((ch = getopt(argc, argv, "0123456789aB:b:cd:f:h:ns:ST:uWw")) != 
> -1)
>   switch (ch) {
>   /* dump level */
> @@ -700,80 +698,4 @@ getduid(char *path)
>   }
>  
>   return (NULL);
> -}
> -
> -/*
> - * obsolete --
> - *   Change set of key letters and ordered arguments into something
> - *   getopt(3) will like.
> - */
> -static void
> -obsolete(int *argcp, char **argvp[])
> -{
> - int argc, flags;
> - char *ap, **argv, *flagsp, **nargv, *p;
> - size_t len;
> -
> - /* Setup. */
> - argv = *argvp;
> - argc = *argcp;
> -
> - /* Return if no arguments or first argument has leading dash. */
> - ap = argv[1];
> - if (argc == 1 || *ap == '-')
> - return;
> -
> - /* Allocate space for new arguments. */
> - if ((*argvp = nargv = calloc(argc + 1, sizeof(char *))) == NULL ||
> - (p = flagsp = malloc(strlen(ap) + 2)) == NULL)
> - err(1, NULL);
> -
> - *nargv++ = *argv;
> - argv += 2;
> -
> - for (flags = 0; *ap; ++ap) {
> - switch (*ap) {
> - case 'B':
> - case 'b':
> - case 'd':
> - case 'f':
> - case 'h':
> - case 's':
> - case 'T':
> - if (*argv == NULL) {
> - warnx("option requires an argument -- %c", *ap);
> - usage();
> - }
> - len = 2 + strlen(*argv) + 1;
> - if ((nargv[0] = malloc(len)) == NULL)
> - err(1, NULL);
> - nargv[0][0] = '-';
> - nargv[0][1] = *ap;
> - (void)strlcpy([0][2], *argv, len - 2);
> - ++argv;
> - ++nargv;
> - break;
> - default:
> - if (!flags) {
> - *p++ = '-';
> - flags = 1;
> - }
> - *p++ = *ap;
> - break;
> - }
> - }
> -
> - /* Terminate flags, or toss the buffer we did not use. */
> - if (flags) {
> - *p = '\0';
> - *nargv++ = flagsp;
> - } else
> - free(flagsp);
> -
> - /* Copy remaining arguments. */
> - while ((*nargv++ = *argv++))
> - continue;
> -
> - /* Update argument count. */
> - *argcp = nargv - *argvp - 1;
>  }
> 
> 



dump(8) wording

2022-06-02 Thread Jan Stary
The following wording of dump(8)
can IMHO be be simplified without any loss:

Rewinding or ejecting tape features after a close operation
on a tape device depend on the name of the tape unit device used.

I am not a native speaker; but if I parse that right,
what "features" are those? Rewinding or ejecting?
Then just say that. (Surely dump does not "rewind tape features"
or "eject tape features"). Also, "feature" being both a noun and a verb
requires extra parsing effort, at least for a non-native speaker. So:

Rewinding or ejecting a tape after a close operation
on a tape device depends on the name of the tape unit device.

I don't know the difference between a "tape device"
and a "tape unit device", so I left the "unit" there;
if it's superfluous, it can perhaps be "name of the device".

Also:

dump requires operator intervention on these conditions:
end of tape, end of dump, ...

dump never required any intervention from me on an "end of dump",
it simply says DUMP IS DONE. Would "volume" be more precise here?
I don't use tapes, but I suppose an intervention is in order
at "end of volume" or perhaps "end of media".

Jan



obsolete dump options syntax

2022-06-02 Thread Jan Stary
# dump /home   
dump: option requires an argument -- h

# dump /music
dump: option requires an argument -- s

# dump /media 
dump: option requires an argument -- d

What? Before passing its options to getopt(),
dump's main() processes them with obsolete():

Change set of key letters and ordered arguments
into something getopt(3) will like.

That results in the above. What obsolete options format
is this trying to accomodate? The manpage doesn't say -
the options it describes are perfectly getopt()-likable.
Looking at the CVS log, this was already "obsolete"
in the original 1995 import. Does anyone still use
that undescribed obsolete syntax dump "supports"?

The diff below simply removes the function.
With the diff, dump does the right thing,
i.e. just tries to dump the given fs to rst0.

# dump /home
DUMP: Date of this level 0 dump: Thu Jun  2 14:20:44 2022
DUMP: Date of last level 0 dump: the epoch
DUMP: Dumping /dev/rsd0a (/home) to /dev/rst0
DUMP: mapping (Pass I) [regular files]
DUMP: mapping (Pass II) [directories]
DUMP: estimated 47580129 tape blocks on 1222.00 tape(s).
DUMP: Cannot open output "/dev/rst0".

Jan


Index: main.c
===
RCS file: /cvs/src/sbin/dump/main.c,v
retrieving revision 1.62
diff -u -p -r1.62 main.c
--- main.c  21 Jan 2021 00:16:36 -  1.62
+++ main.c  2 Jun 2022 12:23:56 -
@@ -99,7 +99,6 @@ struct disklabel lab;
 static int sblock_try[] = SBLOCKSEARCH;
 
 static long long numarg(char *, long long, long long);
-static void obsolete(int *, char **[]);
 static void usage(void);
 
 int
@@ -134,7 +133,6 @@ main(int argc, char *argv[])
if (argc < 2)
usage();
 
-   obsolete(, );
while ((ch = getopt(argc, argv, "0123456789aB:b:cd:f:h:ns:ST:uWw")) != 
-1)
switch (ch) {
/* dump level */
@@ -700,80 +698,4 @@ getduid(char *path)
}
 
return (NULL);
-}
-
-/*
- * obsolete --
- * Change set of key letters and ordered arguments into something
- * getopt(3) will like.
- */
-static void
-obsolete(int *argcp, char **argvp[])
-{
-   int argc, flags;
-   char *ap, **argv, *flagsp, **nargv, *p;
-   size_t len;
-
-   /* Setup. */
-   argv = *argvp;
-   argc = *argcp;
-
-   /* Return if no arguments or first argument has leading dash. */
-   ap = argv[1];
-   if (argc == 1 || *ap == '-')
-   return;
-
-   /* Allocate space for new arguments. */
-   if ((*argvp = nargv = calloc(argc + 1, sizeof(char *))) == NULL ||
-   (p = flagsp = malloc(strlen(ap) + 2)) == NULL)
-   err(1, NULL);
-
-   *nargv++ = *argv;
-   argv += 2;
-
-   for (flags = 0; *ap; ++ap) {
-   switch (*ap) {
-   case 'B':
-   case 'b':
-   case 'd':
-   case 'f':
-   case 'h':
-   case 's':
-   case 'T':
-   if (*argv == NULL) {
-   warnx("option requires an argument -- %c", *ap);
-   usage();
-   }
-   len = 2 + strlen(*argv) + 1;
-   if ((nargv[0] = malloc(len)) == NULL)
-   err(1, NULL);
-   nargv[0][0] = '-';
-   nargv[0][1] = *ap;
-   (void)strlcpy([0][2], *argv, len - 2);
-   ++argv;
-   ++nargv;
-   break;
-   default:
-   if (!flags) {
-   *p++ = '-';
-   flags = 1;
-   }
-   *p++ = *ap;
-   break;
-   }
-   }
-
-   /* Terminate flags, or toss the buffer we did not use. */
-   if (flags) {
-   *p = '\0';
-   *nargv++ = flagsp;
-   } else
-   free(flagsp);
-
-   /* Copy remaining arguments. */
-   while ((*nargv++ = *argv++))
-   continue;
-
-   /* Update argument count. */
-   *argcp = nargv - *argvp - 1;
 }



igc vlan hwtagging

2022-06-02 Thread Moritz Buhl
Dear tech@,

the following diff should implement vlan hwtagging for igc.
I would appreciate feedback from further testing.

OK?
mbuhl

Index: sys/dev/pci/if_igc.c
===
RCS file: /cvs/src/sys/dev/pci/if_igc.c,v
retrieving revision 1.9
diff -u -p -r1.9 if_igc.c
--- sys/dev/pci/if_igc.c2 Jun 2022 07:41:17 -   1.9
+++ sys/dev/pci/if_igc.c2 Jun 2022 12:18:14 -
@@ -790,11 +790,9 @@ igc_setup_interface(struct igc_softc *sc
 
ifp->if_capabilities = IFCAP_VLAN_MTU;
 
-#ifdef notyet
 #if NVLAN > 0
ifp->if_capabilities |= IFCAP_VLAN_HWTAGGING;
 #endif
-#endif
 
ifp->if_capabilities |= IFCAP_CSUM_IPv4;
ifp->if_capabilities |= IFCAP_CSUM_TCPv4 | IFCAP_CSUM_UDPv4;
@@ -1007,17 +1005,19 @@ igc_start(struct ifqueue *ifq)
prod &= mask;
}
 
+   cmd_type_len = IGC_ADVTXD_DCMD_IFCS | IGC_ADVTXD_DTYP_DATA |
+   IGC_ADVTXD_DCMD_DEXT;
+
+   if (ISSET(m->m_flags, M_VLANTAG))
+   cmd_type_len |= IGC_ADVTXD_DCMD_VLE;
+
for (i = 0; i < map->dm_nsegs; i++) {
txdesc = >tx_base[prod];
 
-   cmd_type_len = IGC_ADVTXD_DCMD_IFCS | 
IGC_ADVTXD_DTYP_DATA |
-   IGC_ADVTXD_DCMD_DEXT | map->dm_segs[i].ds_len;
-   if (i == map->dm_nsegs - 1)
-   cmd_type_len |= IGC_ADVTXD_DCMD_EOP |
-   IGC_ADVTXD_DCMD_RS;
-
htolem64(>read.buffer_addr, 
map->dm_segs[i].ds_addr);
-   htolem32(>read.cmd_type_len, cmd_type_len);
+   htolem32(>read.cmd_type_len, cmd_type_len |
+   map->dm_segs[i].ds_len | ((i == map->dm_nsegs - 1)?
+   IGC_ADVTXD_DCMD_EOP | IGC_ADVTXD_DCMD_RS : 0));
htolem32(>read.olinfo_status, olinfo_status);
 
last = prod;
@@ -2006,10 +2006,10 @@ igc_tx_ctx_setup(struct tx_ring *txr, st
struct mbuf *m;
uint32_t type_tucmd_mlhl = 0;
uint32_t vlan_macip_lens = 0;
-   uint32_t iphlen;
+   uint32_t iphlen = 0;
int hoff;
int off = 0;
-   uint8_t ipproto;
+   uint8_t ipproto = 0;
 
vlan_macip_lens |= (sizeof(*eh) << IGC_ADVTXD_MACLEN_SHIFT);
 
@@ -2018,7 +2018,6 @@ igc_tx_ctx_setup(struct tx_ring *txr, st
 * be placed into the context descriptor. Hence
 * we need to make one even if not doing offloads.
 */
-#ifdef notyet
 #if NVLAN > 0
if (ISSET(mp->m_flags, M_VLANTAG)) {
uint32_t vtag = mp->m_pkthdr.ether_vtag;
@@ -2026,7 +2025,6 @@ igc_tx_ctx_setup(struct tx_ring *txr, st
off = 1;
}
 #endif
-#endif
 
switch (ntohs(eh->ether_type)) {
case ETHERTYPE_IP: {
@@ -2062,8 +2060,6 @@ igc_tx_ctx_setup(struct tx_ring *txr, st
break;
}
 #endif
-   default:
-   return 0;
}
 
vlan_macip_lens |= iphlen;



Re: libcrypto: altering tbs sigalg on X509 and X509_CRL

2022-06-02 Thread Theo Buehler
On Wed, Jun 01, 2022 at 11:00:12AM +1000, Alex Wilson wrote:
> I'm trying to sign X509 and X509_CRL objects without using X509_sign et al
> -- since the key I'm signing with isn't in the memory of the process doing
> this and you can't write an ENGINE for something that can't sign a
> pre-calculated hash and needs the whole data.
> 
> To do this I have to alter the signing algorithm, both in the outer X509 and
> the inner X509_CINF (and similar on the X509_CRL). Since X509 and X509_CRL
> became opaque, I've had to use some hacks to achieve this:
> 
> For the outer signature and algo I can use X509_get0_signature() and
> X509_CRL_get0_signature() and then cast off the const on the X509_ALGOR.
> Then I can use X509_ALGOR_set0() to set the algorithm in use.
>
> For the X509 inner algorithm I can use X509_get0_tbs_sigalg() and cast the
> const off there, too. But there is no equivalent of this for X509_CRL.

This sounds quite familiar...

> As a result I have a local patch which adds X509_get_tbs_sigalg() (returning
> a non-const pointer) and X509_CRL_get{,0}_tbs_sigalg(). It's very short, so
> I was hoping somebody could let me know if this is a terrible idea and
> whether there is any possibility of it going back in LibreSSL (or if I
> should try to take it up with OpenSSL and then port it back or something?)
>
> I have also considered adding an X509_get_signature() which returns
> non-const, as well. If any of this seems like a decent idea at all, I'm
> happy to add that and write some tests.

I would be ok with adding X509_CRL_get0_tbs_sigalg(), as this looks like
a straight up oversight. Since OpenSSL are quite liberal with adding
accessors, this should also be easy to upstream.

That one function should be good enough for your needs even if it means
some annoying casting away of const. It seems a bit of a fringe use
case, so I would rather not add more functions to the public API for
this.

As an aside, using _get_ is unfortunate and I'd rather avoid it. This
could mean get0, get1, have const or no const, depending on the phase of
Venus... I don't know OpenSSL's current stance on such cases, i.e.,
whether they added more stuff like X509_CRL_get0_tbs_sigalg_noconst().

For now I'd suggest you send a patch for X509_CRL_get0_tbs_sigalg()
with the prototype gated with #if defined(LIBRESSL_CRYPTO_INTERNAL). We
would then expose that with the next library bump, which will almost
certainly happen before 3.6 goes -stable.

If you want to add a short regress, that would be great, but it's not
strictly required for a simple accessor. I'd probably extend
regress/lib/libcrypto/x509/. You'll want static linking and you'll need
to extend CFLAGS with something like

LDADD_foo = ${CRYPTO_INT}
CFLAGS += -DLIBRESSL_CRYPTO_INTERNAL

> 
> Brief patch attached.

> commit 6f3f7990686517b788278fc26e706e2f0c7472cf
> Author: Alex Wilson 
> Date:   Wed Jun 1 10:49:08 2022 +1000
> 
> libcrypto: want to alter tbs sigalg for X509 and X509_CRL
> 
> diff lib/libcrypto/asn1/x_crl.c lib/libcrypto/asn1/x_crl.c
> --- lib/libcrypto/asn1/x_crl.c
> +++ lib/libcrypto/asn1/x_crl.c
> @@ -722,6 +722,18 @@ X509_CRL_get_lastUpdate(X509_CRL *crl)
>   return crl->crl->lastUpdate;
>  }
>  
> +const X509_ALGOR *
> +X509_CRL_get0_tbs_sigalg(const X509_CRL *crl)
> +{
> + return crl->crl->sig_alg;
> +}
> +
> +X509_ALGOR *
> +X509_CRL_get_tbs_sigalg(X509_CRL *crl)
> +{
> + return crl->crl->sig_alg;
> +}
> +
>  const ASN1_TIME *
>  X509_CRL_get0_nextUpdate(const X509_CRL *crl)
>  {
> diff lib/libcrypto/x509/x509.h lib/libcrypto/x509/x509.h
> --- lib/libcrypto/x509/x509.h
> +++ lib/libcrypto/x509/x509.h
> @@ -399,6 +399,8 @@ X509_NAME *X509_CRL_get_issuer(const X509_CRL *crl);
>  STACK_OF(X509_REVOKED) *X509_CRL_get_REVOKED(X509_CRL *crl);
>  void X509_CRL_get0_signature(const X509_CRL *crl, const ASN1_BIT_STRING 
> **psig,
>  const X509_ALGOR **palg);
> +const X509_ALGOR *X509_CRL_get0_tbs_sigalg(const X509_CRL *crl);
> +X509_ALGOR *X509_CRL_get_tbs_sigalg(X509_CRL *crl);
>  
>  int X509_REQ_get_signature_nid(const X509_REQ *req);
>  
> @@ -768,6 +770,7 @@ int ASN1_item_sign_ctx(const ASN1_ITEM *it,
>  
>  const STACK_OF(X509_EXTENSION) *X509_get0_extensions(const X509 *x);
>  const X509_ALGOR *X509_get0_tbs_sigalg(const X509 *x);
> +X509_ALGOR * X509_get_tbs_sigalg(X509 *x);
>  int  X509_set_version(X509 *x, long version);
>  long X509_get_version(const X509 *x);
>  int  X509_set_serialNumber(X509 *x, ASN1_INTEGER *serial);
> diff lib/libcrypto/x509/x509_set.c lib/libcrypto/x509/x509_set.c
> --- lib/libcrypto/x509/x509_set.c
> +++ lib/libcrypto/x509/x509_set.c
> @@ -77,6 +77,12 @@ X509_get0_tbs_sigalg(const X509 *x)
>   return x->cert_info->signature;
>  }
>  
> +X509_ALGOR *
> +X509_get_tbs_sigalg(X509 *x)
> +{
> + return x->cert_info->signature;
> +}
> +
>  int
>  X509_set_version(X509 *x, long version)
>  {



Re: httpd: add include_dir keyword

2022-06-02 Thread Stuart Henderson
On 2022/06/02 12:53, qorg11 wrote:
> > I don't think we want this functionality.
> 
> Some users have been asking for it in the #openbsd IRC channel.

there are 20+ programs in base which use a config parser derived from
the same source as usr/sbin/httpd's, and generally they are kept in
sync as much as possible. having them diverge by allowing some but not
others to pull in a whole directory's worth of config files isn't
entirely ideal..

On 2022/06/02 12:04, Florian Obser wrote:
> this should be
> "include directory"
> or
> "include /etc/httpd.d/"
> should be made to work.  

or glob.. "include /etc/httpd.d/*"



Re: httpd: add include_dir keyword

2022-06-02 Thread qorg11
Ugh, this is awkward.
Index: httpd.conf.5
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
retrieving revision 1.121
diff -u -p -u -p -r1.121 httpd.conf.5
--- httpd.conf.5	9 Mar 2022 13:50:41 -	1.121
+++ httpd.conf.5	2 Jun 2022 11:21:31 -
@@ -84,6 +84,12 @@ keyword, for example:
 .Bd -literal -offset indent
 include "/etc/httpd.conf.local"
 .Ed
+A directory with configuration files can be included with the
+.Ic include_dir
+keyword, for example:
+.Bd -literal -offset indent
+include_dir "directory"
+.Ed
 .Sh MACROS
 Macros can be defined that will later be expanded in context.
 Macro names must start with a letter, digit, or underscore,
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
retrieving revision 1.128
diff -u -p -u -p -r1.128 parse.y
--- parse.y	27 Feb 2022 20:30:30 -	1.128
+++ parse.y	2 Jun 2022 11:21:31 -
@@ -52,6 +52,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "httpd.h"
 #include "http.h"
@@ -139,7 +140,7 @@ typedef struct {
 %token	LISTEN LOCATION LOG LOGDIR MATCH MAXIMUM NO NODELAY OCSP ON PORT PREFORK
 %token	PROTOCOLS REQUESTS ROOT SACK SERVER SOCKET STRIP STYLE SYSLOG TCP TICKET
 %token	TIMEOUT TLS TYPE TYPES HSTS MAXAGE SUBDOMAINS DEFAULT PRELOAD REQUEST
-%token	ERROR INCLUDE AUTHENTICATE WITH BLOCK DROP RETURN PASS REWRITE
+%token	ERROR INCLUDE INCLUDE_DIR AUTHENTICATE WITH BLOCK DROP RETURN PASS REWRITE
 %token	CA CLIENT CRL OPTIONAL PARAM FORWARDED FOUND NOT
 %token	ERRDOCS GZIPSTATIC
 %token		STRING
@@ -155,6 +156,7 @@ typedef struct {
 
 grammar		: /* empty */
 		| grammar include '\n'
+		| grammar include_dir '\n'
 		| grammar '\n'
 		| grammar varset '\n'
 		| grammar main '\n'
@@ -165,7 +167,6 @@ grammar		: /* empty */
 
 include		: INCLUDE STRING		{
 			struct file	*nfile;
-
 			if ((nfile = pushfile($2, 0)) == NULL) {
 yyerror("failed to include file %s", $2);
 free($2);
@@ -178,6 +179,48 @@ include		: INCLUDE STRING		{
 		}
 		;
 
+include_dir : INCLUDE_DIR STRING {
+			char absolute_path[PATH_MAX];
+			char dir[PATH_MAX];
+			struct file *nfile;
+			DIR *opened_dir;
+			opened_dir = openddir($2);
+			struct dirent *entry;
+			size_t len = 0;
+
+			if(opened_dir == NULL) {
+free($2);
+yyerror("Failed to open directory %s", $2);
+YYERROR;
+			}
+
+			len = strlcpy(dir, $2, PATH_MAX);
+
+			if(len >= sizeof(dir)) {
+free($2);
+yyerror("too long");
+YYERROR;
+			}
+
+			while((entry = readdir(opened_dir))) {
+if(entry->d_name[0] == '.')
+	continue;
+len = snprintf(absolute_path, PATH_MAX, "%s%s", dir, entry->d_name);
+if(len < 0 || len >= sizeof(absolute_path)) {
+	yyerror("too long");
+	YYERROR;
+}
+if((nfile = pushfile(absolute_path, 0)) == NULL) {
+	yyerror("failed to include file %s", $2);
+	YYERROR;
+}
+			}
+
+			file = nfile;
+			lungetc('\n');
+}
+;
+
 varset		: STRING '=' STRING	{
 			char *s = $1;
 			while (*s++) {
@@ -1453,6 +1496,7 @@ lookup(char *s)
 		{ "gzip-static",	GZIPSTATIC },
 		{ "hsts",		HSTS },
 		{ "include",		INCLUDE },
+		{ "include_dir",	INCLUDE_DIR},
 		{ "index",		INDEX },
 		{ "ip",			IP },
 		{ "key",		KEY },


Re: bgpd cleanup RTP_ limit checks in parse.y

2022-06-02 Thread Theo Buehler
On Thu, Jun 02, 2022 at 01:07:07PM +0200, Claudio Jeker wrote:
> On Thu, Jun 02, 2022 at 12:44:49PM +0200, Theo Buehler wrote:
> > On Thu, Jun 02, 2022 at 11:38:05AM +0200, Claudio Jeker wrote:
> > > Lets use the same check for both priority checks in parse.y.
> > > Also rephrase the error messages to be less cryptic.
> > > Both checks do the same check since RTP_NONE = 0 and RTP_LOCAL = 1.
> > 
> > ok
> > 
> > > Using RTP_LOCAL as a priority is actually not possible since that one is
> > > reserved for the kernel (used by interface address entries). So maybe the
> > > check should be 'if ($2 <= RTP_LOCAL'. That would be a followup diff since
> > > that would change behaviour.
> > 
> > That would make sense to me.
> 
> See below for that.

ok

> 
> -- 
> :wq Claudio
> 
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.427
> diff -u -p -r1.427 parse.y
> --- parse.y   2 Jun 2022 11:05:15 -   1.427
> +++ parse.y   2 Jun 2022 11:06:10 -
> @@ -707,9 +707,9 @@ conf_main : AS as4number  {
>   TAILQ_INSERT_TAIL(conf->listen_addrs, la, entry);
>   }
>   | FIBPRIORITY NUMBER{
> - if ($2 < RTP_LOCAL || $2 > RTP_MAX) {
> + if ($2 <= RTP_LOCAL || $2 > RTP_MAX) {
>   yyerror("fib-priority %lld must be between "
> - "%u and %u", $2, RTP_LOCAL, RTP_MAX);
> + "%u and %u", $2, RTP_LOCAL + 1, RTP_MAX);
>   YYERROR;
>   }
>   conf->fib_priority = $2;
> @@ -1045,9 +1045,9 @@ network : NETWORK prefix filter_set {
>   }
>   | NETWORK family PRIORITY NUMBER filter_set {
>   struct network  *n;
> - if ($4 < RTP_LOCAL && $4 > RTP_MAX) {
> + if ($4 <= RTP_LOCAL && $4 > RTP_MAX) {
>   yyerror("priority %lld must be between "
> - "%u and %u", $4, RTP_LOCAL, RTP_MAX);
> + "%u and %u", $4, RTP_LOCAL + 1, RTP_MAX);
>   YYERROR;
>   }
>  



Re: bgpd cleanup RTP_ limit checks in parse.y

2022-06-02 Thread Claudio Jeker
On Thu, Jun 02, 2022 at 12:44:49PM +0200, Theo Buehler wrote:
> On Thu, Jun 02, 2022 at 11:38:05AM +0200, Claudio Jeker wrote:
> > Lets use the same check for both priority checks in parse.y.
> > Also rephrase the error messages to be less cryptic.
> > Both checks do the same check since RTP_NONE = 0 and RTP_LOCAL = 1.
> 
> ok
> 
> > Using RTP_LOCAL as a priority is actually not possible since that one is
> > reserved for the kernel (used by interface address entries). So maybe the
> > check should be 'if ($2 <= RTP_LOCAL'. That would be a followup diff since
> > that would change behaviour.
> 
> That would make sense to me.

See below for that.

-- 
:wq Claudio

Index: parse.y
===
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.427
diff -u -p -r1.427 parse.y
--- parse.y 2 Jun 2022 11:05:15 -   1.427
+++ parse.y 2 Jun 2022 11:06:10 -
@@ -707,9 +707,9 @@ conf_main   : AS as4number  {
TAILQ_INSERT_TAIL(conf->listen_addrs, la, entry);
}
| FIBPRIORITY NUMBER{
-   if ($2 < RTP_LOCAL || $2 > RTP_MAX) {
+   if ($2 <= RTP_LOCAL || $2 > RTP_MAX) {
yyerror("fib-priority %lld must be between "
-   "%u and %u", $2, RTP_LOCAL, RTP_MAX);
+   "%u and %u", $2, RTP_LOCAL + 1, RTP_MAX);
YYERROR;
}
conf->fib_priority = $2;
@@ -1045,9 +1045,9 @@ network   : NETWORK prefix filter_set {
}
| NETWORK family PRIORITY NUMBER filter_set {
struct network  *n;
-   if ($4 < RTP_LOCAL && $4 > RTP_MAX) {
+   if ($4 <= RTP_LOCAL && $4 > RTP_MAX) {
yyerror("priority %lld must be between "
-   "%u and %u", $4, RTP_LOCAL, RTP_MAX);
+   "%u and %u", $4, RTP_LOCAL + 1, RTP_MAX);
YYERROR;
}
 



Re: httpd: add include_dir keyword

2022-06-02 Thread qorg11
Ignore that last patch. It has a wrong indentation in an if block.

Index: httpd.conf.5
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
retrieving revision 1.121
diff -u -p -u -p -r1.121 httpd.conf.5
--- httpd.conf.5	9 Mar 2022 13:50:41 -	1.121
+++ httpd.conf.5	2 Jun 2022 11:02:21 -
@@ -84,6 +84,12 @@ keyword, for example:
 .Bd -literal -offset indent
 include "/etc/httpd.conf.local"
 .Ed
+A directory with configuration files can be included with the
+.Ic include_dir
+keyword, for example:
+.Bd -literal -offset indent
+include_dir "directory"
+.Ed
 .Sh MACROS
 Macros can be defined that will later be expanded in context.
 Macro names must start with a letter, digit, or underscore,
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
retrieving revision 1.128
diff -u -p -u -p -r1.128 parse.y
--- parse.y	27 Feb 2022 20:30:30 -	1.128
+++ parse.y	2 Jun 2022 11:02:21 -
@@ -52,6 +52,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "httpd.h"
 #include "http.h"
@@ -139,7 +140,7 @@ typedef struct {
 %token	LISTEN LOCATION LOG LOGDIR MATCH MAXIMUM NO NODELAY OCSP ON PORT PREFORK
 %token	PROTOCOLS REQUESTS ROOT SACK SERVER SOCKET STRIP STYLE SYSLOG TCP TICKET
 %token	TIMEOUT TLS TYPE TYPES HSTS MAXAGE SUBDOMAINS DEFAULT PRELOAD REQUEST
-%token	ERROR INCLUDE AUTHENTICATE WITH BLOCK DROP RETURN PASS REWRITE
+%token	ERROR INCLUDE INCLUDE_DIR AUTHENTICATE WITH BLOCK DROP RETURN PASS REWRITE
 %token	CA CLIENT CRL OPTIONAL PARAM FORWARDED FOUND NOT
 %token	ERRDOCS GZIPSTATIC
 %token		STRING
@@ -155,6 +156,7 @@ typedef struct {
 
 grammar		: /* empty */
 		| grammar include '\n'
+		| grammar include_dir '\n'
 		| grammar '\n'
 		| grammar varset '\n'
 		| grammar main '\n'
@@ -165,7 +167,6 @@ grammar		: /* empty */
 
 include		: INCLUDE STRING		{
 			struct file	*nfile;
-
 			if ((nfile = pushfile($2, 0)) == NULL) {
 yyerror("failed to include file %s", $2);
 free($2);
@@ -178,6 +179,48 @@ include		: INCLUDE STRING		{
 		}
 		;
 
+include_dir : INCLUDE_DIR STRING {
+			char absolute_path[PATH_MAX];
+			char dir[PATH_MAX];
+			struct file *nfile;
+			DIR *opened_dir;
+			opened_dir = openddir($2);
+			struct dirent *entry;
+			size_t len = 0;
+
+			if(opened_dir == NULL) {
+free($2);
+yyerror("Failed to open directory %s", $2);
+YYERROR;
+			}
+
+			len = strlcpy(dir, $2, PATH_MAX);
+
+			if(len >= sizeof(dir)) {
+			  free($2);
+yyerror("too long");
+YYERROR;
+			}
+
+			while((entry = readdir(opened_dir))) {
+  if(entry->d_name[0] == '.')
+	continue;
+len = snprintf(absolute_path, PATH_MAX, "%s%s", dir, entry->d_name);
+if(len < 0 || len >= sizeof(absolute_path)) {
+	yyerror("too long");
+	YYERROR;
+}
+if((nfile = pushfile(absolute_path, 0)) == NULL) {
+	yyerror("failed to include file %s", $2);
+	YYERROR;
+}
+			}
+
+			file = nfile;
+			lungetc('\n');
+}
+;
+
 varset		: STRING '=' STRING	{
 			char *s = $1;
 			while (*s++) {
@@ -1453,6 +1496,7 @@ lookup(char *s)
 		{ "gzip-static",	GZIPSTATIC },
 		{ "hsts",		HSTS },
 		{ "include",		INCLUDE },
+		{ "include_dir",	INCLUDE_DIR},
 		{ "index",		INDEX },
 		{ "ip",			IP },
 		{ "key",		KEY },


Re: httpd: add include_dir keyword

2022-06-02 Thread qorg11
> I don't think we want this functionality.

Some users have been asking for it in the #openbsd IRC channel.

In any case, I have fixed the patch file.
Index: httpd.conf.5
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
retrieving revision 1.121
diff -u -p -u -p -r1.121 httpd.conf.5
--- httpd.conf.5	9 Mar 2022 13:50:41 -	1.121
+++ httpd.conf.5	2 Jun 2022 10:51:43 -
@@ -84,6 +84,12 @@ keyword, for example:
 .Bd -literal -offset indent
 include "/etc/httpd.conf.local"
 .Ed
+A directory with configuration files can be included with the
+.Ic include_dir
+keyword, for example:
+.Bd -literal -offset indent
+include_dir "directory"
+.Ed
 .Sh MACROS
 Macros can be defined that will later be expanded in context.
 Macro names must start with a letter, digit, or underscore,
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
retrieving revision 1.128
diff -u -p -u -p -r1.128 parse.y
--- parse.y	27 Feb 2022 20:30:30 -	1.128
+++ parse.y	2 Jun 2022 10:51:43 -
@@ -52,6 +52,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "httpd.h"
 #include "http.h"
@@ -139,7 +140,7 @@ typedef struct {
 %token	LISTEN LOCATION LOG LOGDIR MATCH MAXIMUM NO NODELAY OCSP ON PORT PREFORK
 %token	PROTOCOLS REQUESTS ROOT SACK SERVER SOCKET STRIP STYLE SYSLOG TCP TICKET
 %token	TIMEOUT TLS TYPE TYPES HSTS MAXAGE SUBDOMAINS DEFAULT PRELOAD REQUEST
-%token	ERROR INCLUDE AUTHENTICATE WITH BLOCK DROP RETURN PASS REWRITE
+%token	ERROR INCLUDE INCLUDE_DIR AUTHENTICATE WITH BLOCK DROP RETURN PASS REWRITE
 %token	CA CLIENT CRL OPTIONAL PARAM FORWARDED FOUND NOT
 %token	ERRDOCS GZIPSTATIC
 %token		STRING
@@ -155,6 +156,7 @@ typedef struct {
 
 grammar		: /* empty */
 		| grammar include '\n'
+		| grammar include_dir '\n'
 		| grammar '\n'
 		| grammar varset '\n'
 		| grammar main '\n'
@@ -165,7 +167,6 @@ grammar		: /* empty */
 
 include		: INCLUDE STRING		{
 			struct file	*nfile;
-
 			if ((nfile = pushfile($2, 0)) == NULL) {
 yyerror("failed to include file %s", $2);
 free($2);
@@ -178,6 +179,48 @@ include		: INCLUDE STRING		{
 		}
 		;
 
+include_dir : INCLUDE_DIR STRING {
+			char absolute_path[PATH_MAX];
+			char dir[PATH_MAX];
+			struct file *nfile;
+			DIR *opened_dir;
+			opened_dir = openddir($2);
+			struct dirent *entry;
+			size_t len = 0;
+
+			if(opened_dir == NULL) {
+free($2);
+yyerror("Failed to open directory %s", $2);
+YYERROR;
+			}
+
+			len = strlcpy(dir, $2, PATH_MAX);
+
+			if(len >= sizeof(dir)) {
+			  free($2);
+yyerror("too long");
+YYERROR;
+			}
+
+			while((entry = readdir(opened_dir))) {
+  if(entry->d_name[0] == '.')
+	continue;
+len = snprintf(absolute_path, PATH_MAX, "%s%s", dir, entry->d_name);
+if(len < 0 || len >= sizeof(absolute_path)) {
+	yyerror("too long");
+	YYERROR;
+}
+if((nfile = pushfile(absolute_path, 0)) == NULL) {
+yyerror("failed to include file %s", $2);
+	YYERROR;
+}
+			}
+
+			file = nfile;
+			lungetc('\n');
+}
+;
+
 varset		: STRING '=' STRING	{
 			char *s = $1;
 			while (*s++) {
@@ -1453,6 +1496,7 @@ lookup(char *s)
 		{ "gzip-static",	GZIPSTATIC },
 		{ "hsts",		HSTS },
 		{ "include",		INCLUDE },
+		{ "include_dir",	INCLUDE_DIR},
 		{ "index",		INDEX },
 		{ "ip",			IP },
 		{ "key",		KEY },


Re: bgpd cleanup RTP_ limit checks in parse.y

2022-06-02 Thread Theo Buehler
On Thu, Jun 02, 2022 at 11:38:05AM +0200, Claudio Jeker wrote:
> Lets use the same check for both priority checks in parse.y.
> Also rephrase the error messages to be less cryptic.
> Both checks do the same check since RTP_NONE = 0 and RTP_LOCAL = 1.

ok

> Using RTP_LOCAL as a priority is actually not possible since that one is
> reserved for the kernel (used by interface address entries). So maybe the
> check should be 'if ($2 <= RTP_LOCAL'. That would be a followup diff since
> that would change behaviour.

That would make sense to me.

> 
> -- 
> :wq Claudio
> 
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.426
> diff -u -p -r1.426 parse.y
> --- parse.y   2 Jun 2022 09:29:34 -   1.426
> +++ parse.y   2 Jun 2022 09:30:34 -
> @@ -707,8 +707,9 @@ conf_main : AS as4number  {
>   TAILQ_INSERT_TAIL(conf->listen_addrs, la, entry);
>   }
>   | FIBPRIORITY NUMBER{
> - if ($2 <= RTP_NONE || $2 > RTP_MAX) {
> - yyerror("invalid fib-priority");
> + if ($2 < RTP_LOCAL || $2 > RTP_MAX) {
> + yyerror("fib-priority %lld must be between "
> + "%u and %u", $2, RTP_LOCAL, RTP_MAX);
>   YYERROR;
>   }
>   conf->fib_priority = $2;
> @@ -1045,8 +1046,8 @@ network : NETWORK prefix filter_set {
>   | NETWORK family PRIORITY NUMBER filter_set {
>   struct network  *n;
>   if ($4 < RTP_LOCAL && $4 > RTP_MAX) {
> - yyerror("priority %lld > max %d or < min %d", 
> $4,
> - RTP_MAX, RTP_LOCAL);
> + yyerror("priority %lld must be between "
> + "%u and %u", $4, RTP_LOCAL, RTP_MAX);
>   YYERROR;
>   }
>  
> 



Re: httpd: add include_dir keyword

2022-06-02 Thread Florian Obser
On 2022-06-02 11:04 +02, qorg11  wrote:
> This patch addes the "inlcude_dir" keyword for httpd.conf. Which works
> just like "include" but it includes all the files in a directory, for
> example: include "/etc/httpd.d"
>
> The diff file is attatched.

I don't think we want this functionality.

More inline.

>
> Index: httpd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
> retrieving revision 1.121
> diff -u -p -u -p -r1.121 httpd.conf.5
> --- httpd.conf.5  9 Mar 2022 13:50:41 -   1.121
> +++ httpd.conf.5  2 Jun 2022 09:02:22 -
> @@ -84,6 +84,12 @@ keyword, for example:
>  .Bd -literal -offset indent
>  include "/etc/httpd.conf.local"
>  .Ed
> +A directory with configuration files can be included with the
> +.Ic include_dir
> +keyword, for example:
> +.Bd -literal -offset indent
> +include_dir "/etc/httpd.conf.d"

this should be
"include directory"
or
"include /etc/httpd.d/"
should be made to work.

> +.Ed
>  .Sh MACROS
>  Macros can be defined that will later be expanded in context.
>  Macro names must start with a letter, digit, or underscore,
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> retrieving revision 1.128
> diff -u -p -u -p -r1.128 parse.y
> --- parse.y   27 Feb 2022 20:30:30 -  1.128
> +++ parse.y   2 Jun 2022 09:02:22 -
> @@ -52,6 +52,7 @@
>  #include 
>  #include 
>  #include 
> +#include 

trailing whitespace

>  
>  #include "httpd.h"
>  #include "http.h"
> @@ -139,7 +140,7 @@ typedef struct {
>  %token   LISTEN LOCATION LOG LOGDIR MATCH MAXIMUM NO NODELAY OCSP ON 
> PORT PREFORK
>  %token   PROTOCOLS REQUESTS ROOT SACK SERVER SOCKET STRIP STYLE SYSLOG 
> TCP TICKET
>  %token   TIMEOUT TLS TYPE TYPES HSTS MAXAGE SUBDOMAINS DEFAULT PRELOAD 
> REQUEST
> -%token   ERROR INCLUDE AUTHENTICATE WITH BLOCK DROP RETURN PASS REWRITE
> +%token   ERROR INCLUDE INCLUDE_DIR AUTHENTICATE WITH BLOCK DROP RETURN 
> PASS REWRITE
>  %token   CA CLIENT CRL OPTIONAL PARAM FORWARDED FOUND NOT
>  %token   ERRDOCS GZIPSTATIC
>  %token STRING
> @@ -155,6 +156,7 @@ typedef struct {
>  
>  grammar  : /* empty */
>   | grammar include '\n'
> + | grammar include_dir '\n'
>   | grammar '\n'
>   | grammar varset '\n'
>   | grammar main '\n'
> @@ -165,7 +167,6 @@ grammar   : /* empty */
>  
>  include  : INCLUDE STRING{
>   struct file *nfile;
> -
>   if ((nfile = pushfile($2, 0)) == NULL) {
>   yyerror("failed to include file %s", $2);
>   free($2);
> @@ -178,6 +179,46 @@ include  : INCLUDE STRING{
>   }
>   ;
>

the following block has some weird tab space tab indent

> +include_dir : INCLUDE_DIR STRING {
> + char absolute_path[PATH_MAX];
> + char dir[PATH_MAX];
> + struct file *nfile;
> + DIR *opened_dir = opendir($2);

do not initialize a variable with a function call when declaring it

> + struct dirent *entry;
> +
> + if(opened_dir == NULL) {
> + free($2);
> + yyerror("Failed to open directory %s", $2);
> + YYERROR;
> + }
> +
> + size_t len = strlcpy(dir, $2, PATH_MAX);

no declaration in a middle of a block

> +
> + if(len >= sizeof(dir)) {
> + free($2);
> + yyerror("too long");
> + YYERROR;
> + }
> +
> + while((entry = readdir(opened_dir))) {
> + if(entry->d_name[0] == '.')
> + continue;

wrong indent

> + len = 
> snprintf(absolute_path,PATH_MAX,"%s%s",dir, entry->d_name);

missing spaces after ","

> + if(len < 0|| len >= sizeof(absolute_path)) {

missing space ^

> + yyerror("too long");
> + YYERROR;
> + }
> + if((nfile = pushfile(absolute_path, 0)) == 
> NULL) {
> + yyerror("failed to include file %s", 
> $2);
> + YYERROR;
> + }
> + }
> +
> + file = nfile;
> + lungetc('\n');
> +}
> +;
> +
>  varset   : STRING '=' STRING {
>   char *s = $1;
>   

bgpd cleanup RTP_ limit checks in parse.y

2022-06-02 Thread Claudio Jeker
Lets use the same check for both priority checks in parse.y.
Also rephrase the error messages to be less cryptic.
Both checks do the same check since RTP_NONE = 0 and RTP_LOCAL = 1.
Using RTP_LOCAL as a priority is actually not possible since that one is
reserved for the kernel (used by interface address entries). So maybe the
check should be 'if ($2 <= RTP_LOCAL'. That would be a followup diff since
that would change behaviour.

-- 
:wq Claudio

Index: parse.y
===
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.426
diff -u -p -r1.426 parse.y
--- parse.y 2 Jun 2022 09:29:34 -   1.426
+++ parse.y 2 Jun 2022 09:30:34 -
@@ -707,8 +707,9 @@ conf_main   : AS as4number  {
TAILQ_INSERT_TAIL(conf->listen_addrs, la, entry);
}
| FIBPRIORITY NUMBER{
-   if ($2 <= RTP_NONE || $2 > RTP_MAX) {
-   yyerror("invalid fib-priority");
+   if ($2 < RTP_LOCAL || $2 > RTP_MAX) {
+   yyerror("fib-priority %lld must be between "
+   "%u and %u", $2, RTP_LOCAL, RTP_MAX);
YYERROR;
}
conf->fib_priority = $2;
@@ -1045,8 +1046,8 @@ network   : NETWORK prefix filter_set {
| NETWORK family PRIORITY NUMBER filter_set {
struct network  *n;
if ($4 < RTP_LOCAL && $4 > RTP_MAX) {
-   yyerror("priority %lld > max %d or < min %d", 
$4,
-   RTP_MAX, RTP_LOCAL);
+   yyerror("priority %lld must be between "
+   "%u and %u", $4, RTP_LOCAL, RTP_MAX);
YYERROR;
}
 



Re: bgpd, check ktable_exists return value

2022-06-02 Thread Claudio Jeker
On Thu, Jun 02, 2022 at 11:13:31AM +0200, Theo Buehler wrote:
> On Thu, Jun 02, 2022 at 11:05:26AM +0200, Claudio Jeker wrote:
> > When setting the default routing table for bgpd make sure that
> > ktable_exists() does not fail.
> > Also improve the warning message in ktable_exists() a bit.
> 
> Sure, ok.
> 
> The existing checks in parse.y do 'if (ktable_exists(..) != 1)' and the
> check in kroute.c uses 'if (!ktable_exists(..))'. Maybe make them all
> use the same variant?

I will switch them all to if (!ktable_exists(..)) which is more natural
then the != 1 version. That is also how the kroute.c code uses it.
 
> > 
> > -- 
> > :wq Claudio
> > 
> > Index: kroute.c
> > ===
> > RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
> > retrieving revision 1.246
> > diff -u -p -r1.246 kroute.c
> > --- kroute.c23 May 2022 13:40:12 -  1.246
> > +++ kroute.c2 Jun 2022 08:59:10 -
> > @@ -440,7 +440,7 @@ ktable_exists(u_int rtableid, u_int *rdo
> > if (errno == ENOENT)
> > /* table nonexistent */
> > return (0);
> > -   log_warn("%s: sysctl", __func__);
> > +   log_warn("sysctl net.route.rtableid");
> > /* must return 0 so that the table is considered non-existent */
> > return (0);
> > }
> > Index: parse.y
> > ===
> > RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> > retrieving revision 1.425
> > diff -u -p -r1.425 parse.y
> > --- parse.y 31 May 2022 09:45:33 -  1.425
> > +++ parse.y 2 Jun 2022 08:53:43 -
> > @@ -3496,7 +3496,9 @@ init_config(struct bgpd_config *c)
> > c->bgpid = get_bgpid();
> > c->fib_priority = RTP_BGP;
> > c->default_tableid = getrtable();
> > -   ktable_exists(c->default_tableid, );
> > +   if (!ktable_exists(c->default_tableid, ))
> > +   fatalx("current routing table %u does not exist",
> > +   c->default_tableid);
> > if (rdomid != c->default_tableid)
> > fatalx("current routing table %u is not a routing domain",
> > c->default_tableid);
> > 
> 

-- 
:wq Claudio



Re: bgpd, check ktable_exists return value

2022-06-02 Thread Theo Buehler
On Thu, Jun 02, 2022 at 11:05:26AM +0200, Claudio Jeker wrote:
> When setting the default routing table for bgpd make sure that
> ktable_exists() does not fail.
> Also improve the warning message in ktable_exists() a bit.

Sure, ok.

The existing checks in parse.y do 'if (ktable_exists(..) != 1)' and the
check in kroute.c uses 'if (!ktable_exists(..))'. Maybe make them all
use the same variant?

> 
> -- 
> :wq Claudio
> 
> Index: kroute.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
> retrieving revision 1.246
> diff -u -p -r1.246 kroute.c
> --- kroute.c  23 May 2022 13:40:12 -  1.246
> +++ kroute.c  2 Jun 2022 08:59:10 -
> @@ -440,7 +440,7 @@ ktable_exists(u_int rtableid, u_int *rdo
>   if (errno == ENOENT)
>   /* table nonexistent */
>   return (0);
> - log_warn("%s: sysctl", __func__);
> + log_warn("sysctl net.route.rtableid");
>   /* must return 0 so that the table is considered non-existent */
>   return (0);
>   }
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.425
> diff -u -p -r1.425 parse.y
> --- parse.y   31 May 2022 09:45:33 -  1.425
> +++ parse.y   2 Jun 2022 08:53:43 -
> @@ -3496,7 +3496,9 @@ init_config(struct bgpd_config *c)
>   c->bgpid = get_bgpid();
>   c->fib_priority = RTP_BGP;
>   c->default_tableid = getrtable();
> - ktable_exists(c->default_tableid, );
> + if (!ktable_exists(c->default_tableid, ))
> + fatalx("current routing table %u does not exist",
> + c->default_tableid);
>   if (rdomid != c->default_tableid)
>   fatalx("current routing table %u is not a routing domain",
>   c->default_tableid);
> 



httpd: add include_dir keyword

2022-06-02 Thread qorg11
This patch addes the "inlcude_dir" keyword for httpd.conf. Which works
just like "include" but it includes all the files in a directory, for
example: include "/etc/httpd.d"

The diff file is attatched.
Index: httpd.conf.5
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
retrieving revision 1.121
diff -u -p -u -p -r1.121 httpd.conf.5
--- httpd.conf.59 Mar 2022 13:50:41 -   1.121
+++ httpd.conf.52 Jun 2022 09:02:22 -
@@ -84,6 +84,12 @@ keyword, for example:
 .Bd -literal -offset indent
 include "/etc/httpd.conf.local"
 .Ed
+A directory with configuration files can be included with the
+.Ic include_dir
+keyword, for example:
+.Bd -literal -offset indent
+include_dir "/etc/httpd.conf.d"
+.Ed
 .Sh MACROS
 Macros can be defined that will later be expanded in context.
 Macro names must start with a letter, digit, or underscore,
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
retrieving revision 1.128
diff -u -p -u -p -r1.128 parse.y
--- parse.y 27 Feb 2022 20:30:30 -  1.128
+++ parse.y 2 Jun 2022 09:02:22 -
@@ -52,6 +52,7 @@
 #include 
 #include 
 #include 
+#include   
 
 #include "httpd.h"
 #include "http.h"
@@ -139,7 +140,7 @@ typedef struct {
 %token LISTEN LOCATION LOG LOGDIR MATCH MAXIMUM NO NODELAY OCSP ON PORT PREFORK
 %token PROTOCOLS REQUESTS ROOT SACK SERVER SOCKET STRIP STYLE SYSLOG TCP TICKET
 %token TIMEOUT TLS TYPE TYPES HSTS MAXAGE SUBDOMAINS DEFAULT PRELOAD REQUEST
-%token ERROR INCLUDE AUTHENTICATE WITH BLOCK DROP RETURN PASS REWRITE
+%token ERROR INCLUDE INCLUDE_DIR AUTHENTICATE WITH BLOCK DROP RETURN PASS 
REWRITE
 %token CA CLIENT CRL OPTIONAL PARAM FORWARDED FOUND NOT
 %token ERRDOCS GZIPSTATIC
 %token   STRING
@@ -155,6 +156,7 @@ typedef struct {
 
 grammar: /* empty */
| grammar include '\n'
+   | grammar include_dir '\n'
| grammar '\n'
| grammar varset '\n'
| grammar main '\n'
@@ -165,7 +167,6 @@ grammar : /* empty */
 
 include: INCLUDE STRING{
struct file *nfile;
-
if ((nfile = pushfile($2, 0)) == NULL) {
yyerror("failed to include file %s", $2);
free($2);
@@ -178,6 +179,46 @@ include: INCLUDE STRING{
}
;
 
+include_dir : INCLUDE_DIR STRING {
+   char absolute_path[PATH_MAX];
+   char dir[PATH_MAX];
+   struct file *nfile;
+   DIR *opened_dir = opendir($2);
+   struct dirent *entry;
+
+   if(opened_dir == NULL) {
+   free($2);
+   yyerror("Failed to open directory %s", $2);
+   YYERROR;
+   }
+
+   size_t len = strlcpy(dir, $2, PATH_MAX);
+
+   if(len >= sizeof(dir)) {
+   free($2);
+   yyerror("too long");
+   YYERROR;
+   }
+
+   while((entry = readdir(opened_dir))) {
+   if(entry->d_name[0] == '.')
+   continue;
+   len = 
snprintf(absolute_path,PATH_MAX,"%s%s",dir, entry->d_name);
+   if(len < 0|| len >= sizeof(absolute_path)) {
+   yyerror("too long");
+   YYERROR;
+   }
+   if((nfile = pushfile(absolute_path, 0)) == 
NULL) {
+   yyerror("failed to include file %s", 
$2);
+   YYERROR;
+   }
+   }
+
+   file = nfile;
+   lungetc('\n');
+}
+;
+
 varset : STRING '=' STRING {
char *s = $1;
while (*s++) {
@@ -1453,6 +1494,7 @@ lookup(char *s)
{ "gzip-static",GZIPSTATIC },
{ "hsts",   HSTS },
{ "include",INCLUDE },
+   { "include_dir",INCLUDE_DIR},
{ "index",  INDEX },
{ "ip", IP },
{ "key",KEY },


bgpd, check ktable_exists return value

2022-06-02 Thread Claudio Jeker
When setting the default routing table for bgpd make sure that
ktable_exists() does not fail.
Also improve the warning message in ktable_exists() a bit.

-- 
:wq Claudio

Index: kroute.c
===
RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
retrieving revision 1.246
diff -u -p -r1.246 kroute.c
--- kroute.c23 May 2022 13:40:12 -  1.246
+++ kroute.c2 Jun 2022 08:59:10 -
@@ -440,7 +440,7 @@ ktable_exists(u_int rtableid, u_int *rdo
if (errno == ENOENT)
/* table nonexistent */
return (0);
-   log_warn("%s: sysctl", __func__);
+   log_warn("sysctl net.route.rtableid");
/* must return 0 so that the table is considered non-existent */
return (0);
}
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.425
diff -u -p -r1.425 parse.y
--- parse.y 31 May 2022 09:45:33 -  1.425
+++ parse.y 2 Jun 2022 08:53:43 -
@@ -3496,7 +3496,9 @@ init_config(struct bgpd_config *c)
c->bgpid = get_bgpid();
c->fib_priority = RTP_BGP;
c->default_tableid = getrtable();
-   ktable_exists(c->default_tableid, );
+   if (!ktable_exists(c->default_tableid, ))
+   fatalx("current routing table %u does not exist",
+   c->default_tableid);
if (rdomid != c->default_tableid)
fatalx("current routing table %u is not a routing domain",
c->default_tableid);