Re: ftp: allow specifying supported protocols
On Sun, Sep 06, 2020 at 12:55:17AM +0200, Jeremie Courreges-Anglas wrote:
> On Sat, Sep 05 2020, Theo Buehler wrote:
> > I found this small diff useful more than once (admittedly for debugging).
> > It allows specifying the protocols that may be used by ftp the same way
> > as nc -Tprotocols works. For example:
> >
> > $ ftp -Sprotocols=tlsv1.2:tlsv1.1 https://example.com/file
> >
> > Perhaps someone else has use for it, too?
>
> I think it's useful indeed. ok jca@
>
> One nit below,
>
> > Index: ftp.1
> > ===
> > RCS file: /var/cvs/src/usr.bin/ftp/ftp.1,v
> > retrieving revision 1.119
> > diff -u -p -r1.119 ftp.1
> > --- ftp.1 11 Feb 2020 18:41:39 - 1.119
> > +++ ftp.1 5 Sep 2020 18:11:24 -
> > @@ -261,6 +261,12 @@ Don't perform server certificate validat
> > Require the server to present a valid OCSP stapling in the TLS handshake.
> > .It Cm noverifytime
> > Disable validation of certificate times and OCSP validation.
> > +.It Cm protocols Ns = Ns Ar protocol_list
> > +Specify the supported TLS protocols that will be used by
> > +.Nm
> > +(see
> > +.Xr tls_config_parse_protocols 3
> > +for details).
> > .It Cm session Ns = Ns Ar /path/to/session
> > Specify a file to use for TLS session data.
> > If this file has a non-zero length, the session data will be read from
> > this file
> > Index: main.c
> > ===
> > RCS file: /var/cvs/src/usr.bin/ftp/main.c,v
> > retrieving revision 1.132
> > diff -u -p -r1.132 main.c
> > --- main.c 1 Sep 2020 12:33:48 - 1.132
> > +++ main.c 1 Sep 2020 18:13:09 -
> > @@ -209,6 +209,8 @@ char * const ssl_verify_opts[] = {
> > "noverifytime",
> > #define SSL_SESSION8
> > "session",
> > +#define SSL_PROTOCOLS 9
> > + "protocols",
> > NULL
> > };
> >
> > @@ -220,6 +222,7 @@ process_ssl_options(char *cp)
> > {
> > const char *errstr;
> > long long depth;
>
> (Should probably be an int.)
>
> > + uint32_t protocols;
>
> Could be moved below str (smaller size on lp64, see style(9)).
>
> > char *str;
If we care about this, shouldn't I rather move *str up below *errstr?
> >
> > while (*cp) {
> > @@ -278,6 +281,14 @@ process_ssl_options(char *cp)
> > tls_session_fd) == -1)
> > errx(1, "failed to set session: %s",
> > tls_config_error(tls_config));
> > + break;
> > + case SSL_PROTOCOLS:
> > + if (str == NULL)
> > + errx(1, "missing protocol name");
> > + if (tls_config_parse_protocols(&protocols, str) != 0)
> > + errx(1, "failed to parse TLS protocols");
> > + if (tls_config_set_protocols(tls_config, protocols) !=
> > 0)
> > + errx(1, "failed to set TLS protocols");
> > break;
> > default:
> > errx(1, "unknown -S suboption `%s'",
> >
>
>
> --
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: ldapd: adding bsd.schema
Sorry for the late reply. On 8/12/20 8:19 AM, Robert Klein wrote: > Hi, > > On Wed, 12 Aug 2020 09:00:18 +0200 > Theo Buehler wrote: > >> On Tue, Aug 11, 2020 at 10:22:51PM -0400, Aisha Tammy wrote: >>> Another bump. >> >> I think this is useful and am ok with this. >> >> Are there any concerns? If not, I'm going to commit it tomorrow. > > for an sshPublicKey attribute, there's a “openssh-lpk” schema which > seems to be in common use. It's defined as > > # octetString SYNTAX > attributetype ( 1.3.6.1.4.1.24552.500.1.1.1.13 NAME 'sshPublicKey' > DESC 'OpenSSH Public key' > EQUALITY octetStringMatch > SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 ) > I prefer the non-octet version mostly because of inconsistent spacing when copy pasting. > # printableString SYNTAX yes|no > objectclass ( 1.3.6.1.4.1.24552.500.1.1.2.0 NAME 'ldapPublicKey' SUP > top AUXILIARY DESC 'OpenSSH LPK objectclass' > MUST uid > MAY sshPublicKey > ) > > though there are versions of the “ldapPublicKey” definitions with both > uid and sshPublicKye in the MUST and both in the MAY clause. The > “both MAY” version is imho more flexible. > > > The original mail proposing bsd.schema seems to have added both > “shadowPassword” and “bsdaccount” more as an afterthought, it seems. > The bsd account is a bit more flexible than the ldapPublicKey and can be substituted for this. I am fine with moving the `uid` to MAY as well, that would be very nice for virtual user setups, where uid is unimportant and not used. I've attached the updated patch which moves uid to MAY. I would really like this to be in 6.8. OK? Thanks, Aisha > > Best regards > Robert > > >> >> Index: etc/examples/ldapd.conf >> === >> RCS file: /cvs/src/etc/examples/ldapd.conf,v >> retrieving revision 1.1 >> diff -u -p -u -p -r1.1 ldapd.conf >> --- etc/examples/ldapd.conf 11 Jul 2014 21:20:10 - >> 1.1 +++ etc/examples/ldapd.conf 18 May 2018 10:09:45 - >> @@ -3,6 +3,7 @@ >> schema "/etc/ldap/core.schema" >> schema "/etc/ldap/inetorgperson.schema" >> schema "/etc/ldap/nis.schema" >> +schema "/etc/ldap/bsd.schema" >> >> listen on lo0 >> listen on "/var/run/ldapi" >> Index: usr.sbin/ldapd/Makefile >> === >> RCS file: /cvs/src/usr.sbin/ldapd/Makefile,v >> retrieving revision 1.15 >> diff -u -p -u -p -r1.15 Makefile >> --- usr.sbin/ldapd/Makefile 20 Jan 2017 11:55:08 - >> 1.15 +++ usr.sbin/ldapd/Makefile 18 May 2018 10:09:45 - >> @@ -17,7 +17,8 @@ CFLAGS+= -Wshadow -Wpointer-arith -Wcast >> CFLAGS+=-Wsign-compare >> CLEANFILES+=y.tab.h parse.c >> >> -SCHEMA_FILES= core.schema \ >> +SCHEMA_FILES= bsd.schema \ >> +core.schema \ >> inetorgperson.schema \ >> nis.schema >> >> Index: usr.sbin/ldapd/schema/bsd.schema >> === >> RCS file: usr.sbin/ldapd/schema/bsd.schema >> diff -N usr.sbin/ldapd/schema/bsd.schema >> --- /dev/null1 Jan 1970 00:00:00 - >> +++ usr.sbin/ldapd/schema/bsd.schema 18 May 2018 10:09:45 - >> @@ -0,0 +1,17 @@ >> +attributetype ( 1.3.6.1.4.1.30155.115.2 NAME 'shadowPassword' >> +DESC 'POSIX hashed password' >> +EQUALITY caseExactIA5Match >> +SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 ) >> + >> +attributetype ( 1.3.6.1.4.1.30155.115.3 NAME 'sshPublicKey' >> +DESC 'SSH public key' >> +EQUALITY caseExactIA5Match >> +SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 ) >> + >> +objectclass ( 1.3.6.1.4.1.30155.115.1 NAME 'bsdAccount' >> +SUP top >> +AUXILIARY >> +DESC 'Abstraction of an account with OpenBSD attributes' >> +MUST ( uid ) >> +MAY ( shadowPassword $ shadowExpire $ modifyTimestamp $ >> userClass $ >> +sshPublicKey )) >> > diff --git a/etc/examples/ldapd.conf b/etc/examples/ldapd.conf index 1bc6aa462c1..183563d6f9a 100644 --- a/etc/examples/ldapd.conf +++ b/etc/examples/ldapd.conf @@ -3,6 +3,7 @@ schema "/etc/ldap/core.schema" schema "/etc/ldap/inetorgperson.schema" schema "/etc/ldap/nis.schema" +schema "/etc/ldap/bsd.schema" listen on lo0 listen on "/var/run/ldapi" diff --git a/usr.sbin/ldapd/Makefile b/usr.sbin/ldapd/Makefile index bf445832576..5af25895787 100644 --- a/usr.sbin/ldapd/Makefile +++ b/usr.sbin/ldapd/Makefile @@ -17,7 +17,8 @@ CFLAGS+= -Wshadow -Wpointer-arith -Wcast-qual CFLAGS+= -Wsign-compare CLEANFILES+= y.tab.h parse.c -SCHEMA_FILES= core.schema \ +SCHEMA_FILES= bsd.schema \ + core.schema \ inetorgperson.schema \ nis.schema diff --git a/usr.sbin/ldapd/schema/bsd.schema b/usr.sbin/ldapd/schema/bsd.schema new file mode 100644 index 000..d14fcfe7456 --- /dev/null +++ b/usr.sbin/ldapd/schema/bsd.schema @@ -0,0 +1,16 @@ +attributetype ( 1.3.6.1.4.1.30155.115.2 NAME 'shadowPassword' + DESC 'POSIX hashed password' + EQUALITY caseExact
Re: ftp: allow specifying supported protocols
On Sat, Sep 05 2020, Theo Buehler wrote:
> I found this small diff useful more than once (admittedly for debugging).
> It allows specifying the protocols that may be used by ftp the same way
> as nc -Tprotocols works. For example:
>
> $ ftp -Sprotocols=tlsv1.2:tlsv1.1 https://example.com/file
>
> Perhaps someone else has use for it, too?
I think it's useful indeed. ok jca@
One nit below,
> Index: ftp.1
> ===
> RCS file: /var/cvs/src/usr.bin/ftp/ftp.1,v
> retrieving revision 1.119
> diff -u -p -r1.119 ftp.1
> --- ftp.1 11 Feb 2020 18:41:39 - 1.119
> +++ ftp.1 5 Sep 2020 18:11:24 -
> @@ -261,6 +261,12 @@ Don't perform server certificate validat
> Require the server to present a valid OCSP stapling in the TLS handshake.
> .It Cm noverifytime
> Disable validation of certificate times and OCSP validation.
> +.It Cm protocols Ns = Ns Ar protocol_list
> +Specify the supported TLS protocols that will be used by
> +.Nm
> +(see
> +.Xr tls_config_parse_protocols 3
> +for details).
> .It Cm session Ns = Ns Ar /path/to/session
> Specify a file to use for TLS session data.
> If this file has a non-zero length, the session data will be read from this
> file
> Index: main.c
> ===
> RCS file: /var/cvs/src/usr.bin/ftp/main.c,v
> retrieving revision 1.132
> diff -u -p -r1.132 main.c
> --- main.c1 Sep 2020 12:33:48 - 1.132
> +++ main.c1 Sep 2020 18:13:09 -
> @@ -209,6 +209,8 @@ char * const ssl_verify_opts[] = {
> "noverifytime",
> #define SSL_SESSION 8
> "session",
> +#define SSL_PROTOCOLS9
> + "protocols",
> NULL
> };
>
> @@ -220,6 +222,7 @@ process_ssl_options(char *cp)
> {
> const char *errstr;
> long long depth;
(Should probably be an int.)
> + uint32_t protocols;
Could be moved below str (smaller size on lp64, see style(9)).
> char *str;
>
> while (*cp) {
> @@ -278,6 +281,14 @@ process_ssl_options(char *cp)
> tls_session_fd) == -1)
> errx(1, "failed to set session: %s",
> tls_config_error(tls_config));
> + break;
> + case SSL_PROTOCOLS:
> + if (str == NULL)
> + errx(1, "missing protocol name");
> + if (tls_config_parse_protocols(&protocols, str) != 0)
> + errx(1, "failed to parse TLS protocols");
> + if (tls_config_set_protocols(tls_config, protocols) !=
> 0)
> + errx(1, "failed to set TLS protocols");
> break;
> default:
> errx(1, "unknown -S suboption `%s'",
>
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
ftp: allow specifying supported protocols
I found this small diff useful more than once (admittedly for debugging).
It allows specifying the protocols that may be used by ftp the same way
as nc -Tprotocols works. For example:
$ ftp -Sprotocols=tlsv1.2:tlsv1.1 https://example.com/file
Perhaps someone else has use for it, too?
Index: ftp.1
===
RCS file: /var/cvs/src/usr.bin/ftp/ftp.1,v
retrieving revision 1.119
diff -u -p -r1.119 ftp.1
--- ftp.1 11 Feb 2020 18:41:39 - 1.119
+++ ftp.1 5 Sep 2020 18:11:24 -
@@ -261,6 +261,12 @@ Don't perform server certificate validat
Require the server to present a valid OCSP stapling in the TLS handshake.
.It Cm noverifytime
Disable validation of certificate times and OCSP validation.
+.It Cm protocols Ns = Ns Ar protocol_list
+Specify the supported TLS protocols that will be used by
+.Nm
+(see
+.Xr tls_config_parse_protocols 3
+for details).
.It Cm session Ns = Ns Ar /path/to/session
Specify a file to use for TLS session data.
If this file has a non-zero length, the session data will be read from this
file
Index: main.c
===
RCS file: /var/cvs/src/usr.bin/ftp/main.c,v
retrieving revision 1.132
diff -u -p -r1.132 main.c
--- main.c 1 Sep 2020 12:33:48 - 1.132
+++ main.c 1 Sep 2020 18:13:09 -
@@ -209,6 +209,8 @@ char * const ssl_verify_opts[] = {
"noverifytime",
#define SSL_SESSION8
"session",
+#define SSL_PROTOCOLS 9
+ "protocols",
NULL
};
@@ -220,6 +222,7 @@ process_ssl_options(char *cp)
{
const char *errstr;
long long depth;
+ uint32_t protocols;
char *str;
while (*cp) {
@@ -278,6 +281,14 @@ process_ssl_options(char *cp)
tls_session_fd) == -1)
errx(1, "failed to set session: %s",
tls_config_error(tls_config));
+ break;
+ case SSL_PROTOCOLS:
+ if (str == NULL)
+ errx(1, "missing protocol name");
+ if (tls_config_parse_protocols(&protocols, str) != 0)
+ errx(1, "failed to parse TLS protocols");
+ if (tls_config_set_protocols(tls_config, protocols) !=
0)
+ errx(1, "failed to set TLS protocols");
break;
default:
errx(1, "unknown -S suboption `%s'",
Re: amd64: add tsc_delay(), a TSC-based delay(9) implementation
> Date: Sat, 5 Sep 2020 09:49:25 -0500 > From: Scott Cheloha > Cc: Mark Kettenis , Mike Larkin > Content-Type: text/plain; charset=us-ascii > Content-Disposition: inline > > On Tue, Aug 25, 2020 at 12:22:19PM -0700, Mike Larkin wrote: > > On Tue, Aug 25, 2020 at 12:12:36PM -0700, Mike Larkin wrote: > > > On Mon, Aug 24, 2020 at 01:55:45AM +0200, Mark Kettenis wrote: > > > > > Date: Sun, 23 Aug 2020 18:11:12 -0500 > > > > > From: Scott Cheloha > > > > > > > > > > Hi, > > > > > > > > > > Other BSDs use the TSC to implement delay(9) if the TSC is constant > > > > > and invariant. Here's a patch to add something similar to our kernel. > > > > > > > > If the TSC is fine as a timecounter it should be absolutely fine for > > > > use as delay(). And we could even use if the TSC isn't synchronized > > > > between CPUs. > > > > > > > > > This patch (or something equivalent) is a prerequisite to running the > > > > > lapic timer in oneshot or TSC deadline mode. Using the lapic timer to > > > > > implement delay(9) when it isn't running in periodic mode is too > > > > > complicated. However, using the i8254 for delay(9) is too slow. We > > > > > need an alternative. > > > > > > > > Hmm, but what are we going to use on machines where the TSC isn't > > > > constant/invariant? > > > > > > > > In what respect is the i8254 too slow? Does it take more than a > > > > microsecond to read it? > > > > > > It's 3 outb/inb pairs to ensure you get the reading correct. So that could > > > be quite a long time (as cheloha@ points out). Also, that's 6 VM exits if > > > running virtually (I realize that's not the main use case here but just > > > saying...) > > > > > > IIRC the 3 in/out pairs are the latch command followed by reading the > > > LSB/MSB > > > of the counter. It's not MMIO like the HPET or ACPI timer. > > > > > > And as cheloha@ also points out, it is highly likely that none of us have > > > a > > > real i8254 anymore, much of this is probably implemented in some EC > > > somewhere > > > and it's unlikely the developer of said EC put a lot of effort into > > > optimizing > > > the implementation of a legacy device like this. > > > > > > On the topic of virtualization: > > > > > > while (rdtsc() - start < want) > > > rdtsc(); > > > > I just realized the original diff didn't do two rdtscs. It did a pause > > inside the > > loop. So the effect is not *as* bad as I described but it's still > > *somewhat* bad. > > > > PS - pause loop exiting can be enabled to improve performance in this > > situation. > > What I'm getting from Mike's and kettenis@'s replies is that this is a > generally good idea. > > We should add an HPET fallback for the nasty cases where your TSC has > drift because the i8254 is slooow. But "hpet_delay()" can wait > for a later patch because we haven't switched the lapic into oneshot > mode yet, so lapic_delay() is still useable and fast. > > So, is this ok? Or do I need to tweak something? I think I'm setting > delay_func to tsc_delay under the right circumstances: we know the TSC > is invariant. > > kettenis@: as I mentioned, we need to do the delay_func pointer > comparison in lapic_calibrate_timer() to keep from clobbering > tsc_delay. We can't compare it with NULL because it is set to > i8254_delay() by default in amd64/machdep.c. Given that lapic_delay() will disappear in the future, maybe just adding a local proptotype for tsc_delay() in lapic.c would make sense instead of adding the header file? Either way, ok kettenis@ > Index: amd64/lapic.c > === > RCS file: /cvs/src/sys/arch/amd64/amd64/lapic.c,v > retrieving revision 1.56 > diff -u -p -r1.56 lapic.c > --- amd64/lapic.c 3 Sep 2020 21:38:46 - 1.56 > +++ amd64/lapic.c 5 Sep 2020 14:44:08 - > @@ -41,6 +41,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -584,7 +585,8 @@ skip_calibration: >* Now that the timer's calibrated, use the apic timer routines >* for all our timing needs.. >*/ > - delay_func = lapic_delay; > + if (delay_func != tsc_delay) > + delay_func = lapic_delay; > initclock_func = lapic_initclocks; > } > } > Index: amd64/tsc.c > === > RCS file: /cvs/src/sys/arch/amd64/amd64/tsc.c,v > retrieving revision 1.20 > diff -u -p -r1.20 tsc.c > --- amd64/tsc.c 23 Aug 2020 21:38:47 - 1.20 > +++ amd64/tsc.c 5 Sep 2020 14:44:08 - > @@ -26,6 +26,7 @@ > > #include > #include > +#include > > #define RECALIBRATE_MAX_RETRIES 5 > #define RECALIBRATE_SMI_THRESHOLD5 > @@ -252,7 +253,8 @@ tsc_timecounter_init(struct cpu_info *ci > tsc_timecounter.tc_quality = -1000; > tsc_timecounter.tc_user = 0; > tsc_is_invariant
Re: amd64: add tsc_delay(), a TSC-based delay(9) implementation
On Tue, Aug 25, 2020 at 12:22:19PM -0700, Mike Larkin wrote:
> On Tue, Aug 25, 2020 at 12:12:36PM -0700, Mike Larkin wrote:
> > On Mon, Aug 24, 2020 at 01:55:45AM +0200, Mark Kettenis wrote:
> > > > Date: Sun, 23 Aug 2020 18:11:12 -0500
> > > > From: Scott Cheloha
> > > >
> > > > Hi,
> > > >
> > > > Other BSDs use the TSC to implement delay(9) if the TSC is constant
> > > > and invariant. Here's a patch to add something similar to our kernel.
> > >
> > > If the TSC is fine as a timecounter it should be absolutely fine for
> > > use as delay(). And we could even use if the TSC isn't synchronized
> > > between CPUs.
> > >
> > > > This patch (or something equivalent) is a prerequisite to running the
> > > > lapic timer in oneshot or TSC deadline mode. Using the lapic timer to
> > > > implement delay(9) when it isn't running in periodic mode is too
> > > > complicated. However, using the i8254 for delay(9) is too slow. We
> > > > need an alternative.
> > >
> > > Hmm, but what are we going to use on machines where the TSC isn't
> > > constant/invariant?
> > >
> > > In what respect is the i8254 too slow? Does it take more than a
> > > microsecond to read it?
> >
> > It's 3 outb/inb pairs to ensure you get the reading correct. So that could
> > be quite a long time (as cheloha@ points out). Also, that's 6 VM exits if
> > running virtually (I realize that's not the main use case here but just
> > saying...)
> >
> > IIRC the 3 in/out pairs are the latch command followed by reading the
> > LSB/MSB
> > of the counter. It's not MMIO like the HPET or ACPI timer.
> >
> > And as cheloha@ also points out, it is highly likely that none of us have a
> > real i8254 anymore, much of this is probably implemented in some EC
> > somewhere
> > and it's unlikely the developer of said EC put a lot of effort into
> > optimizing
> > the implementation of a legacy device like this.
> >
> > On the topic of virtualization:
> >
> > while (rdtsc() - start < want)
> > rdtsc();
>
> I just realized the original diff didn't do two rdtscs. It did a pause inside
> the
> loop. So the effect is not *as* bad as I described but it's still *somewhat*
> bad.
>
> PS - pause loop exiting can be enabled to improve performance in this
> situation.
What I'm getting from Mike's and kettenis@'s replies is that this is a
generally good idea.
We should add an HPET fallback for the nasty cases where your TSC has
drift because the i8254 is slooow. But "hpet_delay()" can wait
for a later patch because we haven't switched the lapic into oneshot
mode yet, so lapic_delay() is still useable and fast.
So, is this ok? Or do I need to tweak something? I think I'm setting
delay_func to tsc_delay under the right circumstances: we know the TSC
is invariant.
kettenis@: as I mentioned, we need to do the delay_func pointer
comparison in lapic_calibrate_timer() to keep from clobbering
tsc_delay. We can't compare it with NULL because it is set to
i8254_delay() by default in amd64/machdep.c.
Index: amd64/lapic.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/lapic.c,v
retrieving revision 1.56
diff -u -p -r1.56 lapic.c
--- amd64/lapic.c 3 Sep 2020 21:38:46 - 1.56
+++ amd64/lapic.c 5 Sep 2020 14:44:08 -
@@ -41,6 +41,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -584,7 +585,8 @@ skip_calibration:
* Now that the timer's calibrated, use the apic timer routines
* for all our timing needs..
*/
- delay_func = lapic_delay;
+ if (delay_func != tsc_delay)
+ delay_func = lapic_delay;
initclock_func = lapic_initclocks;
}
}
Index: amd64/tsc.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/tsc.c,v
retrieving revision 1.20
diff -u -p -r1.20 tsc.c
--- amd64/tsc.c 23 Aug 2020 21:38:47 - 1.20
+++ amd64/tsc.c 5 Sep 2020 14:44:08 -
@@ -26,6 +26,7 @@
#include
#include
+#include
#define RECALIBRATE_MAX_RETRIES5
#define RECALIBRATE_SMI_THRESHOLD 5
@@ -252,7 +253,8 @@ tsc_timecounter_init(struct cpu_info *ci
tsc_timecounter.tc_quality = -1000;
tsc_timecounter.tc_user = 0;
tsc_is_invariant = 0;
- }
+ } else
+ delay_func = tsc_delay;
tc_init(&tsc_timecounter);
}
@@ -342,4 +344,15 @@ tsc_sync_ap(struct cpu_info *ci)
{
tsc_post_ap(ci);
tsc_post_ap(ci);
+}
+
+void
+tsc_delay(int usecs)
+{
+ uint64_t interval, start;
+
+ interval = (uint64_t)usecs * tsc_frequency / 100;
+ start = rdtsc_lfence();
+ while (rdtsc_lfence() - start < interval)
+ CPU_BUSY_CYCLE();
}
Index: include/cpuvar.h
===
RCS file: /cvs/src/sys/arch/amd6
Re: incorrect result from getppid for ptraced processes
> On 5 Sep 2020, at 03:22, Philip Guenther wrote:
>
> On Fri, Sep 4, 2020 at 2:59 PM Mateusz Guzik wrote:
>
>> On 9/5/20, Philip Guenther wrote:
>>> On Fri, Sep 4, 2020 at 1:06 PM Mateusz Guzik wrote:
>>>
On 9/4/20, Vitaliy Makkoveev wrote:
> On Fri, Sep 04, 2020 at 05:24:42PM +0200, Mateusz Guzik wrote:
>> getppid blindly follows the parent pointer and reads the pid.
>>
>> The problem is that ptrace reparents the traced process, so in
>> particular if you gdb -p $something, the target proc will start
>> seeing
>> gdb instead of its actual parent.
>>
>> There is a lot to say about the entire reparenting business or
>> storing
>> the original pid in ps_oppid (instead of some form of a reference to
>> the process).
>>
>> However, I think the most feasible fix for now is the same thing
>> FreeBSD did: *always* store the actual parent pid in ps_oppid. This
>> means all repareting will keep updating it (most notably when
>> abandoning children on exit), while ptrace will skip that part.
>>
>> Side effect of such a change be that getppid will stop requiring the
>> kernel lock.
>>
>
> Thanks for report. But we are in beta stage now so such modification
>> is
> impossible until next iteration.
>
> Since original parent identifier is stored as `ps_oppid' while process
> is traced we just return it to userland for this case. This is the way
> I
> propose to fix this bug for now.
>
> Comments? OKs?
>
> Index: sys/kern/kern_prot.c
> ===
> RCS file: /cvs/src/sys/kern/kern_prot.c,v
> retrieving revision 1.76
> diff -u -p -r1.76 kern_prot.c
> --- sys/kern/kern_prot.c 9 Jul 2019 12:23:25 - 1.76
> +++ sys/kern/kern_prot.c 4 Sep 2020 21:12:15 -
> @@ -84,7 +84,11 @@ int
> sys_getppid(struct proc *p, void *v, register_t *retval)
> {
>
> - *retval = p->p_p->ps_pptr->ps_pid;
> + if (p->p_p->ps_flags & PS_TRACED)
> + *retval = p->p_p->ps_oppid;
> + else
> + *retval = p->p_p->ps_pptr->ps_pid;
> +
> return (0);
> }
This is definitely a bare minimum fix, but it does the job.
>>>
>>> ptrace() has behaved like this for the life of OpenBSD and an indefinite
>>> number of years previous in the BSD releases. What has happened that a
>>> definitely incomplete fix is needed Right Now?
>>
>> I don't see how this reads as a demand this is fixed Right Now.
>>
>
> I didn't call it a demand, but the point stands: what has changed?
>
>
> I don't see how the fix is incomplete either. It can be done better
>> with more effort, but AFAICS the above results in correct behavior.
>>
>
> There are at least 2 other uses of ps_pptr->ps_pid that should also change,
> unless you like coredumps and ps disagreeing with getppid(), and someone
> needs to think how it affects doas.
>
Thanks for pointing. I missed these two places. However, doas(1) was not
affected because this diff doesn’t modify tty(4) behaviour:
TIOC{GET,SET}VERAUTH still use ps_pptr->ps_pid.
I checked other BSD’s. NetBSD, DragonflyBSD and OSX have the same
behaviour of getppid(2). And this behaviour don’t contradict POSIX.1 [1]. So
I guess Philip is right, there is no reason to follow this way.
1. https://pubs.opengroup.org/onlinepubs/9699919799/functions/getppid.html
Re: httpd.conf.5 TLS defaults
> On Sep 4, 2020, at 9:07 PM, Theo Buehler wrote: > > On Fri, Sep 04, 2020 at 08:48:48PM -0700, na...@airpost.net wrote: >> This is TLS v1.2 & 1.3 now. Delete it here, since the referenced man page is >> updated. > > Thanks, I'm ok with this diff. I had the diff below in my tree for a > long time (I think it was prompted by a question of tj). I did mention > the defaults since the other tls options (except client ca) do: > > Index: httpd.conf.5 > === > RCS file: /var/cvs/src/usr.sbin/httpd/httpd.conf.5,v > retrieving revision 1.112 > diff -u -p -r1.112 httpd.conf.5 > --- httpd.conf.524 Aug 2020 15:49:10 -1.112 > +++ httpd.conf.526 Aug 2020 06:41:31 - > @@ -649,12 +649,10 @@ is empty, OCSP stapling will not be used > The default is to not use OCSP stapling. > .It Ic protocols Ar string > Specify the TLS protocols to enable for this server. > -If not specified, the value > -.Qq default > -will be used (secure protocols; TLSv1.2-only). > Refer to the > .Xr tls_config_parse_protocols 3 > -function for other valid protocol string values. > +function for valid protocol string values. > +By default, TLSv1.3 and TLSv1.2 will be used. > .It Ic ticket lifetime Ar seconds > Enable TLS session tickets with a > .Ar seconds You’re right about the other options listing their defaults. Good to keep it consistent, I’d go with your diff.
Re: timeout(9): add clock-based timeouts (attempt 2)
> Date: Fri, 4 Sep 2020 17:55:39 -0500 > From: Scott Cheloha > > On Sat, Jul 25, 2020 at 08:46:08PM -0500, Scott Cheloha wrote: > > > > [...] > > > > I want to add clock-based timeouts to the kernel because tick-based > > timeouts suffer from a few problems: > > > > [...] > > > > Basically, ticks are a poor approximation for the system clock. We > > should use the real thing where possible. > > > > [...] > > > > Thoughts on this approach? Thoughts on the proposed API? > > 6 week bump. > > Attached is an rebased and streamlined diff. > > Let's try again: > > This patch adds support for timeouts scheduled against the hardware > timecounter. I call these "kclock timeouts". They are distinct from > the current tick-based timeouts because ticks are "software time", not > "real time". So what's the end game here? Are these kclock-based timeouts going to replace the tick-based timeouts at some point in the future? I can see why you want to have both in parallel for a while, but long-term I don't think we want to keep both. We don't really want to do a wholesale conversion of APIs again I'd say. So at some point the existing timeout_add_xxx() calls should be implemented in terms of "kclock timeouts". This implementation is still tick driven, so it doesn't really provide sub-tick resolution. What does that mean for testing this? I mean if we spend a lot of time now to verify that subsystems can tolerate the more fine-grained timeouts, we need to that again when you switch from having a period interrupt driving the wheel to having a scheduled interrupt isn't it? > For now we have one kclock, KCLOCK_UPTIME, which corresponds to > nanouptime(9). In the future I intend to add support for runtime and > UTC kclocks. Do we really need that? I suppose it helps implementing something like clock_nanosleep() with the TIMER_ABSTIME flag for various clock_id values? > Why do we want kclock timeouts at all? > > 1. Kclock timeouts expire at an actual time, not a tick. They >have nanosecond resolution and are NTP-sensitive. Thus, they >will *never* fire early. Is there a lot of overhead in these being NTP-sensitive? I'm asking because for short timeouts you don't really care about NTP corrections. > 2. One upshot of nanosecond resolution is that we don't need to >"round up" to the next tick when scheduling a timeout to prevent >early execution. The extra resolution allows us to reduce >latency in some contexts. > > 3. Kclock timeouts cover the entire range of the kernel timeline. >We can remove the "tick loops" like the one sys_nanosleep(). > > 4. Kclock timeouts are scheduled as absolute deadlines. This makes >supporting absolute timeouts trivial, which means we can add support >for clock_nanosleep(2) and the absolute pthread timeouts to the >kernel. > > Kclock timeouts aren't actually used anywhere yet, so merging this > patch will not break anything like last time (CC bluhm@). > > In a subsequent diff I will put them to use in tsleep_nsec(9) etc. > This will enable those interfaces to block for less than a tick, which > in turn will allow userspace to block for less than a tick in e.g. > futex(2), and poll(2). pd@ has verified that this fixes the "time > problem" in OpenBSD vmm(4) VMs (CC pd@). Like I said above, running the timeout is still tick-driven isn't it? This avoids having to wait at least a tick for timeouts that are shorter than a tick, but it means the timeout can still be extended up to a full tick. > You initialize kclock timeouts with timeout_set_kclock(). You > schedule them with timeout_in_nsec(), a relative timeout interface > that accepts a count of nanoseconds. If your timeout is in some > other unit (seconds, milliseconds, whatever) you must convert it > to nanoseconds before scheduling. Something like this will work: > > timeout_in_nsec(&my_timeout, SEC_TO_NSEC(1)); > > There won't be a flavored API supporting every conceivable time unit. So this is where I get worried. What is the game plan? Slowly convert everything from timeout_add_xxx() to timeout_in_nsec()? Or offer this as a temporary interface for people to test some critical subsystems after which we dump it and simply re-implement timeout_add_xxx() as kclock-based timeouts? > In the future I will expose an absolute timeout interface and a > periodic timeout rescheduling interface. We don't need either of > these interfaces to start, though. Not sure how useful a periodic timeout rescheduling interface really is if you have an absolute timeout interface. And isn't timeout_at_ts() already implemented in the diff? > Tick-based timeouts and kclock-based timeouts are *not* compatible. > You cannot schedule a kclock timeout with timeout_add(9). You cannot > schedule a tick-based timeout with timeout_in_nsec(9). I have added > KASSERTs to prevent this. > > Scheduling a kclock timeout with timeout_in_nsec() is more costly than > scheduling a tick-based t
