Re: First step towards improved unlocking in the VFS layer.
> On Jun 13, 2023, at 7:59 AM, Theo de Raadt wrote: > > Thordur I. Bjornsson wrote: > >> On Mon, Jun 12, 2023 at 9:15 PM Bob Beck wrote: >>> >>> On Mon, Jun 12, 2023 at 11:01:18AM -0600, Theo de Raadt wrote: >>>> + KASSERTMSG(1, "Ich Habe eine Rotweinflarsche in meinem Arsche"); >>>> That part of the diff is not OK. If everyone did this, we would have a >>>> mess on our hands. >>> (or I could simply deleted it) >> +1 >> >> Is it on purpose that this is completely silent ? >> Perhaps the warning in ffs_vfsops should go "WARNING: soft updates are >> now ignored" and the option dropped from GENERIC ? > > The purpose of Bob's diff is to silently (quietly) simply disable softdep > mount requests. > > They will be downgraded to regular mounts. But then we want to make > sure that the back-end code isn't accidentaly called, because of some > bug, that's where his whiny assert comes into play. > > The reason to disable softdep, is that softdep has crazy callback schemes and > context issues that are making it hard to reason about vfs locking. > > If we disable softdep, we may be able to unlock nami / vfs / etc better. > > When / if such locking/scheduling changes are finished, softdep can be > repaired. Softdep is also a significant hindrance to progress by mere mortals in the areas interfacing between the filesystem and the caching mid layer. Don’t get me wrong, it’s is brilliantly engineered from the point of view of it does absolutely everything it can within the realm of what’s permissible in current VFS layer pushing all the boundaries of what you can do in order to do what it does. The problem with that is it is one of the things that adds a lot of complexity, and makes it very hard to change things an evolve for something like, oh, having a log based filesystem or things like that. My desire to get rid to it is hopefully to make it where working in this area of the kernel is something more people could slowly learn how to do and make progress on things, without any change being an insurmountable obstacle with a learning curve so high people will give up in frustration because everything they try has subtle interactions and ends up getting backed out.
Re: First step towards improved unlocking in the VFS layer.
On Mon, Jun 12, 2023 at 11:01:18AM -0600, Theo de Raadt wrote: > + KASSERTMSG(1, "Ich Habe eine Rotweinflarsche in meinem Arsche"); > > That part of the diff is not OK. If everyone did this, we would have a > mess on our hands. Yeah, thats me nodding to my own past stupidity ;) changed to "softdep_mount should not have been called" (or I could simply deleted it)
First step towards improved unlocking in the VFS layer.
Minimal diff, further cleanup and dead code removal to follow.
---
sys/kern/vfs_syscalls.c | 7 +++
sys/sys/mount.h | 2 +-
sys/ufs/ffs/ffs_softdep.c | 2 ++
sys/ufs/ffs/ffs_vfsops.c | 16 +++-
4 files changed, 9 insertions(+), 18 deletions(-)
diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c
index f47f9dfcb69..51fc7b005c0 100644
--- a/sys/kern/vfs_syscalls.c
+++ b/sys/kern/vfs_syscalls.c
@@ -239,11 +239,10 @@ update:
else if (mp->mnt_flag & MNT_RDONLY)
mp->mnt_flag |= MNT_WANTRDWR;
mp->mnt_flag &=~ (MNT_NOSUID | MNT_NOEXEC | MNT_WXALLOWED | MNT_NODEV |
- MNT_SYNCHRONOUS | MNT_ASYNC | MNT_SOFTDEP | MNT_NOATIME |
- MNT_NOPERM | MNT_FORCE);
+ MNT_SYNCHRONOUS | MNT_ASYNC | MNT_NOATIME | MNT_NOPERM | MNT_FORCE);
mp->mnt_flag |= flags & (MNT_NOSUID | MNT_NOEXEC | MNT_WXALLOWED |
- MNT_NODEV | MNT_SYNCHRONOUS | MNT_ASYNC | MNT_SOFTDEP |
- MNT_NOATIME | MNT_NOPERM | MNT_FORCE);
+ MNT_NODEV | MNT_SYNCHRONOUS | MNT_ASYNC | MNT_NOATIME | MNT_NOPERM |
+ MNT_FORCE);
/*
* Mount the filesystem.
*/
diff --git a/sys/sys/mount.h b/sys/sys/mount.h
index 40c12e97602..8f57b18dd02 100644
--- a/sys/sys/mount.h
+++ b/sys/sys/mount.h
@@ -401,7 +401,7 @@ struct mount {
#defineMNT_STALLED 0x0010 /* filesystem stalled */
#defineMNT_SWAPPABLE 0x0020 /* filesystem can be used for
swap */
#define MNT_WANTRDWR 0x0200 /* want upgrade to read/write */
-#define MNT_SOFTDEP 0x0400 /* soft dependencies being done */
+#define MNT_SOFTDEP 0x0400 /* soft dependencies being done - now
ignored */
#define MNT_DOOMED 0x0800 /* device behind filesystem is gone */
#ifdef _KERNEL
diff --git a/sys/ufs/ffs/ffs_softdep.c b/sys/ufs/ffs/ffs_softdep.c
index 15017537b41..a263a1533ac 100644
--- a/sys/ufs/ffs/ffs_softdep.c
+++ b/sys/ufs/ffs/ffs_softdep.c
@@ -1224,6 +1224,8 @@ softdep_mount(struct vnode *devvp, struct mount *mp,
struct fs *fs,
struct buf *bp;
int error, cyl;
+ KASSERTMSG(1, "Ich Habe eine Rotweinflarsche in meinem Arsche");
+
/*
* When doing soft updates, the counters in the
* superblock may have gotten out of sync, so we have
diff --git a/sys/ufs/ffs/ffs_vfsops.c b/sys/ufs/ffs/ffs_vfsops.c
index ffe78ef140f..8350d56ba05 100644
--- a/sys/ufs/ffs/ffs_vfsops.c
+++ b/sys/ufs/ffs/ffs_vfsops.c
@@ -213,12 +213,10 @@ ffs_mount(struct mount *mp, const char *path, void *data,
int error = 0, flags;
int ronly;
-#ifndef FFS_SOFTUPDATES
+ /* Ask not for whom the bell tolls */
if (mp->mnt_flag & MNT_SOFTDEP) {
- printf("WARNING: soft updates isn't compiled in\n");
mp->mnt_flag &= ~MNT_SOFTDEP;
}
-#endif
/*
* Soft updates is incompatible with "async",
@@ -284,8 +282,6 @@ ffs_mount(struct mount *mp, const char *path, void *data,
if (mp->mnt_flag & MNT_FORCE)
flags |= FORCECLOSE;
error = softdep_flushfiles(mp, flags, p);
-#elif FFS_SOFTUPDATES
- mp->mnt_flag |= MNT_SOFTDEP;
#endif
}
/*
@@ -459,10 +455,7 @@ success:
free(fs->fs_contigdirs, M_UFSMNT, fs->fs_ncg);
}
if (!ronly) {
- if (mp->mnt_flag & MNT_SOFTDEP)
- fs->fs_flags |= FS_DOSOFTDEP;
- else
- fs->fs_flags &= ~FS_DOSOFTDEP;
+ fs->fs_flags &= ~FS_DOSOFTDEP;
}
ffs_sbupdate(ump, MNT_WAIT);
#if 0
@@ -923,10 +916,7 @@ ffs_mountfs(struct vnode *devvp, struct mount *mp, struct
proc *p)
}
fs->fs_fmod = 1;
fs->fs_clean = 0;
- if (mp->mnt_flag & MNT_SOFTDEP)
- fs->fs_flags |= FS_DOSOFTDEP;
- else
- fs->fs_flags &= ~FS_DOSOFTDEP;
+ fs->fs_flags &= ~FS_DOSOFTDEP;
error = ffs_sbupdate(ump, MNT_WAIT);
if (error == EROFS)
goto out;
--
2.40.0
Re: Add Miller-Rabin with random bases to BPSW primality check
On Fri, Apr 28, 2023 at 10:23:15AM +0200, Theo Buehler wrote:
> The behavior of BPSW for numbers > 2^64 is not very well understood.
> While there is no known composite that passes the test, there are
> heuristics that indicate that there are likely many. Therefore it seems
> appropriate to harden the test. Having a settable number of MR rounds
> before doing a version of BPSW is also the approach taken by Go's
> primality check in math/big.
>
> This is a first step towards incorporating some of the considerations in
> "A performant misuse-resistant API for Primality Testing" by Massimo and
> Paterson. It should be noted that it is based on this paper (and due to
> FIPS, I suppose) that OpenSSL 3 made their primality check so slow that
> the bn_prime.c regress test took over 10 minutes on an M3000 before the
> libcrypto bump (it took less than a minute with ours).
>
> This basically adds back an equivalent of the old inadequate prime number
> test before running the strong Lucas test, with slightly cleaner code.
> While I don't think performance matters a lot here, we're then effectively
> at about twice the cost of what we had a year ago. I also think that having
> some non-determinism in the algorithm is a plus.
>
> The implementation is straightforward. It could easily be tweaked to use
> the additional gcds in the "enhanced" MR test of FIPS 186-5, but as long
> as we are only going to throw away the additional info, this doesn't add
> all that much.
>
> I'd like to land this and then do further work in tree. We probably want
> to crank the number of MR rounds done by default considerably. I will
> also need to undo some of the clean-up done in BN_generate_prime_ex()
> after BPSW landed.
>
> Index: bn/bn_bpsw.c
> ===
> RCS file: /cvs/src/lib/libcrypto/bn/bn_bpsw.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 bn_bpsw.c
> --- bn/bn_bpsw.c 26 Nov 2022 16:08:51 - 1.8
> +++ bn/bn_bpsw.c 28 Apr 2023 07:56:01 -
> @@ -301,23 +301,94 @@ bn_strong_lucas_selfridge(int *is_prime,
> }
>
> /*
> - * Miller-Rabin primality test for base 2.
> + * Fermat criterion in Miller-Rabin test.
> + *
> + * Check whether 1 < base < n - 1 witnesses that n is composite. For prime n:
> + *
> + * * Fermat's little theorem: base^(n-1) = 1 (mod n).
> + * * The only square roots of 1 (mod n) are 1 and -1.
> + *
> + * Calculate base^((n-1)/2) by writing n - 1 = k * 2^s with odd k:
> iteratively
> + * compute (base^k)^(2^(s-1)) by successive squaring of base^k.
> + *
> + * If -1 is ever reached, base^(n-1) works out to 1 and n is a pseudoprime
> + * for base. If 1 is reached before -1, we have an unexpected square root of
> + * unity and n is composite. Otherwise base^(n-1) != 1, so n is composite.
> */
>
> static int
> -bn_miller_rabin_base_2(int *is_prime, const BIGNUM *n, BN_CTX *ctx)
> +bn_fermat(int *is_prime, const BIGNUM *n, const BIGNUM *n_minus_one,
> +const BIGNUM *k, int s, BIGNUM *base, BN_CTX *ctx, BN_MONT_CTX *mctx)
> {
> - BIGNUM *n_minus_one, *k, *x;
> - int i, s;
> + int ret = 0;
> + int i;
> +
> + /* Sanity check: ensure that 1 < base < n - 1. */
> + if (BN_cmp(base, BN_value_one()) <= 0 || BN_cmp(base, n_minus_one) >= 0)
> + goto err;
> +
> + if (!BN_mod_exp_mont_ct(base, base, k, n, ctx, mctx))
> + goto err;
> +
> + if (BN_is_one(base) || BN_cmp(base, n_minus_one) == 0) {
> + *is_prime = 1;
> + goto done;
> + }
> +
> + /* Loop invariant: base is neither 1 nor -1 (mod n). */
> + for (i = 1; i < s; i++) {
> + if (!BN_mod_sqr(base, base, n, ctx))
> + goto err;
> +
> + /* n is a pseudoprime for base. */
> + if (BN_cmp(base, n_minus_one) == 0) {
> + *is_prime = 1;
> + goto done;
> + }
> +
> + /* n is composite: there's a square root of unity != 1 or -1. */
> + if (BN_is_one(base)) {
> + *is_prime = 0;
> + goto done;
> + }
> + }
> +
> + /*
> + * If we get here, n is definitely composite: base^(n-1) != 1.
> + */
> +
> + *is_prime = 0;
> +
> + done:
> + ret = 1;
> +
> + err:
> + return ret;
> +}
> +
> +/*
> + * Miller-Rabin primality test for base 2 and for |rounds| of random bases.
> + * On success: is_prime == 0 implies n is composite - the converse is false.
> + */
> +
> +static int
> +bn_miller_rabin(int *is_prime, const BIGNUM *n, BN_CTX *ctx, size_t rounds)
> +{
> + BN_MONT_CTX *mctx = NULL;
> + BIGNUM *base, *k, *n_minus_one, *three;
> + size_t i;
> + int s;
> int ret = 0;
>
> BN_CTX_start(ctx);
>
> - if ((n_minus_one = BN_CTX_get(ctx)) == NULL)
> + if ((base = BN_CTX_get(ctx)) == NULL)
> goto err;
> if ((k = BN_CTX_get(ctx)) == NULL)
> go
Re: don't remove known vmd vm's on failure
Tried it out here with my gimpy little test setup and your suggested repro
case.
Seems to be more sane to me in this case, and looks like the right thing to do,
So ok beck@ for what that’s worth.
> On Jan 21, 2023, at 8:08 AM, Dave Voutila wrote:
>
>
> *bump*... Anyone able to test or review? Other than bikeshedding some
> function naming, this isn't a dramatic change.
>
> Dave Voutila writes:
>
>> Dave Voutila writes:
>>
>>> It turns out not only does vmd have numerous error paths for handling
>>> when something is amiss with a guest, most of the paths don't check if
>>> it's a known vm defined in vm.conf.
>>>
>>> As a result, vmd often removes the vm from the SLIST of vm's meaning
>>> one can't easily attempt to start it again or see it in vmctl's status
>>> output.
>>>
>>> A simple reproduction:
>>>
>>> 1. define a vm with memory > 4gb in vm.conf
>>> 2. run vmd in the foreground (doas vmd -d) so it's not started by rc.d
>>> 3. try to start with `vmctl start -c ${vm_name}`, you should trigger
>>> an ENOMEM and get the "Cannot allocate memory" message from vmctl.
>>> 4. try to start the same vm again...now you get EPERM!
>>> 5. the vm is no longer visible in the output from `vmctl status` :(
>>>
>>> The problem is most of the error paths call vm_remove, which not only
>>> tears down the vm via vm_stop, but also removes it from the vm list and
>>> frees it. Only clean stops or restarts seem to perform this check
>>> currently.
>>>
>>> Below diff refactors into checking if the vm is defined in the global
>>> config before deciding to call vm_stop or vm_remove.
>>
>> Slight tweak... __func__->caller to actually pass the correct name to
>> vm_{stop,remove}() from vm_terminate()
>>
>>
>> diff refs/heads/master refs/heads/vmd-accounting
>> commit - d4e23fe7544b01187ebf3ac8ae32e955445ee666
>> commit + 46503195403bfab50cd34bd8682f35a17d54d03d
>> blob - 6bffb2519a31464836aa573dbccb7aa14ea97722
>> blob + f30dc14de1ff9d5cf121cbc08b6db183a06d0c07
>> --- usr.sbin/vmd/vmd.c
>> +++ usr.sbin/vmd/vmd.c
>> @@ -67,6 +67,8 @@ struct vmd *env;
>> int vm_claimid(const char *, int, uint32_t *);
>> void start_vm_batch(int, short, void*);
>>
>> +static inline void vm_terminate(struct vmd_vm *, const char *);
>> +
>> struct vmd *env;
>>
>> static struct privsep_proc procs[] = {
>> @@ -395,14 +397,14 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struc
>> errno = vmr.vmr_result;
>> log_warn("%s: failed to forward vm result",
>>vcp->vcp_name);
>> - vm_remove(vm, __func__);
>> + vm_terminate(vm, __func__);
>> return (-1);
>> }
>> }
>>
>> if (vmr.vmr_result) {
>> log_warnx("%s: failed to start vm", vcp->vcp_name);
>> - vm_remove(vm, __func__);
>> + vm_terminate(vm, __func__);
>> errno = vmr.vmr_result;
>> break;
>> }
>> @@ -410,7 +412,7 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struc
>> /* Now configure all the interfaces */
>> if (vm_priv_ifconfig(ps, vm) == -1) {
>> log_warn("%s: failed to configure vm", vcp->vcp_name);
>> - vm_remove(vm, __func__);
>> + vm_terminate(vm, __func__);
>> break;
>> }
>>
>> @@ -441,10 +443,7 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struc
>> log_info("%s: sent vm %d successfully.",
>>vm->vm_params.vmc_params.vcp_name,
>>vm->vm_vmid);
>> - if (vm->vm_from_config)
>> - vm_stop(vm, 0, __func__);
>> - else
>> - vm_remove(vm, __func__);
>> + vm_terminate(vm, __func__);
>> }
>>
>> /* Send a response if a control client is waiting for it */
>> @@ -470,10 +469,7 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struc
>> }
>> if (vmr.vmr_result != EAGAIN ||
>>vm->vm_params.vmc_bootdevice) {
>> - if (vm->vm_from_config)
>> - vm_stop(vm, 0, __func__);
>> - else
>> - vm_remove(vm, __func__);
>> + vm_terminate(vm, __func__);
>> } else {
>> /* Stop VM instance but keep the tty open */
>> vm_stop(vm, 1, __func__);
>> @@ -509,7 +505,7 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struc
>>imsg->hdr.peerid, -1, &vir, sizeof(vir)) == -1) {
>> log_debug("%s: GET_INFO_VM failed for vm %d, removing",
>>__func__, vm->vm_vmid);
>> - vm_remove(vm, __func__);
>> + vm_terminate(vm, __func__);
>> return (-1);
>> }
>> break;
>> @@ -545,7 +541,7 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struc
>>sizeof(vir)) == -1) {
>> log_debug("%s: GET_INFO_VM_END failed",
>>__func__);
>> - vm_remove(vm, __func__);
>> + vm_terminate(vm, __func__);
>> return (-1);
>> }
>> }
>>
>> @@ -1943,3 +1943,14 @@ getmonotime(struct timeval *tv)
>>
>> TIMESPEC_TO_TIMEVAL(tv, &ts);
>> }
>> +
>> +static inline void
>> +vm_terminate(struct vmd_vm *vm, const char *caller)
>> +{
>> + if (vm->vm_from_config)
>> + vm_stop(vm, 0, caller);
>> + else {
>> + /* vm_remove calls vm_stop */
>> + vm_remove(vm, caller);
>> + }
>> +}
>
Inconsistent isdigit(3) man page
So isdigit(3) says in the first paragraph that 'The complete list of decimal digits is 0 and 1-9, in any locale.' Later on it says: 'On systems supporting non-ASCII single-byte character encodings, different c arguments may correspond to the digits, and the results of isdigit() may depend on the LC_CTYPE locale(1).' Argubly worring about non ASCII single byte character encodings doesn't make sense for an OpenBSD man page, but setting that aside for a minute it's the second part that is of concern.. "It may depend on the LC_CTYPE locale()" - Ok, well does it or doesn't it? Various spec docs seem all over the place on this, so I am also paging Dr. Posix in this email... Hi Philip! :) Is isdigit() safe from being screwed up by locale or not? Should it be as below? Thoughts? Index: isdigit.3 === RCS file: /cvs/src/lib/libc/gen/isdigit.3,v retrieving revision 1.13 diff -u -p -u -p -r1.13 isdigit.3 --- isdigit.3 11 Sep 2022 06:38:10 - 1.13 +++ isdigit.3 20 Jan 2023 16:23:08 - @@ -59,11 +59,7 @@ non-zero if the character tests true. On systems supporting non-ASCII single-byte character encodings, different .Fa c -arguments may correspond to the digits, and the results of -.Fn isdigit -may depend on the -.Ev LC_CTYPE -.Xr locale 1 . +arguments may correspond to the digits. .Sh SEE ALSO .Xr isalnum 3 , .Xr isalpha 3 ,
Re: Please test: unlock mprotect/mmap/munmap
I have now been running it for two days, I *thought * had one hang a day ago, with chrome and local building churning away with me mashing on the editor.. but I’ve now been doing the same thing with witness on for a day and had no issues. So I think whatever I might have seen is not reproducible or unrelated.. Ok beck@ > On Nov 8, 2022, at 10:30 AM, Martin Pieuchot wrote: > > On 08/11/22(Tue) 11:12, Mark Kettenis wrote: >>> Date: Tue, 8 Nov 2022 10:32:14 +0100 >>> From: Christian Weisgerber >>> >>> Martin Pieuchot: >>> These 3 syscalls should now be ready to run w/o KERNEL_LOCK(). This will reduce contention a lot. I'd be happy to hear from test reports on many architectures and possible workloads. >>> >>> This survived a full amd64 package build. >> >> \8/ >> >> I think that means it should be comitted. > > I agree. This has been tested on i386, riscv64, m88k, arm64, amd64 (of > course) and sparc64. I'm pretty confident. >
Re: more unused parts of dig
I keep reading these as "unused parts of dlg" and wondering
why he's not remoing them himself..
ok beck@
On Sat, Jun 25, 2022 at 08:48:48PM +1000, Jonathan Gray wrote:
> Index: lib/dns/gen.c
> ===
> RCS file: /cvs/src/usr.bin/dig/lib/dns/gen.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 gen.c
> --- lib/dns/gen.c 14 Sep 2020 08:40:43 - 1.15
> +++ lib/dns/gen.c 25 Jun 2022 10:43:28 -
> @@ -83,7 +83,6 @@ static const char copyright[] =
> #define TYPECLASSLEN 20 /* DNS mnemonic size. Must be less than
> 100. */
> #define TYPECLASSBUF (TYPECLASSLEN + 1)
> #define TYPECLASSFMT "%" STR(TYPECLASSLEN) "[-0-9a-z]_%d"
> -#define ATTRIBUTESIZE 256
> #define DIRNAMESIZE 256
>
> static struct cc {
> Index: lib/dns/rdata.c
> ===
> RCS file: /cvs/src/usr.bin/dig/lib/dns/rdata.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 rdata.c
> --- lib/dns/rdata.c 2 Apr 2021 06:37:40 - 1.33
> +++ lib/dns/rdata.c 25 Jun 2022 10:43:28 -
> @@ -20,7 +20,6 @@
>
> #include
>
> -#include
> #include
> #include
>
> Index: lib/dns/rdatalist.c
> ===
> RCS file: /cvs/src/usr.bin/dig/lib/dns/rdatalist.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 rdatalist.c
> --- lib/dns/rdatalist.c 25 Jun 2022 10:20:29 - 1.3
> +++ lib/dns/rdatalist.c 25 Jun 2022 10:43:28 -
> @@ -22,7 +22,6 @@
>
> #include
>
> -#include
> #include
> #include
> #include
> Index: lib/dns/rdataset.c
> ===
> RCS file: /cvs/src/usr.bin/dig/lib/dns/rdataset.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 rdataset.c
> --- lib/dns/rdataset.c25 Jun 2022 10:20:29 - 1.11
> +++ lib/dns/rdataset.c25 Jun 2022 10:43:28 -
> @@ -159,19 +159,6 @@ dns_rdataset_makequestion(dns_rdataset_t
> rdataset->attributes |= DNS_RDATASETATTR_QUESTION;
> }
>
> -void
> -dns_rdataset_clone(dns_rdataset_t *source, dns_rdataset_t *target) {
> -
> - /*
> - * Make 'target' refer to the same rdataset as 'source'.
> - */
> -
> - REQUIRE(source->methods != NULL);
> - REQUIRE(target->methods == NULL);
> -
> - (source->methods->clone)(source, target);
> -}
> -
> isc_result_t
> dns_rdataset_first(dns_rdataset_t *rdataset) {
>
> Index: lib/dns/include/dns/rdataset.h
> ===
> RCS file: /cvs/src/usr.bin/dig/lib/dns/include/dns/rdataset.h,v
> retrieving revision 1.12
> diff -u -p -r1.12 rdataset.h
> --- lib/dns/include/dns/rdataset.h25 Jun 2022 10:20:30 - 1.12
> +++ lib/dns/include/dns/rdataset.h25 Jun 2022 10:43:28 -
> @@ -181,20 +181,6 @@ dns_rdataset_makequestion(dns_rdataset_t
> *\li'rdataset' is a valid, associated, question rdataset.
> */
>
> -void
> -dns_rdataset_clone(dns_rdataset_t *source, dns_rdataset_t *target);
> -/*%<
> - * Make 'target' refer to the same rdataset as 'source'.
> - *
> - * Requires:
> - *\li'source' is a valid, associated rdataset.
> - *
> - *\li'target' is a valid, dissociated rdataset.
> - *
> - * Ensures:
> - *\li'target' references the same rdataset as 'source'.
> - */
> -
> isc_result_t
> dns_rdataset_first(dns_rdataset_t *rdataset);
> /*%<
> Index: lib/isc/lex.c
> ===
> RCS file: /cvs/src/usr.bin/dig/lib/isc/lex.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 lex.c
> --- lib/isc/lex.c 17 Jan 2022 18:19:51 - 1.14
> +++ lib/isc/lex.c 25 Jun 2022 10:43:28 -
> @@ -18,7 +18,6 @@
>
> /*! \file */
>
> -#include
> #include
>
> #include
> @@ -238,8 +237,6 @@ typedef enum {
> lexstate_eatline,
> lexstate_qstring
> } lexstate;
> -
> -#define IWSEOL (ISC_LEXOPT_INITIALWS | ISC_LEXOPT_EOL)
>
> static void
> pushback(inputsource *source, int c) {
> Index: lib/isccfg/parser.c
> ===
> RCS file: /cvs/src/usr.bin/dig/lib/isccfg/parser.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 parser.c
> --- lib/isccfg/parser.c 14 Sep 2020 08:40:44 - 1.8
> +++ lib/isccfg/parser.c 25 Jun 2022 10:43:28 -
> @@ -590,49 +590,6 @@ cfg_parse_listelt(cfg_parser_t *pctx, co
> return (result);
> }
>
> -int
> -cfg_obj_islist(const cfg_obj_t *obj) {
> - REQUIRE(obj != NULL);
> - return (obj->type->rep == &cfg_rep_list);
> -}
> -
> -const cfg_listelt_t *
> -cfg_list_first(const cfg_obj_t *obj) {
> - REQUIRE(obj == NULL || obj->type->rep == &cfg_rep_list);
> - if (obj == NULL)
> - return (NULL);
> - return (ISC_LIST_HEAD(obj->value.list));
> -}
> -
> -const cfg_listelt_t *
> -cfg_list_next(const cfg_listelt_t *elt) {
> -
Re: rpki-client: cache X509v3 extensions early
yes makes sense
ok beck@
> On May 11, 2022, at 07:53, Theo Buehler wrote:
>
> Some funky libcrypto business ahead.
>
> X509 API functions such as X509_check_ca() or X509_get_extension_flags()
> cache X509v3 extensions internally if they're not already cached. They
> make decisions based on (or report some of) the cached values. Although
> it's unlikely, this caching may fail halfway through. The result is
> fairly random in the case of X509_check_ca() (which can't report an
> error itself) - in LibreSSL it would actually return 1 due to a bug I
> fixed yesterday. Every use of X509_get_extension_flags() on a cert for
> which we don't know that the extensions are cached already should also
> check the EXFLAG_INVALID, which is annoying.
>
> An old workaround that used to be used in libssl is to call
> X509_check_purpose(x, -1, -1), which is effectively a wrapper around
> x509v3_cache_extensions() that allows error checking. This way, the
> reported values by the affected API functions are reliable. I suggest to
> do this once we get our hands on a cert, so this issue is out of the
> way.
>
> Caching of extensions will happen sooner or later anyway, at the latest
> within X509_verify_cert(). In LibreSSL this also ensures that the
> RFC 3779 extensions are in canonical form before we inspect them which
> I think is a good thing.
>
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.77
> diff -u -p -r1.77 cert.c
> --- cert.c11 May 2022 09:40:00 -1.77
> +++ cert.c11 May 2022 13:16:19 -
> @@ -597,6 +597,12 @@ cert_parse_pre(const char *fn, const uns
>goto out;
>}
>
> +/* Cache X509v3 extensions, see X509_check_ca(3). */
> +if (X509_check_purpose(x, -1, -1) <= 0) {
> +cryptowarnx("%s: could not cache X509v3 extensions", p.fn);
> +goto out;
> +}
> +
>/* Look for X509v3 extensions. */
>
>if ((extsz = X509_get_ext_count(x)) < 0)
> Index: cms.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cms.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 cms.c
> --- cms.c28 Mar 2022 13:04:01 -1.16
> +++ cms.c11 May 2022 13:19:14 -
> @@ -224,6 +224,12 @@ cms_parse_validate(X509 **xp, const char
>}
>*xp = X509_dup(sk_X509_value(certs, 0));
>
> +/* Cache X509v3 extensions, see X509_check_ca(3). */
> +if (X509_check_purpose(*xp, -1, -1) <= 0) {
> +cryptowarnx("%s: could not cache X509v3 extensions", fn);
> +goto out;
> +}
> +
>if (CMS_SignerInfo_get0_signer_id(si, &kid, NULL, NULL) != 1 ||
>kid == NULL) {
>warnx("%s: RFC 6488: could not extract SKI from SID", fn);
>
Re: uvm: Consider BUFPAGES_DEFICIT in swap_shortage
On Thu, May 05, 2022 at 10:16:23AM -0600, Bob Beck wrote:
> Ugh. You???re digging in the most perilous parts of the pile.
>
> I will go look with you??? sigh. (This is not yet an ok for that.)
>
> > On May 5, 2022, at 7:53 AM, Martin Pieuchot wrote:
> >
> > When considering the amount of free pages in the page daemon a small
> > amount is always kept for the buffer cache... except in one place.
> >
> > The diff below gets rid of this exception. This is important because
> > uvmpd_scan() is called conditionally using the following check:
> >
> > if (uvmexp.free - BUFPAGES_DEFICIT) < uvmexp.freetarg) {
> >...
> > }
> >
> > So in case of swap shortage we might end up freeing fewer pages than
> > wanted.
So a bit of backgroud.
I am pretty much of the belief that this "low water mark" for pages is
nonsense now. I was in the midst of trying to prove that
to myself and therefore rip down some of the crazy accounting and
very arbitrary limits in the buffer cache and got distracted.
Maybe something like this to start? (buf failing that I think
your current diff is probably ok).
Index: sys/sys/mount.h
===
RCS file: /cvs/src/sys/sys/mount.h,v
retrieving revision 1.148
diff -u -p -u -p -r1.148 mount.h
--- sys/sys/mount.h 6 Apr 2021 14:17:35 - 1.148
+++ sys/sys/mount.h 5 May 2022 16:50:50 -
@@ -488,10 +488,8 @@ struct bcachestats {
#ifdef _KERNEL
extern struct bcachestats bcstats;
extern long buflowpages, bufhighpages, bufbackpages;
-#define BUFPAGES_DEFICIT (((buflowpages - bcstats.numbufpages) < 0) ? 0 \
-: buflowpages - bcstats.numbufpages)
-#define BUFPAGES_INACT (((bcstats.numcleanpages - buflowpages) < 0) ? 0 \
-: bcstats.numcleanpages - buflowpages)
+#define BUFPAGES_DEFICIT 0
+#define BUFPAGES_INACT bcstats.numcleanpages
extern int bufcachepercent;
extern void bufadjust(int);
struct uvm_constraint_range;
> >
> > ok?
> >
> > Index: uvm/uvm_pdaemon.c
> > ===
> > RCS file: /cvs/src/sys/uvm/uvm_pdaemon.c,v
> > retrieving revision 1.98
> > diff -u -p -r1.98 uvm_pdaemon.c
> > --- uvm/uvm_pdaemon.c 4 May 2022 14:58:26 - 1.98
> > +++ uvm/uvm_pdaemon.c 5 May 2022 13:40:28 -
> > @@ -923,12 +923,13 @@ uvmpd_scan(void)
> > * detect if we're not going to be able to page anything out
> > * until we free some swap resources from active pages.
> > */
> > + free = uvmexp.free - BUFPAGES_DEFICIT;
> > swap_shortage = 0;
> > - if (uvmexp.free < uvmexp.freetarg &&
> > + if (free < uvmexp.freetarg &&
> > uvmexp.swpginuse == uvmexp.swpages &&
> > !uvm_swapisfull() &&
> > pages_freed == 0) {
> > - swap_shortage = uvmexp.freetarg - uvmexp.free;
> > + swap_shortage = uvmexp.freetarg - free;
> > }
> >
> > for (p = TAILQ_FIRST(&uvm.page_active);
> >
>
Re: acme-client: check token names
An ok beck@ from me with my usual curmudgeonly mutterings
about the people who made this necessary for isalnum(), walls,
and revolutions...
> On May 5, 2022, at 7:57 AM, Florian Obser wrote:
>
> On 2022-05-04 13:21 +0430, Ali Farzanrad wrote:
>> OK, I've tested following diff on my own domain and it works.
>> I did 2 modifications:
>>
>> 1. I explicitly call setlocate with "C" to ensure C locale,
>
> I came to the conclusion that it's best to call setlocale in first thing
> in main, that's what other code does, too and seems less surprising.
>
>>
>> 2. I did a string length check. According to RFC it must have 128 bit
>> of random entropy, so it should have at least 22 base64 characters,
>> but I was unsure. So I only check for empty strings.
>
> Checking for an empty string gives us a better error message, we would
> error out with EISDIR in open(2) later, so that's good I guess.
> Trying to enforce entropy seems a bit silly though, it's there to
> protect the CA, if they mess this up it's their problem.
>
> The following diff just moves setlocale to main and is OK florian
>
> Or I can commit it myself is someone else OKs it.
>
> diff --git chngproc.c chngproc.c
> index 0cbfaf27c31..f9cff65311d 100644
> --- chngproc.c
> +++ chngproc.c
> @@ -16,6 +16,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -77,6 +78,18 @@ chngproc(int netsock, const char *root)
> goto out;
> else if ((tok = readstr(netsock, COMM_TOK)) == NULL)
> goto out;
> + else if (strlen(tok) < 1) {
> + warnx("token is too short");
> + goto out;
> + }
> +
> + for (i = 0; tok[i]; ++i) {
> + int ch = (unsigned char)tok[i];
> + if (!isalnum(ch) && ch != '-' && ch != '_') {
> + warnx("token is not a valid base64url");
> + goto out;
> + }
> + }
>
> if (asprintf(&fmt, "%s.%s", tok, th) == -1) {
> warn("asprintf");
> diff --git main.c main.c
> index 65ea2cf3ac3..a3006ca1483 100644
> --- main.c
> +++ main.c
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -56,6 +57,9 @@ main(int argc, char *argv[])
> struct domain_c *domain = NULL;
> struct altname_c*ac;
>
> + if (setlocale(LC_CTYPE, "C") == NULL)
> + errx(1, "setlocale");
> +
> while ((c = getopt(argc, argv, "Fnrvf:")) != -1)
> switch (c) {
> case 'F':
>
>
> --
> I'm not entirely sure you are real.
Re: rpki-client: factor filename extension parsing into a function
I like that.. LGTM
ok beck@
On Fri, Jan 21, 2022 at 08:37:27PM +0100, Theo Buehler wrote:
> > Lets start with that and optimize this in tree. I think we can rename the
> > function to something like rtype_from_mftfile(). In that case I would move
> > the function as well...
>
> Like this?
>
> Index: extern.h
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.111
> diff -u -p -r1.111 extern.h
> --- extern.h 21 Jan 2022 18:49:44 - 1.111
> +++ extern.h 21 Jan 2022 19:36:09 -
> @@ -421,6 +421,8 @@ void mft_free(struct mft *);
> struct mft *mft_parse(X509 **, const char *, const unsigned char *,
> size_t);
> struct mft *mft_read(struct ibuf *);
> +enum rtypertype_from_file_extension(const char *);
> +enum rtypertype_from_mftfile(const char *);
>
> void roa_buffer(struct ibuf *, const struct roa *);
> void roa_free(struct roa *);
> @@ -447,12 +449,9 @@ int valid_ta(const char *, struct auth
> int valid_cert(const char *, struct auth_tree *,
> const struct cert *);
> int valid_roa(const char *, struct auth_tree *, struct roa *);
> -int valid_filename(const char *);
> int valid_filehash(int, const char *, size_t);
> int valid_uri(const char *, size_t, const char *);
> int valid_origin(const char *, const char *);
> -
> -enum rtypertype_from_file_extension(const char *);
>
> /* Working with CMS. */
> unsigned char*cms_parse_validate(X509 **, const char *,
> Index: mft.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 mft.c
> --- mft.c 21 Jan 2022 18:49:44 - 1.49
> +++ mft.c 21 Jan 2022 19:36:10 -
> @@ -16,6 +16,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -121,6 +122,66 @@ check_validity(const ASN1_GENERALIZEDTIM
> }
>
> /*
> + * Determine rtype corresponding to file extension. Returns RTYPE_INVALID
> + * on error or unkown extension.
> + */
> +enum rtype
> +rtype_from_file_extension(const char *fn)
> +{
> + size_t sz;
> +
> + sz = strlen(fn);
> + if (sz < 5)
> + return RTYPE_INVALID;
> +
> + if (strcasecmp(fn + sz - 4, ".tal") == 0)
> + return RTYPE_TAL;
> + if (strcasecmp(fn + sz - 4, ".cer") == 0)
> + return RTYPE_CER;
> + if (strcasecmp(fn + sz - 4, ".crl") == 0)
> + return RTYPE_CRL;
> + if (strcasecmp(fn + sz - 4, ".mft") == 0)
> + return RTYPE_MFT;
> + if (strcasecmp(fn + sz - 4, ".roa") == 0)
> + return RTYPE_ROA;
> + if (strcasecmp(fn + sz - 4, ".gbr") == 0)
> + return RTYPE_GBR;
> +
> + return RTYPE_INVALID;
> +}
> +
> +/*
> + * Validate that a filename listed on a Manifest only contains characters
> + * permitted in draft-ietf-sidrops-6486bis section 4.2.2 and check that
> + * it's a CER, CRL, GBR or a ROA.
> + * Returns corresponding rtype or RTYPE_INVALID on error.
> + */
> +enum rtype
> +rtype_from_mftfile(const char *fn)
> +{
> + const unsigned char *c;
> + enum rtype type;
> +
> + for (c = fn; *c != '\0'; ++c)
> + if (!isalnum(*c) && *c != '-' && *c != '_' && *c != '.')
> + return RTYPE_INVALID;
> +
> + if (strchr(fn, '.') != strrchr(fn, '.'))
> + return RTYPE_INVALID;
> +
> + type = rtype_from_file_extension(fn);
> + switch (type) {
> + case RTYPE_CER:
> + case RTYPE_CRL:
> + case RTYPE_GBR:
> + case RTYPE_ROA:
> + return type;
> + default:
> + return RTYPE_INVALID;
> + }
> +}
> +
> +/*
> * Parse an individual "FileAndHash", RFC 6486, sec. 4.2.
> * Return zero on failure, non-zero on success.
> */
> @@ -161,12 +222,10 @@ mft_parse_filehash(struct parse *p, cons
> if (fn == NULL)
> err(1, NULL);
>
> - if (!valid_filename(fn)) {
> + if ((type = rtype_from_mftfile(fn)) == RTYPE_INVALID) {
> warnx("%s: invalid filename: %s", p->fn, fn);
> goto out;
> }
> -
> - type = rtype_from_file_extension(fn);
>
> /* Now hash value. */
>
> Index: parser.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 parser.c
> --- parser.c 21 Jan 2022 18:49:44 - 1.49
> +++ parser.c 21 Jan 2022 19:36:10 -
> @@ -307,7 +307,7 @@ proc_parser_mft_check(const char *fn, st
>
> for (i = 0; i < p->filesz; i++) {
> const struct mftfile *m = &p->files[i];
> - if (!valid_filename(m->file)) {
> + if (rtype_from_mftfile(m->file) == RTYPE_INVALID) {
>
Re: Add CT methods to standard_exts, fix timestamp printing
ok beck@ > On Nov 23, 2021, at 21:14, Theo Buehler wrote: > > Two small diffs now that beck has linked the certificate transparency > code to the build. > > The diff for ext_dat.h links the CT methods to the standard extensions. > This replaces the gibberish from the CT extensions which are now present > in most certs with something readable. Try > > $ openssl s_client -connect libressl.org:443 | openssl x509 -noout -text > > The diff for ct_prn makes sure that the timestamp is actually printed. > Our ASN1_GENERALIZEDTIME_set_string() does not accept fractional > seconds, so don't feed them into it for printing. eopenssl11 doesn't > print the fractional sections either. > > Index: x509/ext_dat.h > === > RCS file: /cvs/src/lib/libcrypto/x509/ext_dat.h,v > retrieving revision 1.3 > diff -u -p -r1.3 ext_dat.h > --- x509/ext_dat.h2 Sep 2021 21:27:26 -1.3 > +++ x509/ext_dat.h16 Nov 2021 16:56:19 - > @@ -73,6 +73,7 @@ extern X509V3_EXT_METHOD v3_crl_hold, v3 > extern X509V3_EXT_METHOD v3_policy_mappings, v3_policy_constraints; > extern X509V3_EXT_METHOD v3_name_constraints, v3_inhibit_anyp, v3_idp; > extern const X509V3_EXT_METHOD v3_addr, v3_asid; > +extern const X509V3_EXT_METHOD v3_ct_scts[3]; > > /* This table will be searched using OBJ_bsearch so it *must* kept in > * order of the ext_nid values. > @@ -129,6 +130,11 @@ static const X509V3_EXT_METHOD *standard >&v3_idp, >&v3_alt[2], >&v3_freshest_crl, > +#ifndef OPENSSL_NO_CT > +&v3_ct_scts[0], > +&v3_ct_scts[1], > +&v3_ct_scts[2], > +#endif > }; > > /* Number of standard extensions */ > Index: ct/ct_prn.c > === > RCS file: /cvs/src/lib/libcrypto/ct/ct_prn.c,v > retrieving revision 1.3 > diff -u -p -r1.3 ct_prn.c > --- ct/ct_prn.c20 Nov 2021 01:10:49 -1.3 > +++ ct/ct_prn.c21 Nov 2021 15:32:56 - > @@ -71,8 +71,7 @@ timestamp_print(uint64_t timestamp, BIO > * Note GeneralizedTime from ASN1_GENERALIZETIME_adj is always 15 > * characters long with a final Z. Update it with fractional seconds. > */ > -snprintf(genstr, sizeof(genstr), "%.14s.%03dZ", > -ASN1_STRING_get0_data(gen), (unsigned int)(timestamp % 1000)); > +snprintf(genstr, sizeof(genstr), "%.14sZ", ASN1_STRING_get0_data(gen)); >if (ASN1_GENERALIZEDTIME_set_string(gen, genstr)) >ASN1_GENERALIZEDTIME_print(out, gen); >ASN1_GENERALIZEDTIME_free(gen); >
Re: cert.pem sync
ok > On Jun 10, 2021, at 05:05, Theo Buehler wrote: > > On Thu, Jun 10, 2021 at 11:39:46AM +0100, Stuart Henderson wrote: >> I was just reminded of the Apple cert problem with GeoTrust Global CA >> and checked and they're using better intermediates for api.push.apple.com >> now. OK to sync up with Mozilla's CA bundle again, including removal >> of GeoTrust Global CA? > > Thanks! > >> Changes in the list first; diff below: >> >> -AC Camerfirma S.A. > > Ah, good. > >> Staat der Nederlanden >> /C=NL/O=Staat der Nederlanden/CN=Staat der Nederlanden EV Root CA >> - /C=NL/O=Staat der Nederlanden/CN=Staat der Nederlanden Root CA - G3 > > I could not find any information on this removal and it's still in my > firefox. Why is that removed in your diff? >
Re: NiX Spam mirroring
Should be fixed. a bit of a pain because their new site has an expired tls cert. On Thu, Oct 28, 2021 at 07:30:56AM +0200, Jan Johansson wrote: > Hello! > > I write to you because I beleive that you are running the NiX Spam > mirroring script for OpenBSD. The feed has been broken for some > time and I hope that you can fix it. > > More information is available in this thread: > https://marc.info/?t=16328493413&r=1&w=2 > > Best regards, > Jan J >
Re: rpki-client add back keep-alive to http requests
ok beck@ On Thu, Sep 09, 2021 at 09:35:51AM +0200, Claudio Jeker wrote: > While Connection: keep-alive should be the default it seems that at least > some of the CA repositories fail to behave like that. Adding back the > Connection header seems to fix this and delta downloads go faster again. > > -- > :wq Claudio > > Index: http.c > === > RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v > retrieving revision 1.38 > diff -u -p -r1.38 http.c > --- http.c1 Sep 2021 09:39:14 - 1.38 > +++ http.c9 Sep 2021 07:26:35 - > @@ -1026,6 +1026,7 @@ http_request(struct http_connection *con > conn->bufpos = 0; > if ((r = asprintf(&conn->buf, > "GET /%s HTTP/1.1\r\n" > + "Connection: keep-alive\r\n" > "Host: %s\r\n" > "Accept-Encoding: identity\r\n" > "User-Agent: " HTTP_USER_AGENT "\r\n" >
Re: Turn SCHED_LOCK() into a mutex
> > This work has been started by art@ more than a decade ago and I'm > > willing to finish it This is possibly one of the scariest things you can say in OpenBSD. I am now calling my doctor to get a giant bag of flintstones chewable zoloft prescribed to me just so I can recover from seeing you say this. (I do support your efforts doing this however.. tread carefully :)
Re: Correctly set SSL error if x509_verify fails
On Sun, Oct 25, 2020 at 01:43:10PM -0600, Bob Beck wrote:
>
>
>
> On Fri, Oct 23, 2020 at 09:13:23AM +0200, Theo Buehler wrote:
> > On Thu, Oct 22, 2020 at 08:44:29PM -0700, Jeremy Evans wrote:
> > > I was trying to diagnose a certificate validation failure in Ruby's
> > > openssl extension tests with LibreSSL 3.2.2, and it was made more
> > > difficult because the verification error type was dropped, resulting
> > > in a SSL_R_CERTIFICATE_VERIFY_FAILED error where
> > > SSL_get_verify_result returned X509_V_OK.
> >
> > Could you perhaps isolate a test case or explain the reproducer in a bit
> > more detail? I tried running ruby 2.7's regress from ports but that
> > always resulted in a SIGABRT (usually while running test_ftp.rb with or
> > without your diff).
> >
> > With my diff below I once got past this abort and saw this:
> >
> > OpenSSL::TestSSL#test_verify_result
> > [/usr/ports/pobj/ruby-2.7.2/ruby-2.7.2/test/openssl/utils.rb:279]:
> > exceptions on 1 threads:
> > <19> expected but was
> > <20>.
> >
> > Did you see <0> here instead?
> >
> > > I think this patch will fix it. Compile tested only. OKs, or is there
> > > a better way to fix it?
> >
> > This will probably also address the issue with the haproxy test reported
> > on the libressl list:
> >
> > https://marc.info/?l=libressl&m=160339671313132&w=2
> > https://github.com/haproxy/haproxy/issues/916
> >
> > Regarding your diff, I think setting the error on the store context
> > should not be the responsibility of x509_verify()'s caller. After all,
> > x509_verify() will likely end up being public API.
> >
> > The below diff should have the same effect as yours.
>
> This looks ok to me tb@.. and appears to be the correct fix.
>
Actually I rescind the ok -> your diff introduces a new problem.
I added regress to catch this problem and it found an issue:
the call into the x509_vfy_check_id can set the xsc->error but
then we don't set the ctx->error and it's sitting still as V_OK
when your diff sets it. and that's bad.
So this correctly sets the ctx->error coming back from
x509_vfy_check_id so your diff doesn't introduce another regression
you have my ok on this modified version :) I'll commit
the change to the bettertls regress to catch it once you commit.
Index: x509/x509_verify.c
===
RCS file: /cvs/src/lib/libcrypto/x509/x509_verify.c,v
retrieving revision 1.13
diff -u -p -u -p -r1.13 x509_verify.c
--- x509/x509_verify.c 26 Sep 2020 15:44:06 - 1.13
+++ x509/x509_verify.c 25 Oct 2020 23:58:08 -
@@ -458,8 +458,12 @@ x509_verify_cert_hostname(struct x509_ve
size_t len;
if (name == NULL) {
- if (ctx->xsc != NULL)
- return x509_vfy_check_id(ctx->xsc);
+ if (ctx->xsc != NULL) {
+ int ret;
+ if ((ret = x509_vfy_check_id(ctx->xsc)) == 0)
+ ctx->error = ctx->xsc->error;
+ return ret;
+ }
return 1;
}
if ((candidate = strdup(name)) == NULL) {
@@ -853,13 +857,13 @@ x509_verify(struct x509_verify_ctx *ctx,
if (ctx->roots == NULL || ctx->max_depth == 0) {
ctx->error = X509_V_ERR_INVALID_CALL;
- return 0;
+ goto err;
}
if (ctx->xsc != NULL) {
if (leaf != NULL || name != NULL) {
ctx->error = X509_V_ERR_INVALID_CALL;
- return 0;
+ goto err;
}
leaf = ctx->xsc->cert;
@@ -872,34 +876,34 @@ x509_verify(struct x509_verify_ctx *ctx,
*/
if ((ctx->xsc->chain = sk_X509_new_null()) == NULL) {
ctx->error = X509_V_ERR_OUT_OF_MEM;
- return 0;
+ goto err;
}
if (!X509_up_ref(leaf)) {
ctx->error = X509_V_ERR_OUT_OF_MEM;
- return 0;
+ goto err;
}
if (!sk_X509_push(ctx->xsc->chain, leaf)) {
X509_free(leaf);
ctx->error = X509_V_ERR_OUT_OF_MEM;
- return 0;
+ goto err;
}
ctx->xsc->error_depth = 0;
ctx->xsc->current_cert = leaf;
}
if (!x509_verify_cert_valid(ctx, leaf, NULL))
-
Re: Correctly set SSL error if x509_verify fails
On Fri, Oct 23, 2020 at 09:13:23AM +0200, Theo Buehler wrote:
> On Thu, Oct 22, 2020 at 08:44:29PM -0700, Jeremy Evans wrote:
> > I was trying to diagnose a certificate validation failure in Ruby's
> > openssl extension tests with LibreSSL 3.2.2, and it was made more
> > difficult because the verification error type was dropped, resulting
> > in a SSL_R_CERTIFICATE_VERIFY_FAILED error where
> > SSL_get_verify_result returned X509_V_OK.
>
> Could you perhaps isolate a test case or explain the reproducer in a bit
> more detail? I tried running ruby 2.7's regress from ports but that
> always resulted in a SIGABRT (usually while running test_ftp.rb with or
> without your diff).
>
> With my diff below I once got past this abort and saw this:
>
> OpenSSL::TestSSL#test_verify_result
> [/usr/ports/pobj/ruby-2.7.2/ruby-2.7.2/test/openssl/utils.rb:279]:
> exceptions on 1 threads:
> <19> expected but was
> <20>.
>
> Did you see <0> here instead?
>
> > I think this patch will fix it. Compile tested only. OKs, or is there
> > a better way to fix it?
>
> This will probably also address the issue with the haproxy test reported
> on the libressl list:
>
> https://marc.info/?l=libressl&m=160339671313132&w=2
> https://github.com/haproxy/haproxy/issues/916
>
> Regarding your diff, I think setting the error on the store context
> should not be the responsibility of x509_verify()'s caller. After all,
> x509_verify() will likely end up being public API.
>
> The below diff should have the same effect as yours.
This looks ok to me tb@.. and appears to be the correct fix.
>
> > Thanks,
> > Jeremy
> >
> > Index: x509_vfy.c
> > ===
> > RCS file: /cvs/src/lib/libcrypto/x509/x509_vfy.c,v
> > retrieving revision 1.81
> > diff -u -p -r1.81 x509_vfy.c
> > --- x509_vfy.c 26 Sep 2020 02:06:28 - 1.81
> > +++ x509_vfy.c 23 Oct 2020 03:34:10 -
> > @@ -680,6 +680,9 @@ X509_verify_cert(X509_STORE_CTX *ctx)
> > if ((vctx = x509_verify_ctx_new_from_xsc(ctx, roots)) != NULL) {
> > ctx->error = X509_V_OK; /* Initialize to OK */
> > chain_count = x509_verify(vctx, NULL, NULL);
> > + if (vctx->error) {
> > + ctx->error = vctx->error;
> > + }
> > }
> >
> > sk_X509_pop_free(roots, X509_free);
> >
>
>
> Index: x509/x509_verify.c
> ===
> RCS file: /var/cvs/src/lib/libcrypto/x509/x509_verify.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 x509_verify.c
> --- x509/x509_verify.c26 Sep 2020 15:44:06 - 1.13
> +++ x509/x509_verify.c23 Oct 2020 05:04:05 -
> @@ -853,13 +853,13 @@ x509_verify(struct x509_verify_ctx *ctx,
>
> if (ctx->roots == NULL || ctx->max_depth == 0) {
> ctx->error = X509_V_ERR_INVALID_CALL;
> - return 0;
> + goto err;
> }
>
> if (ctx->xsc != NULL) {
> if (leaf != NULL || name != NULL) {
> ctx->error = X509_V_ERR_INVALID_CALL;
> - return 0;
> + goto err;
> }
> leaf = ctx->xsc->cert;
>
> @@ -872,34 +872,34 @@ x509_verify(struct x509_verify_ctx *ctx,
>*/
> if ((ctx->xsc->chain = sk_X509_new_null()) == NULL) {
> ctx->error = X509_V_ERR_OUT_OF_MEM;
> - return 0;
> + goto err;
> }
> if (!X509_up_ref(leaf)) {
> ctx->error = X509_V_ERR_OUT_OF_MEM;
> - return 0;
> + goto err;
> }
> if (!sk_X509_push(ctx->xsc->chain, leaf)) {
> X509_free(leaf);
> ctx->error = X509_V_ERR_OUT_OF_MEM;
> - return 0;
> + goto err;
> }
> ctx->xsc->error_depth = 0;
> ctx->xsc->current_cert = leaf;
> }
>
> if (!x509_verify_cert_valid(ctx, leaf, NULL))
> - return 0;
> + goto err;
>
> if (!x509_verify_cert_hostname(ctx, leaf, name))
> - return 0;
> + goto err;
>
> if ((current_chain = x509_verify_chain_new()) == NULL) {
> ctx->error = X509_V_ERR_OUT_OF_MEM;
> - return 0;
> + goto err;
> }
> if (!x509_verify_chain_append(current_chain, leaf, &ctx->error)) {
> x509_verify_chain_free(current_chain);
> - return 0;
> + goto err;
> }
> if (x509_verify_ctx_cert_is_root(ctx, leaf))
> x509_verify_ctx_add_chain(ctx, current_chain);
> @@ -925,4 +925,9 @@ x509_verify(struct x509_verify_ctx *ctx,
> return ctx->xsc->verify_cb(ctx->chains_count, ctx->xsc);
> }
> return (ctx->chains_count);
> +
> + err:
> +
Happy 25th Birthday OpenBSD!
Yeah, it's just a number. But it's been a pretty wild ride. Thanks everyone for 25 years. -Bob
Re: [PATCH netcat] Only force fd's to -1 once
On Sun, Sep 27, 2020 at 02:46:39PM +1000, Duncan Roe wrote:
> The motivation for this is to make debug logs less confusing.
What is this fixing and what behavior are you changing?
>
> All changed lines have previously demonstrated the problem.
>
> Signed-off-by: Duncan Roe
> ---
> usr.bin/nc/netcat.c | 27 ++-
> 1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/usr.bin/nc/netcat.c b/usr.bin/nc/netcat.c
> index 528dbeea678..b152cfaf635 100644
> --- a/usr.bin/nc/netcat.c
> +++ b/usr.bin/nc/netcat.c
> @@ -1196,7 +1196,7 @@ readwrite(int net_fd, struct tls *tls_ctx)
> !(pfd[POLL_NETIN].revents & POLLIN))
> pfd[POLL_NETIN].fd = -1;
>
> - if (pfd[POLL_NETOUT].revents & POLLHUP) {
> + if ((pfd[POLL_NETOUT].revents & POLLHUP) && pfd[POLL_NETOUT].fd
> != -1) {
> if (Nflag)
> shutdown(pfd[POLL_NETOUT].fd, SHUT_WR);
> pfd[POLL_NETOUT].fd = -1;
> @@ -1205,14 +1205,14 @@ readwrite(int net_fd, struct tls *tls_ctx)
> if (pfd[POLL_STDOUT].revents & POLLHUP)
> pfd[POLL_STDOUT].fd = -1;
> /* if no net out, stop watching stdin */
> - if (pfd[POLL_NETOUT].fd == -1)
> + if (pfd[POLL_NETOUT].fd == -1 && pfd[POLL_STDIN].fd != -1)
> pfd[POLL_STDIN].fd = -1;
> /* if no stdout, stop watching net in */
> - if (pfd[POLL_STDOUT].fd == -1) {
> - if (pfd[POLL_NETIN].fd != -1)
> - shutdown(pfd[POLL_NETIN].fd, SHUT_RD);
> - pfd[POLL_NETIN].fd = -1;
> - }
> + if (pfd[POLL_STDOUT].fd == -1 &&
> + pfd[POLL_NETIN].fd != -1) {
> + shutdown(pfd[POLL_NETIN].fd, SHUT_RD);
> + pfd[POLL_NETIN].fd = -1;
> + }
>
> /* try to read from stdin */
> if (pfd[POLL_STDIN].revents & POLLIN && stdinbufpos < BUFSIZE) {
> @@ -1299,15 +1299,16 @@ readwrite(int net_fd, struct tls *tls_ctx)
> }
>
> /* stdin gone and queue empty? */
> - if (pfd[POLL_STDIN].fd == -1 && stdinbufpos == 0) {
> - if (pfd[POLL_NETOUT].fd != -1 && Nflag)
> - shutdown(pfd[POLL_NETOUT].fd, SHUT_WR);
> + if (pfd[POLL_STDIN].fd == -1 && stdinbufpos == 0 &&
> + pfd[POLL_NETOUT].fd != -1) {
> + if (Nflag)
> + shutdown(pfd[POLL_NETOUT].fd, SHUT_WR);
> pfd[POLL_NETOUT].fd = -1;
> }
> /* net in gone and queue empty? */
> - if (pfd[POLL_NETIN].fd == -1 && netinbufpos == 0) {
> - pfd[POLL_STDOUT].fd = -1;
> - }
> + if (pfd[POLL_NETIN].fd == -1 && netinbufpos == 0 &&
> + pfd[POLL_STDOUT].fd != -1)
> + pfd[POLL_STDOUT].fd = -1;
> }
> }
>
> --
> 2.17.5
>
Re: agentx and clang static analyzer
On Tue, Sep 15, 2020 at 11:08:04AM +0200, Martijn van Duren wrote:
> There are 3 things that actually look like valid complaints when running
> clang's static analyzer.
>
> 1) A dead store in agentx_recv.
> 2) sizeof(ipaddress) intead of sizeof(*ipaddress). Since this is ipv4,
>this is only a problem if sizeof(pointer) is smaller then 4 bytes,
>which can't happen afaik.
> 3) dst does indeed need to be dereffed, but since dstlen and buflen are
>initialized to 0 and srclen is practically always larger then 0
>we're fine. I'm keeping the *dst here as an additional safeguard.
>
> The rest seem like false positives to me.
>
> OK?
>
> martijn@
>
> Index: agentx.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/agentx.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 agentx.c
> --- agentx.c 14 Sep 2020 11:30:25 - 1.16
> +++ agentx.c 15 Sep 2020 09:05:57 -
> @@ -135,7 +135,6 @@ agentx_recv(struct agentx *ax)
> header.aph_packetid = agentx_pdutoh32(&header, u8);
> u8 += 4;
> header.aph_plength = agentx_pdutoh32(&header, u8);
> - u8 += 4;
>
> if (header.aph_version != 1) {
> errno = EPROTO;
ok for this piece above
> Index: subagentx.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/subagentx.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 subagentx.c
> --- subagentx.c 14 Sep 2020 11:30:25 - 1.1
> +++ subagentx.c 15 Sep 2020 09:05:58 -
> @@ -2929,7 +2929,7 @@ getnext:
> index->sav_idatacomplete = 1;
> break;
> case AGENTX_DATA_TYPE_IPADDRESS:
> - ipaddress = calloc(1, sizeof(ipaddress));
> + ipaddress = calloc(1, sizeof(*ipaddress));
> if (ipaddress == NULL) {
> subagentx_log_sag_warn(sag,
> "Failed to bind ipaddress index");
Not ok for this, it's probably correct, but there are other instances of this
in this code and so you need to engage brain, not static analyzer, go fix or
don't fix them all in a separate commit.
> @@ -3833,7 +3833,7 @@ subagentx_strcat(char **dst, const char
> }
>
> srclen = strlen(src);
> - if (dst == NULL || dstlen + srclen > buflen) {
> + if (*dst == NULL || dstlen + srclen > buflen) {
> nbuflen = (((dstlen + srclen) / 512) + 1) * 512;
> tmp = recallocarray(*dst, buflen, nbuflen, sizeof(*tmp));
> if (tmp == NULL)
>
ok for this piece above
Re: acme-client: improve account creation error message
But what if I like json and I am already set up to be a hipster and
feed all the untrusted inputs through jq..
(ok beck@)
On Mon, Sep 14, 2020 at 03:37:25PM +0200, Florian Obser wrote:
> not helpful:
> $ doas acme-client $(hostname)
> acme-client: https://api.test4.buypass.no/acme-v02/new-acct: bad HTTP: 400
>
> vomitting unformated json is not better:
> $ doas acme-client -v $(hostname)
> acme-client: transfer buffer:
> [{"type":"urn:ietf:params:acme:error:malformed","detail":"Email is a required
> contact","code":400,"message":"MALFORMED_BAD_REQUEST","details":"HTTP 400 Bad
> Request"}] (164 bytes)
>
> let's do this:
> $ doas obj/acme-client -v $(hostname)
> acme-client: Email is a required contact
>
> OK?
>
> diff --git extern.h extern.h
> index 529d3350205..364425b0500 100644
> --- extern.h
> +++ extern.h
> @@ -259,6 +259,7 @@ intjson_parse_order(struct jsmnn *,
> struct order *);
> int json_parse_upd_order(struct jsmnn *, struct order *);
> void json_free_capaths(struct capaths *);
> int json_parse_capaths(struct jsmnn *, struct capaths *);
> +char *json_getstr(struct jsmnn *, const char *);
>
> char *json_fmt_newcert(const char *);
> char *json_fmt_chkacc(void);
> diff --git json.c json.c
> index 61d2631359f..a6762eeb258 100644
> --- json.c
> +++ json.c
> @@ -297,7 +297,7 @@ json_getobj(struct jsmnn *n, const char *name)
> * that it's the correct type.
> * Returns NULL on failure.
> */
> -static char *
> +char *
> json_getstr(struct jsmnn *n, const char *name)
> {
> size_t i;
> diff --git netproc.c netproc.c
> index 7b8152196d1..05e36897c38 100644
> --- netproc.c
> +++ netproc.c
> @@ -371,15 +371,27 @@ sreq(struct conn *c, const char *addr, int kid, const
> char *req, char **loc)
> static int
> donewacc(struct conn *c, const struct capaths *p)
> {
> + struct jsmnn*j = NULL;
> int rc = 0;
> - char*req;
> + char*req, *detail, *error = NULL;
> long lc;
>
> if ((req = json_fmt_newacc()) == NULL)
> warnx("json_fmt_newacc");
> else if ((lc = sreq(c, p->newaccount, 0, req, &c->kid)) < 0)
> warnx("%s: bad comm", p->newaccount);
> - else if (lc != 200 && lc != 201)
> + else if (lc == 400) {
> + if ((j = json_parse(c->buf.buf, c->buf.sz)) == NULL)
> + warnx("%s: bad JSON object", p->newaccount);
> + else {
> + detail = json_getstr(j, "detail");
> + if (detail != NULL && stravis(&error, detail, VIS_SAFE)
> + != -1) {
> + warnx("%s", error);
> + free(error);
> + }
> + }
> + } else if (lc != 200 && lc != 201)
> warnx("%s: bad HTTP: %ld", p->newaccount, lc);
> else if (c->buf.buf == NULL || c->buf.sz == 0)
> warnx("%s: empty response", p->newaccount);
>
>
> --
> I'm not entirely sure you are real.
>
Re: dt: add static vfs probes
ok beck@
On Mon, Sep 14, 2020 at 12:45:55PM +0200, Jasper Lievisse Adriaanse wrote:
> Hi,
>
> Whilst analyzing the cleaner I added tracepoints called 'cleaner' and
> 'bufcache_take' to
> track its behaviour.
>
> For the sake of symmetry I've added one in bufcache_release() too and moved
> the assignment
> of 'pages' until after the KASSERT(), following the flow of bufcache_take().
>
> Sample usage of these probes:
>
> tracepoint:vfs:bufcache_take {
> printf("bcache_take:%d(%s) flags: 0x%x cache: %d pages: %d\n",
> tid, comm, arg0 , arg1, arg2);
> }
>
> tracepoint:vfs:bufcache_rel{
> printf("bcache_rel:%d(%s) flags: 0x%x cache: %d pages: %d\n",
> tid, comm, arg0, arg1, arg2);
> }
>
> tracepoint:vfs:cleaner{
> printf("cleaner:%d(%s) flags: 0x%x pushed: %d lodirtypages: %d,
> hidirtypages: %d\n",
> tid, comm, arg0, arg1, arg2, arg3);
> }
>
> OK to commit this?
>
> Index: dev/dt/dt_prov_static.c
> ===
> RCS file: /cvs/src/sys/dev/dt/dt_prov_static.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 dt_prov_static.c
> --- dev/dt/dt_prov_static.c 13 Sep 2020 14:55:08 - 1.4
> +++ dev/dt/dt_prov_static.c 14 Sep 2020 10:43:43 -
> @@ -58,6 +58,13 @@ DT_STATIC_PROBE3(uvm, map_insert, "vaddr
> DT_STATIC_PROBE3(uvm, map_remove, "vaddr_t", "vaddr_t", "vm_prot_t");
>
> /*
> + * VFS
> + */
> +DT_STATIC_PROBE3(vfs, bufcache_rel, "long", "int", "int64_t");
> +DT_STATIC_PROBE3(vfs, bufcache_take, "long", "int", "int64_t");
> +DT_STATIC_PROBE4(vfs, cleaner, "long", "int", "long", "long");
> +
> +/*
> * List of all static probes
> */
> struct dt_probe *dtps_static[] = {
> @@ -76,6 +83,10 @@ struct dt_probe *dtps_static[] = {
> &_DT_STATIC_P(uvm, fault),
> &_DT_STATIC_P(uvm, map_insert),
> &_DT_STATIC_P(uvm, map_remove),
> + /* VFS */
> + &_DT_STATIC_P(vfs, bufcache_rel),
> + &_DT_STATIC_P(vfs, bufcache_take),
> + &_DT_STATIC_P(vfs, cleaner),
> };
>
> int
> Index: kern/vfs_bio.c
> ===
> RCS file: /cvs/src/sys/kern/vfs_bio.c,v
> retrieving revision 1.202
> diff -u -p -r1.202 vfs_bio.c
> --- kern/vfs_bio.c12 Sep 2020 11:57:24 - 1.202
> +++ kern/vfs_bio.c14 Sep 2020 10:43:43 -
> @@ -57,6 +57,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> /* XXX Should really be in buf.h, but for uvm_constraint_range.. */
> @@ -1209,6 +1210,9 @@ buf_daemon(void *arg)
> }
>
> while ((bp = bufcache_getdirtybuf())) {
> + TRACEPOINT(vfs, cleaner, bp->b_flags, pushed,
> + lodirtypages, hidirtypages);
> +
> if (UNCLEAN_PAGES < lodirtypages &&
> bcstats.kvaslots_avail > 2 * RESERVE_SLOTS &&
> pushed >= 16)
> @@ -1693,6 +1697,9 @@ bufcache_take(struct buf *bp)
> KASSERT((bp->cache < NUM_CACHES));
>
> pages = atop(bp->b_bufsize);
> +
> + TRACEPOINT(vfs, bufcache_take, bp->b_flags, bp->cache, pages);
> +
> struct bufcache *cache = &cleancache[bp->cache];
> if (!ISSET(bp->b_flags, B_DELWRI)) {
> if (ISSET(bp->b_flags, B_COLD)) {
> @@ -1756,8 +1763,11 @@ bufcache_release(struct buf *bp)
> int64_t pages;
> struct bufcache *cache = &cleancache[bp->cache];
>
> - pages = atop(bp->b_bufsize);
> KASSERT(ISSET(bp->b_flags, B_BC));
> + pages = atop(bp->b_bufsize);
> +
> + TRACEPOINT(vfs, bufcache_rel, bp->b_flags, bp->cache, pages);
> +
> if (fliphigh) {
> if (ISSET(bp->b_flags, B_DMA) && bp->cache > 0)
> panic("B_DMA buffer release from cache %d",
> --
> jasper
>
Re: rpki-client cleanup includes
ok beck@
On Sat, Sep 12, 2020 at 05:42:39PM +0200, Claudio Jeker wrote:
> extern.h uses stuff from openssl/x509.h so put that include in there
> and remove all the various other openssl includes in other files that
> actually don't need x509 functions.
>
> --
> :wq Claudio
>
> Index: as.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/as.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 as.c
> --- as.c 27 Nov 2019 17:18:24 - 1.5
> +++ as.c 12 Sep 2020 15:02:20 -
> @@ -25,8 +25,6 @@
> #include
> #include
>
> -#include
> -
> #include "extern.h"
>
> /*
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 cert.c
> --- cert.c28 Jul 2020 07:35:04 - 1.17
> +++ cert.c12 Sep 2020 15:02:20 -
> @@ -26,7 +26,6 @@
> #include
> #include
>
> -#include
> #include /* DIST_POINT */
>
> #include "extern.h"
> Index: crl.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/crl.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 crl.c
> --- crl.c 2 Apr 2020 09:16:43 - 1.8
> +++ crl.c 12 Sep 2020 15:02:20 -
> @@ -26,8 +26,6 @@
> #include
> #include
>
> -#include
> -
> #include "extern.h"
>
> X509_CRL *
> Index: extern.h
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.33
> diff -u -p -r1.33 extern.h
> --- extern.h 12 Sep 2020 10:02:01 - 1.33
> +++ extern.h 12 Sep 2020 15:02:20 -
> @@ -20,6 +20,8 @@
> #include
> #include
>
> +#include
> +
> enum cert_as_type {
> CERT_AS_ID, /* single identifier */
> CERT_AS_INHERIT, /* inherit from parent */
> Index: io.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/io.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 io.c
> --- io.c 29 Nov 2019 05:09:50 - 1.8
> +++ io.c 12 Sep 2020 15:02:20 -
> @@ -25,8 +25,6 @@
> #include
> #include
>
> -#include
> -
> #include "extern.h"
>
> void
> Index: ip.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/ip.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 ip.c
> --- ip.c 16 Apr 2020 14:39:44 - 1.12
> +++ ip.c 12 Sep 2020 15:02:20 -
> @@ -25,8 +25,6 @@
> #include
> #include
>
> -#include
> -
> #include "extern.h"
>
> #define PREFIX_SIZE(x) (((x) + 7) / 8)
> Index: log.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/log.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 log.c
> --- log.c 29 Nov 2019 05:14:11 - 1.5
> +++ log.c 12 Sep 2020 15:02:20 -
> @@ -21,7 +21,6 @@
> #include
>
> #include
> -#include
>
> #include "extern.h"
>
> Index: mft.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 mft.c
> --- mft.c 30 Jun 2020 12:52:44 - 1.15
> +++ mft.c 12 Sep 2020 15:02:20 -
> @@ -24,7 +24,6 @@
> #include
> #include
>
> -#include
> #include
>
> #include "extern.h"
> Index: output-bgpd.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/output-bgpd.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 output-bgpd.c
> --- output-bgpd.c 28 Apr 2020 13:41:35 - 1.17
> +++ output-bgpd.c 12 Sep 2020 15:02:20 -
> @@ -16,7 +16,6 @@
> */
>
> #include
> -#include
>
> #include "extern.h"
>
> Index: output-bird.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/output-bird.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 output-bird.c
> --- output-bird.c 28 Apr 2020 15:03:39 - 1.9
> +++ output-bird.c 12 Sep 2020 15:02:20 -
> @@ -17,7 +17,6 @@
> */
>
> #include
> -#include
>
> #include "extern.h"
>
> Index: output-csv.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/output-csv.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 output-csv.c
> --- output-csv.c 28 Apr 2020 13:41:35 - 1.7
> +++ output-csv.c 12 Sep 2020 15:02:20 -
> @@ -16,7 +16,6 @@
> */
>
> #include
> -#include
>
> #include "extern.h"
>
> Index: output-json.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/output-json.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 output-json.c
> --- output-json.c 3 May 2020 20:24:02 - 1.12
> +
Re: tmpfs bug in reclaim
In the spirit of be careful what sticks to you, this has ok beck@ On Mon, Jul 13, 2020 at 11:56:18AM +0200, Gerhard Roth wrote: > tmpfs_reclaim() has to make sure that the VFS cache has no more > locks held for the vnode. Else vclean() could panic because v_holdcnt > is non-zero. > > I know that tmpfs is disabled by default, but it would be nice > to have this fix in the code base anyway. > > Gerhard > > > Index: sys/tmpfs/tmpfs_vnops.c > === > RCS file: /cvs/src/sys/tmpfs/tmpfs_vnops.c,v > retrieving revision 1.42 > diff -u -p -u -p -r1.42 tmpfs_vnops.c > --- sys/tmpfs/tmpfs_vnops.c 11 Jun 2020 09:18:43 - 1.42 > +++ sys/tmpfs/tmpfs_vnops.c 13 Jul 2020 09:48:37 - > @@ -1080,6 +1080,8 @@ tmpfs_reclaim(void *v) > racing = TMPFS_NODE_RECLAIMING(node); > rw_exit_write(&node->tn_nlock); > > + cache_purge(vp); > + > /* >* If inode is not referenced, i.e. no links, then destroy it. >* Note: if racing - inode is about to get a new vnode, leave it. >
Re: Stuck in Needbuf state, trying to understand (6.7)
On Mon, Jun 29, 2020 at 03:56:43PM -0400, sven falempin wrote: > On Mon, Jun 29, 2020 at 12:58 PM sven falempin > wrote: > > It works in the original problematic setup. > > Will it go to base ? > Yes. revision 1.201 date: 2020/07/14 06:02:50; author: beck; state: Exp; lines: +9 -3; commitid: G6yRUUYskLjLY0oH; Do not convert the NOCACHE buffers that come from a vnd strategy routine into more delayed writes if the vnd is mounted from a file on an MNT_ASYNC filesystem. This prevents a situaiton where the cleaner can not clean delayed writes out without making more delayed writes, and we end up waiting for the syncer to spit things occasionaly when it runs. noticed and reported by sven falempin on tech, who validated this fixes his issue. ok krw@
Re: Stuck in Needbuf state, trying to understand (6.7)
> Awesome, thanks! > > I will test that, ASAP, > do not hesitate to slay dragon, > i heard the bathing in the blood pool is good for the skin > > Little concern, I did the test without the MFS and ran into issues , > anyway i get back to you (or list ?) when i have test report with patched > kernel Yes, howver, you didn't tell my what options you had on the filesystem mounted when you did the test without MFS, because it matters. If you had your filesystem mounted ASYNC it would have exhibited the same behavoir. the issue is due to the async mount, which MFS does by default, not strictly to do with MFS. > > Again thanks for helping. > > -- > -- > - > Knowing is not enough; we must apply. Willing is not enough; we must do
Re: Stuck in Needbuf state, trying to understand (6.7)
On Sun, Jun 28, 2020 at 12:18:06PM -0400, sven falempin wrote:
> On Sun, Jun 28, 2020 at 2:40 AM Bryan Linton wrote:
>
> > On 2020-06-27 19:29:31, Bob Beck wrote:
> > >
> > > No.
> > >
> > > I know *exactly* what needbuf is but to attempt to diagnose what your
> > > problem is we need exact details. especially:
> > >
> > > 1) The configuration of your system including all the details of the
> > filesystems
> > > you have mounted, all options used, etc.
> > >
> > > 2) The script you are using to generate the problem (Not a paraphrasing
> > of what
> > > you think the script does) What filesystems it is using.
> > >
> >
> > Not the OP, but this problem sounds almost exactly like the bug I
> > reported last year.
> >
> > There is a detailed list of steps I used to reproduce the bug in
> > the following bug report.
> >
> > https://marc.info/?l=openbsd-bugs&m=156412299418191
> >
> > I was even able to bisect and identify the commit which first
> > caused the breakage for me.
> >
> >
> > ---8<---
> >
> > CVSROOT:/cvs
> > Module name:src
> > Changes by: b...@cvs.openbsd.org2019/05/08 06:40:57
> >
> > Modified files:
> > sys/kern : vfs_bio.c vfs_biomem.c
> >
> > Log message:
> > Modify the buffer cache to always flip recovered DMA buffers high.
> >
> > This also modifies the backoff logic to only back off what is requested
> > and not a "mimimum" amount. Tested by me, benno@, tedu@ anda ports build
> > by naddy@.
> >
> > ok tedu@
> >
> > ---8<---
> >
> > However, I have since migrated away from using vnd(4)s since I was
> > able to find other solutions that worked for my use cases. So I
> > may not be able to provide much additional information other than
> > what is contained in the above bug report.
> >
> > --
> > Bryan
> >
> > >
> > >
> >
> >
> Reproduction of BUG.
>
>
> # optional
> mkdir /tmpfs
> mount_mfs -o rw -s 2500M swap /tmpfs # i mounted through fstab so this line
> is not tested
> #the bug
> /bin/dd if=/dev/zero of=/tmpfs/img.dd count=0 bs=1 seek=25
> vnconfig vnd3 /tmpfs/img.dd
> printf "a a\n\n\n\nw\nq\n" | disklabel -E vnd3
> newfs vnd3a
> mount /dev/vnd3a /mnt
> cd /tmp && ftp https://cdn.openbsd.org/pub/OpenBSD/6.7/amd64/base67.tgz
> cd /mnt
> #will occur here (the mkdir was ub beedbuf state for a while ...
> for v in 1 2 3 4 5 6 7 8 9; do mkdir /tmp/$v; tar xzvf /tmp/base67.tgz -C
> /mnt/$v; done
>
> Ready to test patches.
>
>
So, your problem is that you have your vnd created in an mfs
filesystem, when I run your test with the vnd backed by a regular
filesystem (withe softdep even) it works fine.
The trouble happens when your VND has buffers cached in it's
"filesystem" but then is not flushing them out to the underlyin file
(vnode) that you have your vnd backed by. On normal filesystems this
works fine, since vnd tells the lower layer to not cache the writes
and to do them syncrhonously, to avoid an explosion of delayed writes
and dependencies of buffers.
The problem happens when we convert syncryonous bwrites to
asynchronous bdwrites if the fileystem is mounted ASYNC, which,
curiously, MFS always is (I don't know why, it doesn't really make any
sense, and I might even look at changing that) All the writes you do
end up being delayed anc chewing up more buffer space. And they are
all tied to one vnode (your image). once you exhaust the buffer
space, the cleaner runs, but as you have noticed can't clean out your
vnode until the syncer runs (every 60 seconds). This is why your
thing "takes a long time", and things stall in need buffer. softdep
has deep dark voodoo in it to avoid this problem and therefore when I
use a softdep filesystem instead of an ASYNC filesystem it works.
Anyway, what's below fixes your issue on my machine. I'm not sure I'm
happy that it's the final fix but it does fix it. there are many other
dragons lurking in here.
Index: sys/kern/vfs_bio.c
===
RCS file: /cvs/src/sys/kern/vfs_bio.c,v
retrieving revision 1.200
diff -u -p -u -p -r1.200 vfs_bio.c
--- sys/kern/vfs_bio.c 29 Apr 2020 02:25:48 - 1.200
+++ sys/kern/vfs_bio.c 29 Jun 2020 15:18:21 -
@@ -706,8 +706,14 @@ bwrite(struct buf *bp)
*/
async = ISSET(bp->b_flags, B_ASYNC);
if (!async && mp && ISSET(mp->mnt_flag, MNT_ASYNC)) {
- bdwrite(bp);
- return (0);
+ /*
+* Don't convert writes from VND on async filesystems
+* that already have delayed writes in the upper layer.
+*/
+ if (!ISSET(bp->b_flags, B_NOCACHE)) {
+ bdwrite(bp);
+ return (0);
+ }
}
/*
Re: Stuck in Needbuf state, trying to understand (6.7)
No.
I know *exactly* what needbuf is but to attempt to diagnose what your
problem is we need exact details. especially:
1) The configuration of your system including all the details of the filesystems
you have mounted, all options used, etc.
2) The script you are using to generate the problem (Not a paraphrasing of what
you think the script does) What filesystems it is using.
On Sat, Jun 27, 2020 at 08:09:18PM -0400, sven falempin wrote:
> On Fri, Jun 26, 2020 at 7:35 PM sven falempin
> wrote:
>
> >
> >
> > On Fri, Jun 26, 2020 at 5:22 PM Stuart Henderson
> > wrote:
> >
> >> On 2020/06/26 15:30, sven falempin wrote:
> >> > behavior confirmed on current.
> >> >
> >> > Once the process stalls, ( could be anything writing to the vnconfig
> >> disk,
> >> > cp , umount )
> >> > a few other calls like df , or ps, etc may hang, never the same
> >> > sp or mp kernel, reproduced on today's snapshots.
> >>
> >> vnconfig is used as part of "make release", many builds are done every
> >> week using this so it's not a general problem with vnconfig.
> >>
> >> Can you show some commands or a script to trigger the behaviour?
> >>
> >
> > the perl script use the system to call :
> >
> > vnconfig.
> > mount.
> > umount. <- saw hanged
> > cp.<- saw hanged
> > tar.<- saw hanged
> > svn up.<- saw hanged
> > and dd.
> > newfs.
> >
> > really nothing fancy, only stuff writing to disk got stuck.
> >
> > At one point it does a chroot but it never hangs near that , most of the
> > time it hangs before.
> >
> > The script has been used like 1000 times on 6.0 and maybe twice more on
> > 6.4.
> >
> > I have absolutely no idea what the 'needbuf' of top is .
> >
> > the script hangs at random position , always writing into vnconfig.
> >
> > I have no idea how to reproduce outside the perl script , so maybe it is
> > related
> > to some devious perl stdin/stdout buffer .
> >
> > Nevertheless there's like a 5% chance that's the script will work( slowly )
> >
> > Most of the system call are inside a routine to log
> >
> > sub debug_system {
> > $logger->debug('running: '.join(' ', @_));
> > return system(@_);
> > }
> >
> > so i can easily put things inside to try to understand the issue.
> >
> > It is really a strange behavior, and the device must be shut down
> > electrically.
> > Something really odd, i run syslogd on a buffer, and syslogc buffer is
> > stuck too
> > when the device stuck (but it supposed to be mostly already allocated
> > memory ).
> >
> > It's really like the vm does not want to give anymore bucket (<- i
> > don't know what i m talking about here,
> > but i looks like that anything that doesn't malloc is ok , computer reply
> > to ping , can do a few things for a while , and then complete
> > hang )
> >
> > I ran the 6.7 release on a VM somewhere and another device with many perl
> > script and they work.
> >
> > Only this fails 95% of the time and is VERY VERY slow when ok.
> > compared to what i saw in /usr/src the vnconfig is big , ( forgot to copy
> > df -h ),
> > like 2GB
> >
>
>
> i put ktrace in front of the perl system call
>
> An di was able to recover a 800MB trace
>
> $ kdump -f ./trace.out | tail -20
> kdump: realloc: Cannot allocate memory
> 25955 UNKNOWN(1634890859)
> 72466 ? CALL syscall()
>
>
> could that be of some use ?
>
>
> --
> --
> -
> Knowing is not enough; we must apply. Willing is not enough; we must do
Re: drop addtrust from cert.pem?
On Mon, Jun 01, 2020 at 06:04:17PM +0100, Stuart Henderson wrote: > OK to drop the expired AddTrust cert from cert.pem? yes, thanks. > > I checked against the firefox set, there are no new/removed certs that > work with libressl there. There are now two with GENERALIZEDTIME notAfter > dates from before 2050 that don't work though (I only remember seeing one > of those when I last looked).. but that is a separate issue. > > /C=EE/O=AS Sertifitseerimiskeskus/CN=EE Certification Centre Root > CA/emailAddress=p...@sk.ee > /C=PL/O=Unizeto Technologies S.A./OU=Certum Certification Authority/CN=Certum > Trusted Network CA 2 I suspect these can safely be dropped too. > > > Index: cert.pem > === > RCS file: /cvs/src/lib/libcrypto/cert.pem,v > retrieving revision 1.20 > diff -u -p -r1.20 cert.pem > --- cert.pem 10 Apr 2020 12:13:17 - 1.20 > +++ cert.pem 1 Jun 2020 16:59:23 - > @@ -350,58 +350,6 @@ LysRJyU3eExRarDzzFhdFPFqSBX/wge2sY0PjlxQ > LnPqZih4zR0Uv6CPLy64Lo7yFIrM6bV8+2ydDKXhlg== > -END CERTIFICATE- > > -### AddTrust AB > - > -=== /C=SE/O=AddTrust AB/OU=AddTrust External TTP Network/CN=AddTrust > External CA Root > -Certificate: > -Data: > -Version: 3 (0x2) > -Serial Number: 1 (0x1) > -Signature Algorithm: sha1WithRSAEncryption > -Validity > -Not Before: May 30 10:48:38 2000 GMT > -Not After : May 30 10:48:38 2020 GMT > -Subject: C=SE, O=AddTrust AB, OU=AddTrust External TTP Network, > CN=AddTrust External CA Root > -X509v3 extensions: > -X509v3 Subject Key Identifier: > -AD:BD:98:7A:34:B4:26:F7:FA:C4:26:54:EF:03:BD:E0:24:CB:54:1A > -X509v3 Key Usage: > -Certificate Sign, CRL Sign > -X509v3 Basic Constraints: critical > -CA:TRUE > -X509v3 Authority Key Identifier: > - > keyid:AD:BD:98:7A:34:B4:26:F7:FA:C4:26:54:EF:03:BD:E0:24:CB:54:1A > -DirName:/C=SE/O=AddTrust AB/OU=AddTrust External TTP > Network/CN=AddTrust External CA Root > -serial:01 > - > -SHA1 Fingerprint=02:FA:F3:E2:91:43:54:68:60:78:57:69:4D:F5:E4:5B:68:85:18:68 > -SHA256 > Fingerprint=68:7F:A4:51:38:22:78:FF:F0:C8:B1:1F:8D:43:D5:76:67:1C:6E:B2:BC:EA:B4:13:FB:83:D9:65:D0:6D:2F:F2 > --BEGIN CERTIFICATE- > -MIIENjCCAx6gAwIBAgIBATANBgkqhkiG9w0BAQUFADBvMQswCQYDVQQGEwJTRTEU > -MBIGA1UEChMLQWRkVHJ1c3QgQUIxJjAkBgNVBAsTHUFkZFRydXN0IEV4dGVybmFs > -IFRUUCBOZXR3b3JrMSIwIAYDVQQDExlBZGRUcnVzdCBFeHRlcm5hbCBDQSBSb290 > -MB4XDTAwMDUzMDEwNDgzOFoXDTIwMDUzMDEwNDgzOFowbzELMAkGA1UEBhMCU0Ux > -FDASBgNVBAoTC0FkZFRydXN0IEFCMSYwJAYDVQQLEx1BZGRUcnVzdCBFeHRlcm5h > -bCBUVFAgTmV0d29yazEiMCAGA1UEAxMZQWRkVHJ1c3QgRXh0ZXJuYWwgQ0EgUm9v > -dDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALf3GjPm8gAELTngTlvt > -H7xsD821+iO2zt6bETOXpClMfZOfvUq8k+0DGuOPz+VtUFrWlymUWoCwSXrbLpX9 > -uMq/NzgtHj6RQa1wVsfwTz/oMp50ysiQVOnGXw94nZpAPA6sYapeFI+eh6FqUNzX > -mk6vBbOmcZSccbNQYArHE504B4YCqOmoaSYYkKtMsE8jqzpPhNjfzp/haW+710LX > -a0Tkx63ubUFfclpxCDezeWWkWaCUN/cALw3CknLa0Dhy2xSoRcRdKn23tNbE7qzN > -E0S3ySvdQwAl+mG5aWpYIxG3pzOPVnVZ9c0p10a3CitlttNCbxWyuHv77+ldU9U0 > -WicCAwEAAaOB3DCB2TAdBgNVHQ4EFgQUrb2YejS0Jvf6xCZU7wO94CTLVBowCwYD > -VR0PBAQDAgEGMA8GA1UdEwEB/wQFMAMBAf8wgZkGA1UdIwSBkTCBjoAUrb2YejS0 > -Jvf6xCZU7wO94CTLVBqhc6RxMG8xCzAJBgNVBAYTAlNFMRQwEgYDVQQKEwtBZGRU > -cnVzdCBBQjEmMCQGA1UECxMdQWRkVHJ1c3QgRXh0ZXJuYWwgVFRQIE5ldHdvcmsx > -IjAgBgNVBAMTGUFkZFRydXN0IEV4dGVybmFsIENBIFJvb3SCAQEwDQYJKoZIhvcN > -AQEFBQADggEBALCb4IUlwtYj4g+WBpKdQZic2YR5gdkeWxQHIzZlj7DYd7usQWxH > -YINRsPkyPef89iYTx4AWpb9a/IfPeHmJIZriTAcKhjW88t5RxNKWt9x+Tu5w/Rw5 > -6wwCURQtjr0W4MHfRnXnJK3s9EK0hZNwEGe6nQY1ShjTK3rMUUKhemPR5ruhxSvC > -Nr4TDea9Y355e6cJDUCrat2PisP29owaQgVR1EX1n6diIWgVIEM8med8vSTYqZEX > -c4g/VhsxOBi0cQ+azcgOno4uG+GMmIPLHzHxREzGBHNJdmAPx/i9F4BrLunMTA5a > -mnkPIAou1Z5jJh5VkpTYghdae9C8x49OhgQ= > --END CERTIFICATE- > - > ### AffirmTrust > > === /C=US/O=AffirmTrust/CN=AffirmTrust Commercial >
Re: drop addtrust from cert.pem?
On Mon, Jun 01, 2020 at 07:17:28PM +0200, Theo Buehler wrote: > On Mon, Jun 01, 2020 at 06:04:17PM +0100, Stuart Henderson wrote: > > OK to drop the expired AddTrust cert from cert.pem? > > Thanks for taking care of this (and for checking the firefox set). I see > no reason to keep it. > > ok > > > I checked against the firefox set, there are no new/removed certs that > > work with libressl there. There are now two with GENERALIZEDTIME notAfter > > dates from before 2050 that don't work though (I only remember seeing one > > of those when I last looked).. but that is a separate issue. > > > > /C=EE/O=AS Sertifitseerimiskeskus/CN=EE Certification Centre Root > > CA/emailAddress=p...@sk.ee > > /C=PL/O=Unizeto Technologies S.A./OU=Certum Certification > > Authority/CN=Certum Trusted Network CA 2 > > Ugh...
Re: smtpd: make smarthost to use SNI when relaying
looks good to me
ok beck@
On Sun, May 31, 2020 at 03:38:00PM +0200, Sebastien Marie wrote:
> Hi,
>
> updated diff after millert@ and beck@ remarks:
> - use union to collapse in_addr + in6_addr
> - doesn't allocate buffer and directly use s->relay->domain->name
>
> Thanks.
> --
> Sebastien Marie
>
>
> diff 73b535ef4537e8454483912fc3420bc304759e96 /home/semarie/repos/openbsd/src
> blob - d384692a0e43de47d645142a6b99e72b7d83b687
> file + usr.sbin/smtpd/mta_session.c
> --- usr.sbin/smtpd/mta_session.c
> +++ usr.sbin/smtpd/mta_session.c
> @@ -26,6 +26,7 @@
> #include
> #include
>
> +#include
> #include
> #include
> #include
> @@ -1604,6 +1605,10 @@ mta_cert_init_cb(void *arg, int status, const char *na
> struct mta_session *s = arg;
> void *ssl;
> char *xname = NULL, *xcert = NULL;
> + union {
> + struct in_addr in4;
> + struct in6_addr in6;
> + } addrbuf;
>
> if (s->flags & MTA_WAIT)
> mta_tree_pop(&wait_tls_init, s->id);
> @@ -1623,6 +1628,22 @@ mta_cert_init_cb(void *arg, int status, const char *na
> free(xcert);
> if (ssl == NULL)
> fatal("mta: ssl_mta_init");
> +
> + /*
> + * RFC4366 (SNI): Literal IPv4 and IPv6 addresses are not
> + * permitted in "HostName".
> + */
> + if (s->relay->domain->as_host == 1) {
> + if (inet_pton(AF_INET, s->relay->domain->name, &addrbuf) != 1 &&
> + inet_pton(AF_INET6, s->relay->domain->name, &addrbuf) != 1)
> {
> + log_debug("%016"PRIx64" mta tls setting SNI name=%s",
> + s->id, s->relay->domain->name);
> + if (SSL_set_tlsext_host_name(ssl,
> s->relay->domain->name) == 0)
> + log_warnx("%016"PRIx64" mta tls setting SNI
> failed",
> +s->id);
> + }
> + }
> +
> io_start_tls(s->io, ssl);
> }
>
>
Re: smtpd: make smarthost to use SNI when relaying
On Sat, May 30, 2020 at 05:40:43PM +0200, Sebastien Marie wrote:
> Hi,
>
> I am looking to make smtpd to set SNI (SSL_set_tlsext_host_name) when
> connecting
> to smarthost when relaying mail.
>
> After digging a bit in libtls (to stole the right code) and smtpd (to see
> where
> to put the stolen code), I have the following diff:
>
>
> diff 73b535ef4537e8454483912fc3420bc304759e96 /home/semarie/repos/openbsd/src
> blob - d384692a0e43de47d645142a6b99e72b7d83b687
> file + usr.sbin/smtpd/mta_session.c
> --- usr.sbin/smtpd/mta_session.c
> +++ usr.sbin/smtpd/mta_session.c
> @@ -26,6 +26,7 @@
> #include
> #include
>
> +#include
> #include
> #include
> #include
> @@ -1604,6 +1605,8 @@ mta_cert_init_cb(void *arg, int status, const char *na
> struct mta_session *s = arg;
> void *ssl;
> char *xname = NULL, *xcert = NULL;
> + struct in_addr addrbuf4;
> + struct in6_addr addrbuf6;
>
> if (s->flags & MTA_WAIT)
> mta_tree_pop(&wait_tls_init, s->id);
> @@ -1623,6 +1626,24 @@ mta_cert_init_cb(void *arg, int status, const char *na
> free(xcert);
> if (ssl == NULL)
> fatal("mta: ssl_mta_init");
> +
> + /*
> + * RFC4366 (SNI): Literal IPv4 and IPv6 addresses are not
> + * permitted in "HostName".
> + */
> + if (s->relay->domain->as_host == 1) {
> + xname = xstrdup(s->relay->domain->name);
This allocation appears to be unnecessary. I believe you should be
able to simply check with inet_pton, and then use
s->relay->domain->name
On a strictly smtpd front, it seems odd to have somthing called
domain->name possibly being an ip address in text form. It would seem
prudent to keep these things separate as we resolve things. (domain->ip, or
domain->mxip if we have resolved down to that might be better) but
that's a separate issue and larger change.
> + if (inet_pton(AF_INET, xname, &addrbuf4) != 1 &&
> + inet_pton(AF_INET6, xname, &addrbuf6) != 1) {
> + log_info("%016"PRIx64" mta setting SNI name=%s",
> + s->id, xname);
> + if (SSL_set_tlsext_host_name(ssl, xname) == 0)
> + log_warnx("%016"PRIx64" mta setting SNI failed",
> +s->id);
> + }
> + free(xname);
> + }
> +
> io_start_tls(s->io, ssl);
> }
>
>
>
> For what I understood:
>
> mta_cert_init_cb() function is responsable to prepare a connection. the SSL
> initialization (SSL_new() call) occured in ssl_mta_init() which was just
> called,
> so it seems it is the right place to call SSL_set_tlsext_host_name().
>
> We just need the hostname to configure it.
>
> Regarding mta_session structure, relay->domain->as_host is set to 1 when the
> domain is linked to smarthost configuration (or when the mx is ip address I
> think). And in smarthost case, the domain->name is the hostname. For SNI, we
> are
> excluding ip, so I assume it should copte with domain->name as ip.
>
> Does someone with better understanding of smtpd code source could confirm the
> approch is right and comment ?
>
> Please note I have only tested it on simple configuration.
>
> Thanks.
> --
> Sebastien Marie
>
Re: official ports vs DEBUG_PACKAGES
> (iirc python does something strange) Inconcievable!
Re: official ports vs DEBUG_PACKAGES
On Fri, May 29, 2020 at 06:14:44PM +0200, Marc Espie wrote:
> In a trace:
>
> > > > #3 0x15e48c95459e in WebVfx::shutdown ()
> > > > at /usr/obj/ports/webvfx-1.2.0/webvfx-1.2.0/webvfx/webvfx.cpp:193
>
> Now, this is NOT the default location for WRKOBJDIR, but we are shipping
> packages with debug information...
>
> This could be a bit cumbersome, if in order to debug locally, you have
> to make patch NO_DEPENDS=Yes, and then you figure out you have to mimic the
> "official" setup for ports, which does not even match the default.
>
> There are about 3 solutions to that:
> - change the bulk machines to /usr/ports/pobj
> - change the ports default to /usr/obj/ports
It's not realpathing is it? correct me if I am wrong but would
making /usr/ports/pobj a symlink to /usr/obj/ports on the bulk
machines and running them with the default config just "fix" this?
> - have some magic I don't know in ELF handling that would allow to either
> tweak the default location/introduce ${WRKOBJDIR} in that debug info.
>
Re: nsd 4.3.1
> On May 8, 2020, at 03:00, Stuart Henderson wrote: > > On 2020/05/08 06:58, Florian Obser wrote: >> I'm running this for about 2 weeks or so. >> Tests, OKs? > > Just off to look at a radio link in a church tower that I suspect a pigeon > may have knocked out of alignment, This is possibly one of the most English outages I have ever heard of > I'll put this on some machines when I get > back, just wanted to comment: > >> - I'm adding RELNOTES, it helps my process of merging upstream >> - I forgot to update ChangeLog previously, so there are unrelated >> changes in there as well. I'll first bring in the ChangeLog and >> RELNOTES changes up to 4.2.4 and then update to 4.3.1 on top. > > Yes please, I found this very helpful when doing Unbound updates. > > I wonder if testers will notice what's different about responses (I think > it's a good thing). >
Recent 'ftplist' changes visible in the installer
So, as some of you know the installer hits ftp.openbsd.org during the install process to query a CGI to provide you with a list of nearby mirrors and some other useful things. I've recently made some changes to modernize and improve this after the retirement of the GEO:IP modules and increasing fun with using maxmind without hitting an API server. While it's true that if you install from a mirror or hit the cgi, we know where you came from, I'm not super comfortable handing that information to a third party for no good reason. Fortunately with the help of fcambus@, I managed to switch over to the free dbip databases, which can be installed from ports and kept local to the machine. With the help of afresh1@ we managed to get self contained timezone information as well, based on some earlier work by andy hook. Which is long way to say, there should be improvements in what mirrors you are offered in the installer, especially if you are in some of the more isolated places lacking an openbsd mirror in your country. If you can try it out, and you see anything problematic, please let me know. Thanks -Bob
Re: suggest to run rpki-client hourly
On Mon, Apr 13, 2020 at 09:23:23PM -0600, Todd C. Miller wrote:
> On Mon, 13 Apr 2020 20:27:30 -0600, Bob Beck wrote:
>
> > In my hearts desire I'd love for "R" to be chosen for each line once at
> > start
> > up. (so in
> > the above example the things are randomly distributed). but not sure how
> > har
> > d that is..
> >
> > If it saves code and effort I really think this is only useful for hours
> > and
> > minutes
>
> Here's a version that uses a suggestion from Theo to support ranges
> like "0~30" to mean a random number between 0 and 30 and just a
> bare "~" to mean a random value in the valid range for that field.
>
> The random values are chosen when the file is parsed which means
> that on reload due to crontab edit they will change. I was trying
> to avoid that initially but now I don't think it is a big deal.
Like this one plenty. I think it's ok the values change on reload.
>
> - todd
>
> Index: usr.sbin/cron/entry.c
> ===
> RCS file: /cvs/src/usr.sbin/cron/entry.c,v
> retrieving revision 1.49
> diff -u -p -u -r1.49 entry.c
> --- usr.sbin/cron/entry.c 13 Jun 2018 11:27:30 - 1.49
> +++ usr.sbin/cron/entry.c 14 Apr 2020 01:52:13 -
> @@ -450,13 +450,30 @@ static int
> get_range(bitstr_t *bits, int low, int high, const char *names[],
> int ch, FILE *file)
> {
> - /* range = number | number "-" number [ "/" number ]
> + /* range = number | number* "~" number* | number "-" number ["/" number]
>*/
>
> int i, num1, num2, num3;
>
> + /* XXX - parse ~, X~Y, X~ and ~Y */
> +
> + if (ch == '~') {
> + /* '~' means a random number [high, low]
> + */
> + num1 = arc4random_uniform(high - low + 1) + low;
> + ch = get_char(file);
> + if (ch == EOF)
> + return (EOF);
> +
> + if (EOF == set_element(bits, low, high, num1)) {
> + unget_char(ch, file);
> + return (EOF);
> + }
> + return (ch);
> + }
> +
> if (ch == '*') {
> - /* '*' means "first-last" but can still be modified by /step
> + /* '*' means "high-low" but can still be modified by /step
>*/
> num1 = low;
> num2 = high;
> @@ -464,30 +481,45 @@ get_range(bitstr_t *bits, int low, int h
> if (ch == EOF)
> return (EOF);
> } else {
> - ch = get_number(&num1, low, names, ch, file, ",- \t\n");
> + ch = get_number(&num1, low, names, ch, file, ",-~ \t\n");
> if (ch == EOF)
> return (EOF);
>
> - if (ch != '-') {
> - /* not a range, it's a single number.
> - */
> - if (EOF == set_element(bits, low, high, num1)) {
> - unget_char(ch, file);
> - return (EOF);
> - }
> - return (ch);
> - } else {
> - /* eat the dash
> + switch (ch) {
> + case '-':
> + case '~':
> + num3 = ch;
> +
> + /* eat the dash/tilde
>*/
> ch = get_char(file);
> if (ch == EOF)
> return (EOF);
>
> - /* get the number following the dash
> + /* get the number following the dash/tilde
>*/
> ch = get_number(&num2, low, names, ch, file, "/, \t\n");
> if (ch == EOF || num1 > num2)
> return (EOF);
> +
> + /* if it's a standard range, we're done here.
> + */
> + if (num3 == '-')
> + break;
> +
> + /* get a random number in the range [num1, num2]
> + */
> + num3 = num1;
> + num1 = arc4random_uniform(num2 - num3 + 1) + num3;
> + /* FALLTHROUGH */
> + default:
> + /* not a range, it's a single number.
> + */
> + if (EOF == set_element(bits, low, high, num1)) {
> + unget_char(ch, file);
> + return (EOF);
> + }
> + return (ch);
> }
> }
>
>
Re: suggest to run rpki-client hourly
A couple thoughts:
1) I'm not sure how useful random months or days of the week are, but for
consistency maybe?
2) if I do something like
R * * * * /usr/local/bin/thing1
R * * * * /usr/local/bin/thing2
R * * * * /usr/local/bin/thing3
...
R * * * * /usr/local/bin/thing1000
I still have the thundering herd problem a bit, in that all my things fire at
the same R.
In my hearts desire I'd love for "R" to be chosen for each line once at
startup. (so in
the above example the things are randomly distributed). but not sure how hard
that is..
If it saves code and effort I really think this is only useful for hours and
minutes
On Mon, Apr 13, 2020 at 12:54:34PM -0600, Todd C. Miller wrote:
> On Mon, 13 Apr 2020 10:00:52 -0600, Bob Beck wrote:
>
> > +1000. a new random time chosen at cron start.
> >
> > We see this all the time, and it would be a lot better than the hacks for
> > all
> > the things
> >
> > "R" for random sounds good to me
>
> How about this? If you guys like it I'll update the man page too.
> So far only tested with a crontab entry like:
>
> R * * * * date
>
> - todd
>
> Index: usr.sbin/cron/cron.c
> ===
> RCS file: /cvs/src/usr.sbin/cron/cron.c,v
> retrieving revision 1.78
> diff -u -p -u -r1.78 cron.c
> --- usr.sbin/cron/cron.c 11 Feb 2020 12:42:01 - 1.78
> +++ usr.sbin/cron/cron.c 13 Apr 2020 16:25:44 -
> @@ -129,6 +129,7 @@ main(int argc, char *argv[])
> syslog(LOG_INFO, "(CRON) STARTUP (%s)", CRON_VERSION);
> }
>
> + init_random();
> load_database(&database);
> scan_atjobs(&at_database, NULL);
> set_time(TRUE);
> Index: usr.sbin/cron/entry.c
> ===
> RCS file: /cvs/src/usr.sbin/cron/entry.c,v
> retrieving revision 1.49
> diff -u -p -u -r1.49 entry.c
> --- usr.sbin/cron/entry.c 13 Jun 2018 11:27:30 - 1.49
> +++ usr.sbin/cron/entry.c 13 Apr 2020 18:49:56 -
> @@ -54,6 +54,12 @@ static const char *ecodes[] = {
> "out of memory"
> };
>
> +typedef enum r_type {
> + r_minute, r_hour, r_dom, r_month, r_dow
> +} rtype_e;
> +
> +static int rand_vals[5];
> +
> static const char *MonthNames[] = {
> "Jan", "Feb", "Mar", "Apr", "May", "Jun",
> "Jul", "Aug", "Sep", "Oct", "Nov", "Dec",
> @@ -70,6 +76,8 @@ static int get_list(bitstr_t *, int, int
> get_number(int *, int, const char *[], int, FILE *, const char
> *),
> set_element(bitstr_t *, int, int, int);
>
> +static int set_random(bitstr_t *, int, int, FILE *);
> +
> void
> free_entry(entry *e)
> {
> @@ -187,10 +195,15 @@ load_entry(FILE *file, void (*error_func
> goto eof;
> }
> } else {
> - if (ch == '*')
> - e->flags |= MIN_STAR;
> - ch = get_list(e->minute, FIRST_MINUTE, LAST_MINUTE,
> - NULL, ch, file);
> + if (ch == 'R') {
> + ch = set_random(e->minute, rand_vals[r_minute], ch,
> + file);
> + } else {
> + if (ch == '*')
> + e->flags |= MIN_STAR;
> + ch = get_list(e->minute, FIRST_MINUTE, LAST_MINUTE,
> + NULL, ch, file);
> + }
> if (ch == EOF) {
> ecode = e_minute;
> goto eof;
> @@ -199,10 +212,14 @@ load_entry(FILE *file, void (*error_func
> /* hours
>*/
>
> - if (ch == '*')
> - e->flags |= HR_STAR;
> - ch = get_list(e->hour, FIRST_HOUR, LAST_HOUR,
> - NULL, ch, file);
> + if (ch == 'R') {
> + ch = set_random(e->hour, rand_vals[r_hour], ch, file);
> + } else {
> + if (ch == '*')
> + e->flags |= HR_STAR;
> + ch = get_list(e->hour, FIRST_HOUR, LAST_HOUR,
> + NULL, ch, file);
> + }
> if (ch == EOF) {
> ecode = e_hour;
> goto eof;
> @@ -211,10 +228,15 @@ load_entry(FILE *file, void (*error_fu
Re: suggest to run rpki-client hourly
On Mon, Apr 13, 2020 at 09:56:52AM -0600, Todd C. Miller wrote: > On Mon, 13 Apr 2020 09:37:14 -0600, "Theo de Raadt" wrote: > > > While I understand what RANDOM is trying to do, I am not a fan. I've > > thought often of an improvement, where the minute marker in a crontab > > file could be a letter 'R', which indicates the specific random value > > for this cron start. That value would be selected at cron start, and > > remain constant for the runtime of cron. > > I was thinking the same thing, but using '?' as the character. It > doesn't really matter what we choose, the implementation should be > straight-forward. +1000. a new random time chosen at cron start. We see this all the time, and it would be a lot better than the hacks for all the things "R" for random sounds good to me > > - todd >
Re: fts and unveil issue
yes you are seeing the limitation of 6.4 unveil as mentioned at the bottom
of the man page. this should be fixed in current
On Sun, Feb 3, 2019 at 03:29 Kristaps Dzonsons wrote:
> When I unveil(2), fts doesn't behave well. But only in a subtle way.
> Enclosed is a demonstration. I found this with openrsync, which unveils
> before using fts_open to scan for files.
>
> When run with a directory with only empty subdirectories or just files,
> this works fine. But when run with a directory that contains other
> non-empty directories, the fts_read fails in the nested directories.
>
> This is on stock OpenBSD 6.4, syspatched, amd64.
>
> For example, consider the following abridged output (to fit into this
> e-mail window):
>
> % find ~/tmp/test -ls
> drwxr-xr-x3 /home/kristaps/tmp/test
> drwxr-xr-x3 /home/kristaps/tmp/test/test2
> -rw-r--r--1 /home/kristaps/tmp/test/test2/test2
> drwxr-xr-x2 /home/kristaps/tmp/test/test2/test3
> -rw-r--r--1 /home/kristaps/tmp/test/test1
> % gcc -W -Wall -Wextra -g foo.c
> % ./a.out /home/kristaps/tmp/test/
> a.out: /home/kristaps/tmp/test/
> a.out: /home/kristaps/tmp/test/test2
> a.out: /home/kristaps/tmp/test/test2/test2
> a.out: /home/kristaps/tmp/test/test2/test3
> a.out: /home/kristaps/tmp/test/test2/test3
> a.out: /home/kristaps/tmp/test/test2
> a.out: /home/kristaps/tmp/test/test1
> a.out: /home/kristaps/tmp/test/
> a.out: TRYING AGAIN.
> a.out: /home/kristaps/tmp/test/
> a.out: /home/kristaps/tmp/test/test2
> a.out: /home/kristaps/tmp/test/test2/test2: no stat
> a.out: ...but regular stat works
>
> So the first nested child fails (regardless of whether it's a file or
> directory, by the way). But a regular stat still works.
>
> The same happens if I use unveil("/", "r").
>
Re: unveil spamlogd
ok beck@ as well On Wed, Oct 24, 2018 at 06:13 Todd C. Miller wrote: > On Wed, 24 Oct 2018 08:05:11 +0100, Ricardo Mestre wrote: > > > The only file that spamlogd needs to access after calling pledge is > > PATH_SPAMD_DB, so unveil it with O_RDWR permissions. > > Looks good. OK millert@ > > - todd >
Re: Reuse VM ids.
works here and I like it. but probably for after unlock
On Sun, Oct 7, 2018 at 22:11 Mischa Peters wrote:
> No idea if the code works yet.
> Hopefully I can try later. But love the idea.
>
> Mischa
>
> > On 8 Oct 2018, at 04:31, Ori Bernstein wrote:
> >
> > Keep a list of known vms, and reuse the VM IDs. This means that when
> using
> > '-L', the IP addresses of the VMs are stable.
> >
> > diff --git usr.sbin/vmd/config.c usr.sbin/vmd/config.c
> > index af12b790002..522bae32501 100644
> > --- usr.sbin/vmd/config.c
> > +++ usr.sbin/vmd/config.c
> > @@ -61,7 +61,10 @@ config_init(struct vmd *env)
> >if (what & CONFIG_VMS) {
> >if ((env->vmd_vms = calloc(1, sizeof(*env->vmd_vms))) == NULL)
> >return (-1);
> > +if ((env->vmd_known = calloc(1, sizeof(*env->vmd_known))) ==
> NULL)
> > +return (-1);
> >TAILQ_INIT(env->vmd_vms);
> > +TAILQ_INIT(env->vmd_known);
> >}
> >if (what & CONFIG_SWITCHES) {
> >if ((env->vmd_switches = calloc(1,
> > diff --git usr.sbin/vmd/vmd.c usr.sbin/vmd/vmd.c
> > index 18a5e0d3d5d..732691b4381 100644
> > --- usr.sbin/vmd/vmd.c
> > +++ usr.sbin/vmd/vmd.c
> > @@ -1169,6 +1169,27 @@ vm_remove(struct vmd_vm *vm, const char *caller)
> >free(vm);
> > }
> >
> > +static uint32_t
> > +claim_vmid(const char *name)
> > +{
> > +struct name2id *n2i = NULL;
> > +
> > +TAILQ_FOREACH(n2i, env->vmd_known, entry)
> > +if (strcmp(n2i->name, name) == 0)
> > +return n2i->id;
> > +
> > +if (++env->vmd_nvm == 0)
> > +fatalx("too many vms");
> > +if ((n2i = calloc(1, sizeof(struct name2id))) == NULL)
> > +fatalx("could not alloc vm name");
> > +n2i->id = env->vmd_nvm;
> > +if (strlcpy(n2i->name, name, sizeof(n2i->name)) >=
> sizeof(n2i->name))
> > +fatalx("overlong vm name");
> > +TAILQ_INSERT_TAIL(env->vmd_known, n2i, entry);
> > +
> > +return n2i->id;
> > +}
> > +
> > int
> > vm_register(struct privsep *ps, struct vmop_create_params *vmc,
> > struct vmd_vm **ret_vm, uint32_t id, uid_t uid)
> > @@ -1300,11 +1321,8 @@ vm_register(struct privsep *ps, struct
> vmop_create_params *vmc,
> >vm->vm_cdrom = -1;
> >vm->vm_iev.ibuf.fd = -1;
> >
> > -if (++env->vmd_nvm == 0)
> > -fatalx("too many vms");
> > -
> >/* Assign a new internal Id if not specified */
> > -vm->vm_vmid = id == 0 ? env->vmd_nvm : id;
> > +vm->vm_vmid = (id == 0) ? claim_vmid(vcp->vcp_name) : id;
> >
> >log_debug("%s: registering vm %d", __func__, vm->vm_vmid);
> >TAILQ_INSERT_TAIL(env->vmd_vms, vm, vm_entry);
> > diff --git usr.sbin/vmd/vmd.h usr.sbin/vmd/vmd.h
> > index b7c012854e8..86fad536e59 100644
> > --- usr.sbin/vmd/vmd.h
> > +++ usr.sbin/vmd/vmd.h
> > @@ -276,6 +276,13 @@ struct vmd_user {
> > };
> > TAILQ_HEAD(userlist, vmd_user);
> >
> > +struct name2id {
> > +charname[VMM_MAX_NAME_LEN];
> > +int32_tid;
> > +TAILQ_ENTRY(name2id)entry;
> > +};
> > +TAILQ_HEAD(name2idlist, name2id);
> > +
> > struct address {
> >struct sockaddr_storage ss;
> >int prefixlen;
> > @@ -300,6 +307,7 @@ struct vmd {
> >
> >uint32_t vmd_nvm;
> >struct vmlist*vmd_vms;
> > +struct name2idlist*vmd_known;
> >uint32_t vmd_nswitches;
> >struct switchlist*vmd_switches;
> >struct userlist*vmd_users;
> >
> > --
> >Ori Bernstein
> >
>
>
Re: openssl s_time: different tally marks for different TLS versions
I'm generally opposed to breaking stdout compatibility with the
"openssl" command tools because we have no clue what shell scripts and
other applications this will break.
with a *very good reason* I think it's ok, but this (I think this
looks better) isn't one of them. the "openssl" command is kept the
way it is *for compatibilityt with crap that wants it*.
If you truly dislike the output - WRITE A NEW TOOL THAT DOESN'T SUCK ;)
On Sat, Sep 15, 2018 at 1:21 PM Scott Cheloha wrote:
>
> Bump.
>
> On Tue, Aug 28, 2018 at 10:33:34AM -0500, Scott Cheloha wrote:
> > Two diffs here.
> >
> > First, move the tally mark printing out of the benchmark loop.
> >
> > Second, print '0' for TLS 1.0, '1' for TLS 1.1, etc.
> >
> > This breaks stdout compatibility with OpenSSL s_time, and prior
> > versions of s_time in general, because 't' was used for TLS 1.0
> > (behavior change) and '2' was used for SSLv2 (marker collision).
> >
> > (The choice of a single character as the mark predated any plans
> > for a successor to SSL. The choice of 't' predated any plans for
> > a revision to TLS.)
> >
> > I think the utility of distinguishing between the various TLS
> > versions at a glance outweighs the value of compatibility with
> > older versions of the software. Especially given how haphazard
> > the stdout behavior of this code is anyway, I don't think we're
> > going to break a zillion scripts. The primary utility of this
> > app is interactive testing and eyeballing your performance.
> >
> > But... if this is unacceptable the alternative is to just print
> > 't' for any and all TLS versions. I think this is less useful,
> > but one can always use s_client, so it isn't the end of the world.
> >
> > Thoughts? ok?
> >
> > PS. Using DTLS to encrypt HTTP isn't a thing, right? It isn't
> > useful to check for DTLS1_VERSION from SSL_version(3)?
> >
> > Diff 1:
> >
> > Index: s_time.c
> > ===
> > RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
> > retrieving revision 1.31
> > diff -u -p -r1.31 s_time.c
> > --- s_time.c 28 Aug 2018 14:30:48 - 1.31
> > +++ s_time.c 28 Aug 2018 15:13:18 -
> > @@ -92,6 +92,7 @@ extern int verify_depth;
> > static void s_time_usage(void);
> > static int run_test(SSL *);
> > static int benchmark(int);
> > +static void print_tally_mark(SSL *);
> >
> > static SSL_CTX *tm_ctx = NULL;
> > static const SSL_METHOD *s_time_meth = NULL;
> > @@ -393,6 +394,24 @@ run_test(SSL *scon)
> > return 1;
> > }
> >
> > +static void
> > +print_tally_mark(SSL *scon)
> > +{
> > + int ver;
> > +
> > + if (SSL_session_reused(scon))
> > + ver = 'r';
> > + else {
> > + ver = SSL_version(scon);
> > + if (ver == TLS1_VERSION)
> > + ver = 't';
> > + else
> > + ver = '*';
> > + }
> > + fputc(ver, stdout);
> > + fflush(stdout);
> > +}
> > +
> > static int
> > benchmark(int reuse_session)
> > {
> > @@ -400,7 +419,6 @@ benchmark(int reuse_session)
> > int nConn = 0;
> > SSL *scon = NULL;
> > int ret = 1;
> > - int ver;
> >
> > if (reuse_session) {
> > /* Get an SSL object so we can reuse the session id */
> > @@ -429,18 +447,7 @@ benchmark(int reuse_session)
> > if (!run_test(scon))
> > goto end;
> > nConn += 1;
> > - if (SSL_session_reused(scon))
> > - ver = 'r';
> > - else {
> > - ver = SSL_version(scon);
> > - if (ver == TLS1_VERSION)
> > - ver = 't';
> > - else
> > - ver = '*';
> > - }
> > - fputc(ver, stdout);
> > - fflush(stdout);
> > -
> > + print_tally_mark(scon);
> > if (!reuse_session) {
> > SSL_free(scon);
> > scon = NULL;
> >
> > Diff 1+2:
> >
> > Index: s_time.c
> > ===
> > RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
> > retrieving revision 1.31
> > diff -u -p -r1.31 s_time.c
> > --- s_time.c 28 Aug 2018 14:30:48 - 1.31
> > +++ s_time.c 28 Aug 2018 15:15:27 -
> > @@ -92,6 +92,7 @@ extern int verify_depth;
> > static void s_time_usage(void);
> > static int run_test(SSL *);
> > static int benchmark(int);
> > +static void print_tally_mark(SSL *);
> >
> > static SSL_CTX *tm_ctx = NULL;
> > static const SSL_METHOD *s_time_meth = NULL;
> > @@ -393,6 +394,33 @@ run_test(SSL *scon)
> > return 1;
> > }
> >
> > +static void
> > +print_tally_mark(SSL *scon)
> > +{
> > + int mark;
> > +
> > + if (SSL_session_reused(scon)) {
> > + mark = 'r';
> > + goto print;
> > + }
> > + switch (SSL_version(scon)) {
> > + case TLS1_VERSION:
> > +
Nuke PLEDGE_STAT for further pledge/unveil disentaglement.
So this gets rid of unveil's PLEDGE_STAT.
Instead we use UNVEIL_INSPECT which is set by the stat and access opeerations
that are needed for realpath() type traversals that effectively call stat/access
for each component of a pathname before doing a final operation on the end.
The intended semantic of UNVEIL_INSPECT (which is only used in the kernel)
is to allow inspection of vnodes that are traversed on the way to an
unveil'ed component - just like what PLEDGE_STAT did.
This also removes the use of PLEDGE_STATLIE in unveil - theo and I had
discussed that this was probably fine in lubljana, but I never did it
then. I'll remove STATLIE later if we decide that's the way we
are going.
Passes regress - realpath still works, etc. etc.
ok?
Index: kern/kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.239
diff -u -p -u -p -r1.239 kern_pledge.c
--- kern/kern_pledge.c 2 Aug 2018 15:34:07 - 1.239
+++ kern/kern_pledge.c 5 Aug 2018 17:45:52 -
@@ -608,14 +608,14 @@ pledge_namei(struct proc *p, struct name
switch (p->p_pledge_syscall) {
case SYS_access:
/* tzset() needs this. */
- if ((ni->ni_pledge == (PLEDGE_RPATH | PLEDGE_STAT)) &&
+ if (ni->ni_pledge == PLEDGE_RPATH &&
strcmp(path, "/etc/localtime") == 0) {
ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
return (0);
}
/* when avoiding YP mode, getpw* functions touch this */
- if (ni->ni_pledge == (PLEDGE_RPATH | PLEDGE_STAT) &&
+ if (ni->ni_pledge == PLEDGE_RPATH &&
strcmp(path, "/var/run/ypbind.lock") == 0) {
if (p->p_p->ps_pledge & PLEDGE_GETPW) {
ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
@@ -713,7 +713,7 @@ pledge_namei(struct proc *p, struct name
break;
case SYS_readlink:
/* Allow /etc/malloc.conf for malloc(3). */
- if ((ni->ni_pledge == (PLEDGE_RPATH | PLEDGE_STAT)) &&
+ if ((ni->ni_pledge == PLEDGE_RPATH) &&
strcmp(path, "/etc/malloc.conf") == 0) {
ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
return (0);
@@ -721,7 +721,7 @@ pledge_namei(struct proc *p, struct name
break;
case SYS_stat:
/* DNS needs /etc/resolv.conf. */
- if ((ni->ni_pledge == (PLEDGE_RPATH | PLEDGE_STAT)) &&
+ if ((ni->ni_pledge == PLEDGE_RPATH) &&
(p->p_p->ps_pledge & PLEDGE_DNS) &&
strcmp(path, "/etc/resolv.conf") == 0) {
ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
@@ -732,9 +732,9 @@ pledge_namei(struct proc *p, struct name
/*
* Ensure each flag of ni_pledge has counterpart allowing it in
-* ps_pledge. discard PLEDGE_STAT as it is unveil(2) stuff.
+* ps_pledge.
*/
- if ((ni->ni_pledge & ~PLEDGE_STAT) & ~p->p_p->ps_pledge)
+ if (ni->ni_pledge & ~p->p_p->ps_pledge)
return (pledge_fail(p, EPERM, (ni->ni_pledge &
~p->p_p->ps_pledge)));
/* continue, and check unveil if present */
Index: kern/kern_unveil.c
===
RCS file: /cvs/src/sys/kern/kern_unveil.c,v
retrieving revision 1.11
diff -u -p -u -p -r1.11 kern_unveil.c
--- kern/kern_unveil.c 5 Aug 2018 14:23:57 - 1.11
+++ kern/kern_unveil.c 5 Aug 2018 17:45:52 -
@@ -379,13 +379,20 @@ unveil_add_vnode(struct process *pr, str
return (uv);
}
-void
+int
unveil_add_traversed_vnodes(struct proc *p, struct nameidata *ndp)
{
/*
-* add the traversed vnodes with 0 flags if they
-* are not already present.
+* Add the traversed vnodes with the UNVEIL_INSPECT flag
+* if they are not already present to allow traversal
+* operations such as access and stat. This lets
+* TOCTOU fans that call access on all components of
+* an unveil'ed path before the final operation
+* work.
*/
+ int ret = 0;
+ struct unveil *uv;
+
if (ndp->ni_tvpsize) {
size_t i;
@@ -394,10 +401,15 @@ unveil_add_traversed_vnodes(struct proc
if (unveil_lookup(vp, p) == NULL) {
vref(vp);
vp->v_uvcount++;
- unveil_add_vnode(p->p_p, vp);
+ uv = unveil_add_vnode(p->p_p, vp);
+ if (uv != NULL)
+ uv->uv_flags = UNVEIL_INSPECT;
+ else
+ ret = E2BIG;
}
}
}
+ return ret
Re: unveil: incomplete unveil_flagmatch semantic
> Some examples that will need consideration for unveil(2): > - mount(2) > - unmount(2) > - quotactl(2) > - chroot(2) > - getfh(2) > - acct(2) > - coredump() > - loadfirmware() - I think ifconfig(1) could make the kernel loading a > firmware for some network card > > so having ni_unveil separated from ni_pledge could be good to be able to > manage these namei() calls in unveiled programs. > And yes, I am in violent agreement with this :) this lets us have a cleaner separation and unveil things that aren't pledgeable.
Re: unveil: incomplete unveil_flagmatch semantic
> On Sat, Aug 04, 2018 at 10:40:11AM -0600, Bob Beck wrote: > > On Fri, Aug 03, 2018 at 06:31:00AM +0200, Sebastien Marie wrote: > > > On Thu, Aug 02, 2018 at 03:42:03PM +0200, Sebastien Marie wrote: > > > > On Mon, Jul 30, 2018 at 07:55:35AM -0600, Bob Beck wrote: > > > > > yeah the latter will be the way to go > > > > > > > > > > > > > > > new diff with direct lookup using an indirection table. > > > > > > > > > > new (emergency) version with PLEDGE_CHOWN consideration for unveil(2). > > > > > > sorry for having missed it. > > > > > > > All good because you gave me inspiration, after I ran your diff. > > > > I tied unveil to the pledge flags when I first did it because it was > > convenient - I think this thig with chmod (and the awkwardness of > > PLEDGE_STAT, etc. etc.) just shows that this was a decision of > > convienience in the short term that is going to bite us in the long > > term. > > > > The lookup table is clever, but is frankly, voodoo :) I don't like > > trying to follow the logic of what maps to what and be concerned > > about what flags are where just for the sake of this, and it > > makes things ugly to read. > > > > I think I would rather add my own char to the namei structure, > > and set it appropriately in the same places that pledge does. IMO > > this makes looking at the source code for system calls much clearer > > int the kernel - rather than trying to fathom in your head how a > > combination of pledge flags will turn into unveil. > > it seems to me it is a bit duplicating annotations: annotating syscalls > for pledge, and annotating syscalls for unveil, while pledge has just > more granuality. but I can understand the problem with an additional > level for translating pledge-flags to unveil-flags. > > in consequence, you will have to change some bits in man page, as "r" > will not be anymore "corresponding to the pledge(2) promise rpath" :) > > > So this is a somewhat "minimal" diff tha puts the flags in > > namei.h, and checks them as per your change, but rather > > than using a lookup table just expressly sets them > > for each system call appropriately.. it passes regress > > as is. > > > > I think after doing this I can probably go in an get rid of > > the awkward PLEDGE_STAT and simplify BYPASS considerably > > as well, but I will do that separately. > > > > ok? > > some comments inlined. > > and one important for starting: you should consider (ni_unveil==0) as > deny by default, instead of allow by default. eventually having an > panic(9) or an assert(9) (but it should be too early for now in case we > want it). > > it is important because else if we miss an explicit initialisation (the > struct is zeroed, ni_unveil=0 by default), it will be an allowed > operation by default and we will *not* see the problem. with deny or a > panic(9) we will not miss it: someone will complain. > > for a panic(9), it could be done in namei(), eventually just after > pledge_namei() call, to avoid checking the variable too often. > > > another important thing I just realized: not all filesystem operations > were pledeable, and some haven't ni_pledge because the function was > unreachable while pledged (but pledge_namei() has a panic for such > cases). > > Some examples that will need consideration for unveil(2): > - mount(2) > - unmount(2) > - quotactl(2) > - chroot(2) > - getfh(2) > - acct(2) > - coredump() > - loadfirmware() - I think ifconfig(1) could make the kernel loading a > firmware for some network card > > so having ni_unveil separated from ni_pledge could be good to be able to > manage these namei() calls in unveiled programs. > > > > Index: kern/kern_unveil.c > > === > > RCS file: /cvs/src/sys/kern/kern_unveil.c,v > > retrieving revision 1.9 > > diff -u -p -u -p -r1.9 kern_unveil.c > > --- kern/kern_unveil.c 30 Jul 2018 15:16:27 - 1.9 > > +++ kern/kern_unveil.c 4 Aug 2018 16:13:07 - > > @@ -40,6 +40,11 @@ > > #define UNVEIL_MAX_VNODES 128 > > #define UNVEIL_MAX_NAMES 128 > > > > +#defineUNVEIL_READ 0x01 > > +#defineUNVEIL_WRITE0x02 > > +#defineUNVEIL_CREATE 0x04 > > +#defineUNVEIL_EXEC 0x08 > > + > > the flags are duplicated with namei.h. I think namei.h is the right > place, and there could be removed from here. &
Re: unveil: incomplete unveil_flagmatch semantic
> > + nd.ni_unveil = 0; /* XXX No flags == allow it */ > > see my comment about ni_unveil != 0. > > as you still have check on (ni_pledge & PLEDGE_STAT), it should be still > ok. > It doesn't actually do this yt.. this comment was a reminder for me and should have had allow it? for my dealig with PLEDGE_STAT afterwards I'm intend on making another flag for that the "you have to be able to access it" a-la PLEDGE_STAT which was a hack - and clean that up in a separate diff. so no, 0 flags won't be "allow it"
Re: unveil: incomplete unveil_flagmatch semantic
On Fri, Aug 03, 2018 at 06:31:00AM +0200, Sebastien Marie wrote:
> On Thu, Aug 02, 2018 at 03:42:03PM +0200, Sebastien Marie wrote:
> > On Mon, Jul 30, 2018 at 07:55:35AM -0600, Bob Beck wrote:
> > > yeah the latter will be the way to go
> > >
> >
> > new diff with direct lookup using an indirection table.
> >
>
> new (emergency) version with PLEDGE_CHOWN consideration for unveil(2).
>
> sorry for having missed it.
>
All good because you gave me inspiration, after I ran your diff.
I tied unveil to the pledge flags when I first did it because it was
convenient - I think this thig with chmod (and the awkwardness of
PLEDGE_STAT, etc. etc.) just shows that this was a decision of
convienience in the short term that is going to bite us in the long
term.
The lookup table is clever, but is frankly, voodoo :) I don't like
trying to follow the logic of what maps to what and be concerned
about what flags are where just for the sake of this, and it
makes things ugly to read.
I think I would rather add my own char to the namei structure,
and set it appropriately in the same places that pledge does. IMO
this makes looking at the source code for system calls much clearer
int the kernel - rather than trying to fathom in your head how a
combination of pledge flags will turn into unveil.
So this is a somewhat "minimal" diff tha puts the flags in
namei.h, and checks them as per your change, but rather
than using a lookup table just expressly sets them
for each system call appropriately.. it passes regress
as is.
I think after doing this I can probably go in an get rid of
the awkward PLEDGE_STAT and simplify BYPASS considerably
as well, but I will do that separately.
ok?
Index: dev/diskmap.c
===
RCS file: /cvs/src/sys/dev/diskmap.c,v
retrieving revision 1.22
diff -u -p -u -p -r1.22 diskmap.c
--- dev/diskmap.c 4 Jul 2018 12:42:30 - 1.22
+++ dev/diskmap.c 3 Aug 2018 02:38:26 -
@@ -85,6 +85,7 @@ diskmapioctl(dev_t dev, u_long cmd, cadd
NDINIT(&ndp, 0, 0, UIO_SYSSPACE, devname, p);
ndp.ni_pledge = PLEDGE_RPATH;
+ ndp.ni_unveil = UNVEIL_READ;
if ((error = vn_open(&ndp, fp0->f_flag, 0)) != 0)
goto invalid;
Index: kern/exec_elf.c
===
RCS file: /cvs/src/sys/kern/exec_elf.c,v
retrieving revision 1.145
diff -u -p -u -p -r1.145 exec_elf.c
--- kern/exec_elf.c 20 Jul 2018 21:57:26 - 1.145
+++ kern/exec_elf.c 3 Aug 2018 02:38:26 -
@@ -332,6 +332,7 @@ elf_load_file(struct proc *p, char *path
NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF, UIO_SYSSPACE, path, p);
nd.ni_pledge = PLEDGE_RPATH;
+ nd.ni_unveil = UNVEIL_READ;
if ((error = namei(&nd)) != 0) {
return (error);
}
Index: kern/kern_exec.c
===
RCS file: /cvs/src/sys/kern/kern_exec.c,v
retrieving revision 1.200
diff -u -p -u -p -r1.200 kern_exec.c
--- kern/kern_exec.c20 Jul 2018 21:57:26 - 1.200
+++ kern/kern_exec.c3 Aug 2018 02:38:26 -
@@ -275,6 +275,7 @@ sys_execve(struct proc *p, void *v, regi
NDINIT(&nid, LOOKUP, NOFOLLOW, UIO_USERSPACE, SCARG(uap, path), p);
nid.ni_pledge = PLEDGE_EXEC;
+ nid.ni_unveil = UNVEIL_EXEC;
/*
* initialize the fields of the exec package.
Index: kern/kern_ktrace.c
===
RCS file: /cvs/src/sys/kern/kern_ktrace.c,v
retrieving revision 1.98
diff -u -p -u -p -r1.98 kern_ktrace.c
--- kern/kern_ktrace.c 20 Jun 2018 10:48:55 - 1.98
+++ kern/kern_ktrace.c 3 Aug 2018 02:38:26 -
@@ -513,6 +513,7 @@ sys_ktrace(struct proc *p, void *v, regi
cred = p->p_ucred;
NDINIT(&nd, LOOKUP, FOLLOW, UIO_USERSPACE, fname, p);
nd.ni_pledge = PLEDGE_CPATH | PLEDGE_WPATH;
+ nd.ni_unveil = UNVEIL_CREATE | UNVEIL_WRITE;
if ((error = vn_open(&nd, FWRITE|O_NOFOLLOW, 0)) != 0)
return error;
vp = nd.ni_vp;
Index: kern/kern_unveil.c
===
RCS file: /cvs/src/sys/kern/kern_unveil.c,v
retrieving revision 1.9
diff -u -p -u -p -r1.9 kern_unveil.c
--- kern/kern_unveil.c 30 Jul 2018 15:16:27 - 1.9
+++ kern/kern_unveil.c 4 Aug 2018 16:13:07 -
@@ -40,6 +40,11 @@
#define UNVEIL_MAX_VNODES 128
#define UNVEIL_MAX_NAMES 128
+#defineUNVEIL_READ 0x01
+#defineUNVEIL_WRITE0x02
+#defineUNVEIL_CREATE 0x04
+#defineUNVEIL_EXEC 0x08
+
static inline int
unvname_compare(const struct unvname *n1, const struct unvname *n2)
{
@@ -50,7 +55,7 @@ unvname_compa
Re: unveil: incomplete unveil_flagmatch semantic
yeah the latter will be the way to go On Mon, Jul 30, 2018 at 06:02 Sebastien Marie wrote: > Hi, > > I think unveil_flagmatch() isn't complete and/or has not the right > semantic. > > A bit of internals for starting (I will speak about ni_pledge, people > that know what it is and how it works with pledge/unveil could go to > "what is the problem" part). > > unveil(2) works with the syscall annotation introduced for pledge(2). > > When kernel needs to resolv a name to vnode, it used namei() function. > For that it initializes a special structure used as namei() argument: > `struct nameidata`. This struct has a field `ni_pledge` used to let vfs > subsystem know what kind of syscalls called it. This way, underline > subsystem doesn't have to know all syscalls, and could work on them by > "group". > > For example, when you call open(2), depending the flags argument used, > ni_pledge will contain PLEDGE_RPATH, PLEDGE_WPATH, or PLEDGE_CPATH. The > subsystem has a quick and accurate view of the intented usage of the > vnode. > > pledge(2) uses it to check that you have the promise of intent to use, > and unveil(2) uses it too in a similar way, in particular in > unveil_flagmatch() to check if the process has the accurate permission > for this particular vnode. > > > > Now, what is the problem with unveil_flagmatch() ? > > If I exclude the PLEDGE_STAT stuff from the equation (I am not still > convinced it is the right way to do it - but it isn't the question for > now), unveil_flagmatch() has a default policy to allow the operation, > and only check for some flags in ni_pledge to deny it. > > For simple syscall like open(2) there is no problem. The possible > value of ni_pledge are composed with only PLEDGE_RPATH, PLEDGE_WPATH, or > PLEDGE_CPATH. > > But for some others syscalls, it isn't the case. > > For example, chmod(2) will set ni_pledge to "PLEDGE_FATTR | > PLEDGE_RPATH". For pledge(2), it means you must have "fattr" (capability > to change attribute on the node) and "rpath" (capability to read the > node) promise to use such syscall. > > As unveil(2) doesn't have the "fattr" concept, and unveil_flagmatch() > will check only for PLEDGE_RPATH, PLEDGE_WPATH, PLEDGE_CPATH and > PLEDGE_EXEC, we end with unsound policy: you could use chmod(2) with just > "r" on unveiled part of the filesystem. > > Some others flags could occurs in ni_pledge: > - PLEDGE_CHOWN: chown(2) family > - PLEDGE_DPATH: mknod(2), mkfifo(2) > - PLEDGE_FATTR: utimes(2) family, chmod(2) family, truncate(2), chflags(2) > - PLEDGE_TTY: revoke(2) > - PLEDGE_UNIX: bind(2), connect(2) > > unveil_flagmatch() has a special case for "" flag: it means deny. > but as soon as you have "r", "w", "x" or "c", you have an allow policy > by default. Check will be done only for a part of ni_pledge. > > > I see basically two possibilities: > - extending unveil(2) permissions to match pledge(2) flags - but I don't > like it, unveil(2) should be kept simple. > > - having a separate namespace for unveil and pledge flags (to avoid > confusion), and translating all pledge flags to unveil flags. > > PLEDGE_RPATH => UNVEIL_READ > PLEDGE_WPATH => UNVEIL_WRITE > PLEDGE_CPATH => UNVEIL_CREATE > PLEDGE_EXEC => UNVEIL_EXEC > PLEDGE_CHOWN => UNVEIL_WRITE > PLEDGE_DPATH => UNVEIL_CREATE > PLEDGE_FATTR => UNVEIL_WRITE > PLEDGE_TTY => UNVEIL_WRITE > PLEDGE_UNIX => UNVEIL_READ|UNVEIL_WRITE (requiring both) > any others => panic(9) > > This way, we could be really exhaustive in unveil_flagmatch() without > having to bother for future PLEDGE flag addition (as we will panic(9) if > some developer doesn't add it where intented). > > Thanks. > -- > Sebastien Marie >
Re: unveil: incorrect type flags on unvname_new()
ok beck@
On Mon, Jul 16, 2018 at 15:53 Sebastien Marie wrote:
> Hi,
>
> While reviewing unveil(2) code, I found an incorrect type on
> unvname_new() function: flags argument should be uint64_t.
>
> It is called by unveil_add_name() which uses uint64_t for flags, and
> store the value in struct unvname un_flags member which is uint64_t.
>
> Thanks.
> --
> Sebastien Marie
>
>
> Index: kern/kern_unveil.c
> ===
> RCS file: /cvs/src/sys/kern/kern_unveil.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 kern_unveil.c
> --- kern/kern_unveil.c 13 Jul 2018 13:47:41 - 1.2
> +++ kern/kern_unveil.c 16 Jul 2018 13:08:40 -
> @@ -50,7 +50,7 @@ unvname_compare(const struct unvname *n1
> }
>
> struct unvname *
> -unvname_new(const char *name, size_t size, int flags)
> +unvname_new(const char *name, size_t size, uint64_t flags)
> {
> struct unvname *ret = malloc(sizeof(struct unvname), M_PROC,
> M_WAITOK);
> ret->un_name = malloc(size, M_PROC, M_WAITOK);
>
Re: const qualifiers for EVP_*
ok
On Sat, May 12, 2018 at 13:14 Theo Buehler wrote:
> Here's another straightforward batch. As usual, it's been tested in a
> bulk by sthen and there was no fallout.
>
> Index: lib/libcrypto/asn1/ameth_lib.c
> ===
> RCS file: /var/cvs/src/lib/libcrypto/asn1/ameth_lib.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 ameth_lib.c
> --- lib/libcrypto/asn1/ameth_lib.c 21 Jan 2017 04:31:25 -
> 1.16
> +++ lib/libcrypto/asn1/ameth_lib.c 12 May 2018 18:42:51 -
> @@ -299,7 +299,7 @@ EVP_PKEY_asn1_get0_info(int *ppkey_id, i
> }
>
> const EVP_PKEY_ASN1_METHOD*
> -EVP_PKEY_get0_asn1(EVP_PKEY *pkey)
> +EVP_PKEY_get0_asn1(const EVP_PKEY *pkey)
> {
> return pkey->ameth;
> }
> Index: lib/libcrypto/evp/evp.h
> ===
> RCS file: /var/cvs/src/lib/libcrypto/evp/evp.h,v
> retrieving revision 1.59
> diff -u -p -r1.59 evp.h
> --- lib/libcrypto/evp/evp.h 2 May 2018 15:51:41 - 1.59
> +++ lib/libcrypto/evp/evp.h 12 May 2018 18:42:51 -
> @@ -617,7 +617,8 @@ int EVP_DigestSignFinal(EVP_MD_CTX *ctx,
>
> int EVP_DigestVerifyInit(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx,
> const EVP_MD *type, ENGINE *e, EVP_PKEY *pkey);
> -int EVP_DigestVerifyFinal(EVP_MD_CTX *ctx, unsigned char *sig, size_t
> siglen);
> +int EVP_DigestVerifyFinal(EVP_MD_CTX *ctx, const unsigned char *sig,
> +size_t siglen);
>
> int EVP_OpenInit(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *type,
> const unsigned char *ek, int ekl, const unsigned char *iv, EVP_PKEY
> *priv);
> @@ -866,12 +867,12 @@ int EVP_PKEY_encrypt_old(unsigned char *
> int EVP_PKEY_type(int type);
> int EVP_PKEY_id(const EVP_PKEY *pkey);
> int EVP_PKEY_base_id(const EVP_PKEY *pkey);
> -int EVP_PKEY_bits(EVP_PKEY *pkey);
> +int EVP_PKEY_bits(const EVP_PKEY *pkey);
> int EVP_PKEY_size(EVP_PKEY *pkey);
> int EVP_PKEY_set_type(EVP_PKEY *pkey, int type);
> int EVP_PKEY_set_type_str(EVP_PKEY *pkey, const char *str, int len);
> int EVP_PKEY_assign(EVP_PKEY *pkey, int type, void *key);
> -void *EVP_PKEY_get0(EVP_PKEY *pkey);
> +void *EVP_PKEY_get0(const EVP_PKEY *pkey);
>
> #ifndef OPENSSL_NO_RSA
> struct rsa_st;
> @@ -995,7 +996,7 @@ int EVP_PKEY_asn1_get0_info(int *ppkey_i
> const char **pinfo, const char **ppem_str,
> const EVP_PKEY_ASN1_METHOD *ameth);
>
> -const EVP_PKEY_ASN1_METHOD* EVP_PKEY_get0_asn1(EVP_PKEY *pkey);
> +const EVP_PKEY_ASN1_METHOD* EVP_PKEY_get0_asn1(const EVP_PKEY *pkey);
> EVP_PKEY_ASN1_METHOD* EVP_PKEY_asn1_new(int id, int flags, const char
> *pem_str,
> const char *info);
> void EVP_PKEY_asn1_copy(EVP_PKEY_ASN1_METHOD *dst,
> Index: lib/libcrypto/evp/evp_pkey.c
> ===
> RCS file: /var/cvs/src/lib/libcrypto/evp/evp_pkey.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 evp_pkey.c
> --- lib/libcrypto/evp/evp_pkey.c29 Jan 2017 17:49:23 -
> 1.19
> +++ lib/libcrypto/evp/evp_pkey.c12 May 2018 18:42:51 -
> @@ -181,7 +181,8 @@ EVP_PKEY_get_attr_by_NID(const EVP_PKEY
> }
>
> int
> -EVP_PKEY_get_attr_by_OBJ(const EVP_PKEY *key, ASN1_OBJECT *obj, int
> lastpos)
> +EVP_PKEY_get_attr_by_OBJ(const EVP_PKEY *key, const ASN1_OBJECT *obj,
> +int lastpos)
> {
> return X509at_get_attr_by_OBJ(key->attributes, obj, lastpos);
> }
> Index: lib/libcrypto/evp/m_sigver.c
> ===
> RCS file: /var/cvs/src/lib/libcrypto/evp/m_sigver.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 m_sigver.c
> --- lib/libcrypto/evp/m_sigver.c29 Jan 2017 17:49:23 - 1.6
> +++ lib/libcrypto/evp/m_sigver.c12 May 2018 18:42:51 -
> @@ -166,7 +166,7 @@ EVP_DigestSignFinal(EVP_MD_CTX *ctx, uns
> }
>
> int
> -EVP_DigestVerifyFinal(EVP_MD_CTX *ctx, unsigned char *sig, size_t siglen)
> +EVP_DigestVerifyFinal(EVP_MD_CTX *ctx, const unsigned char *sig, size_t
> siglen)
> {
> EVP_MD_CTX tmp_ctx;
> unsigned char md[EVP_MAX_MD_SIZE];
> Index: lib/libcrypto/evp/p_lib.c
> ===
> RCS file: /var/cvs/src/lib/libcrypto/evp/p_lib.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 p_lib.c
> --- lib/libcrypto/evp/p_lib.c 14 Apr 2018 07:09:21 - 1.21
> +++ lib/libcrypto/evp/p_lib.c 12 May 2018 18:42:51 -
> @@ -85,7 +85,7 @@
> static void EVP_PKEY_free_it(EVP_PKEY *x);
>
> int
> -EVP_PKEY_bits(EVP_PKEY *pkey)
> +EVP_PKEY_bits(const EVP_PKEY *pkey)
> {
> if (pkey && pkey->ameth && pkey->ameth->pkey_bits)
> return pkey->ameth->pkey_bits(pkey);
> @@ -277,7 +277,7 @@ EVP_PKEY_assign(EVP_PKEY *pkey, int type
> }
>
> void *
> -EVP_PKEY_get0(EVP_PKEY *pkey)
> +EVP_PKEY_get0(const EVP_PKEY *pkey)
> {
> return pkey->pkey.ptr;
> }
> Index: lib/libcrypto/x509/x509.h
> ===
Re: Anyone can suggest a BitCoin processor to the OpenBSD Foundation? BitPay has become terrible
So, related to this topic, Apparently BitPay has now fixed us up again. I have put the button back on the web site, if anyone wants to try a bitcoin donation is is supposed to be possible again
Anyone can suggest a BitCoin processor to the OpenBSD Foundation? BitPay has become terrible
So, as some of you may know, the OpenBSD Foundation has accepted BitCoin donations for some time via BitPay.com BitPay was convenient for us since they will sell the BTC donations immediately, and convert to Canadian Dollars. We then periodically get bank transfers of the balance, and this works great. Obviously, for money laundering rules, we do need to provide them with some documentation to prove the foundation's legitimacy, which we have repatedly, and this used to be not a problem. Lately they seem to have decided in spite of being in posession of 1) A copy of two of our personal passports 2) A copy of the Canadian Federal Incorporation documents for a not for profit 3) A copy of the Electric Bill for the address of record 4) A copy of an invoice sent to the foundation at the address of record 5) A copy of a bank statement from the foundation That they seem to be suspending our access to BitPay - this in spite of this being the only documents they request from us and then they seem to say they won't work - because apparently they don't understand Canada our countries outside of the USA. So in short, bitPay has always been a bit of a pain for us, but now seems to be unable to be used by us. So, what I'm looking for is opinions on an online processor. Yes I am aware they charge fees, I am looking for convenience, basically, the ability to put a button on the foundation donation page that links to them. which will take a donation in BTC, convert it for us, and get it to us at a Canadian bank. What are all you people using? For a number of reasons, we do *not* want to hold a bitCoin wallet ourselves, and be in the business of selling bitcoins, so please don't suggest this, and No, we can't have a bank account outside of Canada, so please don't suggest that either :)
Re: libressl: crash in DES_fcrypt
why AA? why not just choose two random ascii salt chars at that point? or
since this is effectively a failure case encrypt a random ascii salt and
random string?
using AA will produce a usable result based on the original string.
encrypting a random string with a random salt means the failure return
wont be accidentally used
On Wed, Dec 13, 2017 at 06:51 Theo de Raadt wrote:
> I still don't understand.
>
> This feels like fail-open.
>
> > I would like to commit this fix. I tried to avoid the crash in
> > libcrypto with least possible impact for the DES_fcrypt() API.
> >
> > ok?
> >
> > bluhm
> >
> > On Tue, Dec 05, 2017 at 05:20:49PM +0100, Alexander Bluhm wrote:
> > > On Fri, Oct 27, 2017 at 01:50:26AM +0200, Jan Engelhardt wrote:
> > > > #include
> > > > int main(void) {
> > > > char salt[3] = {0xf8, 0xd0, 0x00};
> > > > char out[32];
> > > > DES_fcrypt("foo", salt, out);
> > > > }
> > >
> > > This program produces a Segmentation fault in OpenBSD current.
> > >
> > > > openssl 1.1.x has it fixed (but 1.0.2l does not!) - their commit
> > > > seems to be 6493e4801e9edbe1ad1e256d4ce9cd55c8aa2242 in
> > > > https://github.com/openssl/openssl .
> > >
> > > Their fix changes the semantics of DES_crypt() and DES_fcrypt().
> > > They may fail and return NULL now. This was never the case before
> > > so we may expect that programs do not check it. With DES_fcrypt()
> > > it is very likely that some uninitilaized content of the return
> > > string is used.
> > >
> > > So I have extended the workaround that was there already. Read the
> > > comment above the fix. If the salt does not consist of two ascii
> > > characters, replace it with "AA". This gives a safe result in all
> > > cases.
> > >
> > > ok?
> > >
> > > bluhm
> > >
> > > Index: lib/libcrypto/des/fcrypt.c
> > > ===
> > > RCS file: /data/mirror/openbsd/cvs/src/lib/libcrypto/des/fcrypt.c,v
> > > retrieving revision 1.12
> > > diff -u -p -r1.12 fcrypt.c
> > > --- lib/libcrypto/des/fcrypt.c 26 Dec 2016 21:30:10 -
> 1.12
> > > +++ lib/libcrypto/des/fcrypt.c 5 Dec 2017 16:03:57 -
> > > @@ -78,9 +78,15 @@ char *DES_fcrypt(const char *buf, const
> > > * crypt to "*". This was found when replacing the crypt in
> > > * our shared libraries. People found that the disabled
> > > * accounts effectively had no passwd :-(. */
> > > - x=ret[0]=((salt[0] == '\0')?'A':salt[0]);
> > > + x = salt[0];
> > > + if (x == 0 || x >= sizeof(con_salt))
> > > + x = 'A';
> > > + ret[0] = x;
> > > Eswap0=con_salt[x]<<2;
> > > - x=ret[1]=((salt[1] == '\0')?'A':salt[1]);
> > > + x = (salt[0] == '\0') ? 'A' : salt[1];
> > > + if (x == 0 || x >= sizeof(con_salt))
> > > + x = 'A';
> > > + ret[1] = x;
> > > Eswap1=con_salt[x]<<6;
> > > /* EAY
> > > r=strlen(buf);
> > > Index: regress/lib/libcrypto/des/destest.c
> > > ===
> > > RCS file:
> /data/mirror/openbsd/cvs/src/regress/lib/libcrypto/des/destest.c,v
> > > retrieving revision 1.3
> > > diff -u -p -r1.3 destest.c
> > > --- regress/lib/libcrypto/des/destest.c 30 Oct 2015 15:58:40
> - 1.3
> > > +++ regress/lib/libcrypto/des/destest.c 5 Dec 2017 16:02:18 -
> > > @@ -749,18 +749,38 @@ plain[8+4], plain[8+5], plain[8+6], plai
> > > }
> > > printf("\n");
> > > printf("fast crypt test ");
> > > - str=crypt("testing","ef");
> > > + str=DES_crypt("testing","ef");
> > > if (strcmp("efGnQx2725bI2",str) != 0)
> > > {
> > > printf("fast crypt error, %s should be
> efGnQx2725bI2\n",str);
> > > err=1;
> > > }
> > > - str=crypt("bca76;23","yA");
> > > + str=DES_crypt("bca76;23","yA");
> > > if (strcmp("yA1Rp/1hZXIJk",str) != 0)
> > > {
> > > printf("fast crypt error, %s should be
> yA1Rp/1hZXIJk\n",str);
> > > err=1;
> > > }
> > > + str = DES_crypt("testing", "\202B");
> > > + if (strncmp("AB",str,2) != 0) {
> > > + printf("salt %s first non ascii not replaced with A\n",
> str);
> > > + err = 1;
> > > + }
> > > + str = DES_crypt("testing", "B\202");
> > > + if (strncmp("BA",str,2) != 0) {
> > > + printf("salt %s second non ascii not replaced with A\n",
> str);
> > > + err = 1;
> > > + }
> > > + str = DES_crypt("testing", "\0B");
> > > + if (strncmp("AA",str,2) != 0) {
> > > + printf("salt %s first NUL not replaced with AA\n", str);
> > > + err = 1;
> > > + }
> > > + str = DES_crypt("testing", "B");
> > > + if (strncmp("BA",str,2) != 0) {
> > > + printf("salt %s second NUL not replaced with A\n", str);
> > > + err = 1;
> > > + }
> > > printf("\n");
> > > return(err);
> > > }
> >
>
>
Re: iked, don't return NULL in print_host
ok beck@
On Wed, Nov 29, 2017 at 02:17:21AM +0100, Claudio Jeker wrote:
> On Wed, Nov 29, 2017 at 01:59:06AM +0100, Claudio Jeker wrote:
> > Seen in my log file:
> > Nov 28 17:47:22 dramaqueen iked: vfprintf %s NULL in "%s: %s %s from %s to
> > %s ms gid %u, %ld bytes%s"
> >
> > and
> >
> > Nov 29 01:02:39 dramaqueen iked[49967]: ikev2_msg_send: IKE_SA_INIT
> > request from (null) to 62.48.30.5:500 msgid 0, 438 bytes
> >
> > The problem seems to be in print_host so try to not return NULL in there.
> > Maybe we could return something else but this is a start IMO.
>
> beck@ prefers to just print unknown instead of the gai_strerror -- guess
> that is more sensible.
>
> --
> :wq Claudio
>
> Index: util.c
> ===
> RCS file: /cvs/src/sbin/iked/util.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 util.c
> --- util.c9 Jan 2017 14:49:21 - 1.33
> +++ util.c29 Nov 2017 01:16:21 -
> @@ -654,8 +654,8 @@ print_host(struct sockaddr *sa, char *bu
>
> if (getnameinfo(sa, sa->sa_len,
> buf, len, NULL, 0, NI_NUMERICHOST) != 0) {
> - buf[0] = '\0';
> - return (NULL);
> + strlcpy(buf, "unknown", len);
> + return (buf);
> }
>
> if ((port = socket_getport(sa)) != 0) {
>
Official OpenBSD 6.2 CD set up for auction on Ebay
So, the only 6.2 set to be produced is up for auction, featuring hand-drawn artwork by Theo. Artisanally Made in Canada! All proceeds of the sale to fund OpenBSD development. Go have a look at http://www.ebay.ca/itm/Official-OpenBSD-6-2-CD-Set/253265944606
Re: [patch] ocspcheck: nextUpdate is optional according to RFC 6960
effectivelyu providing a limitless OCSP staple is kind of stupid - you may
as well simply *not staple*
On Wed, Sep 6, 2017 at 8:23 AM, Bob Beck wrote:
> I'm not super inclined to make this "flexible" unless we see this used int
> the wild, which I have not. We are more restrictive than
> OpenSSL in many areas.
>
> On Wed, Sep 6, 2017 at 1:31 AM, Andreas Bartelt wrote:
>
>> On 09/06/17 04:40, Bob Beck wrote:
>>
>>> Andreas where are you seeing this as being a real issue - who is shipping
>>> out OCSP responses without a next update field?
>>>
>>>
>> I've noticed this while playing with a local CA and a corresponding OCSP
>> responder on my LAN. For openssl ocsp, the -nmin or -ndays argument is
>> optional. If these arguments are not explicitly provided, the next update
>> field will not be set.
>>
>>
>>
>>>
>>> On Sat, Sep 2, 2017 at 11:28 AM, Andreas Bartelt
>>> wrote:
>>>
>>> ocspcheck effectively treats a missing nextUpdate like an error, i.e., it
>>>> always provides a warning and no staplefile is written out. According to
>>>> RFC 6960, the nextUpdate field is optional. The following patch should
>>>> handle this case more gracefully and include a suitable debug message
>>>> only
>>>> in case -vv is specified.
>>>>
>>>> OK?
>>>>
>>>> Index: src/usr.sbin/ocspcheck/ocspcheck.c
>>>> ===
>>>> RCS file: /cvs/src/usr.sbin/ocspcheck/ocspcheck.c,v
>>>> retrieving revision 1.21
>>>> diff -u -p -u -r1.21 ocspcheck.c
>>>> --- src/usr.sbin/ocspcheck/ocspcheck.c 8 May 2017 20:15:34 -
>>>> 1.21
>>>> +++ src/usr.sbin/ocspcheck/ocspcheck.c 2 Sep 2017 17:09:00 -
>>>> @@ -368,7 +368,7 @@ validate_response(char *buf, size_t size
>>>> {
>>>> ASN1_GENERALIZEDTIME *revtime = NULL, *thisupd = NULL,
>>>> *nextupd =
>>>> NULL;
>>>> const unsigned char **p = (const unsigned char **)&buf;
>>>> - int status, cert_status=0, crl_reason=0;
>>>> + int status, cert_status=0, crl_reason=0, next_update=0;
>>>> time_t now, rev_t = -1, this_t, next_t;
>>>> OCSP_RESPONSE *resp;
>>>> OCSP_BASICRESP *bresp;
>>>> @@ -447,12 +447,14 @@ validate_response(char *buf, size_t size
>>>> return 0;
>>>> }
>>>> if ((next_t = parse_ocsp_time(nextupd)) == -1) {
>>>> - warnx("unable to parse next update time in OCSP reply");
>>>> - return 0;
>>>> + if (verbose >= 2)
>>>> + fprintf(stderr, "Optional timestamp for next
>>>> update not included in OCSP reply\n");
>>>> }
>>>> + else
>>>> + next_update = 1;
>>>>
>>>> /* Don't allow this update to precede next update */
>>>> - if (this_t >= next_t) {
>>>> + if (next_update == 1 && this_t >= next_t) {
>>>> warnx("Invalid OCSP reply: this update >= next
>>>> update");
>>>> return 0;
>>>> }
>>>> @@ -481,7 +483,7 @@ validate_response(char *buf, size_t size
>>>> /*
>>>> * Check that next update is still valid
>>>> */
>>>> - if (next_t < now - JITTER_SEC) {
>>>> + if (next_update == 1 && next_t < now - JITTER_SEC) {
>>>> warnx("Invalid OCSP reply: reply has expired (%s)",
>>>> ctime(&next_t));
>>>> return 0;
>>>> @@ -489,7 +491,8 @@ validate_response(char *buf, size_t size
>>>>
>>>> vspew("OCSP response validated from %s\n", host);
>>>> vspew("This Update: %s", ctime(&this_t));
>>>> - vspew("Next Update: %s", ctime(&next_t));
>>>> + if (next_update == 1)
>>>> + vspew("Next Update: %s", ctime(&next_t));
>>>> return 1;
>>>> }
>>>>
>>>>
>>>>
>>>
>>
>
Re: [patch] ocspcheck: nextUpdate is optional according to RFC 6960
I'm not super inclined to make this "flexible" unless we see this used int
the wild, which I have not. We are more restrictive than
OpenSSL in many areas.
On Wed, Sep 6, 2017 at 1:31 AM, Andreas Bartelt wrote:
> On 09/06/17 04:40, Bob Beck wrote:
>
>> Andreas where are you seeing this as being a real issue - who is shipping
>> out OCSP responses without a next update field?
>>
>>
> I've noticed this while playing with a local CA and a corresponding OCSP
> responder on my LAN. For openssl ocsp, the -nmin or -ndays argument is
> optional. If these arguments are not explicitly provided, the next update
> field will not be set.
>
>
>
>>
>> On Sat, Sep 2, 2017 at 11:28 AM, Andreas Bartelt wrote:
>>
>> ocspcheck effectively treats a missing nextUpdate like an error, i.e., it
>>> always provides a warning and no staplefile is written out. According to
>>> RFC 6960, the nextUpdate field is optional. The following patch should
>>> handle this case more gracefully and include a suitable debug message
>>> only
>>> in case -vv is specified.
>>>
>>> OK?
>>>
>>> Index: src/usr.sbin/ocspcheck/ocspcheck.c
>>> ===
>>> RCS file: /cvs/src/usr.sbin/ocspcheck/ocspcheck.c,v
>>> retrieving revision 1.21
>>> diff -u -p -u -r1.21 ocspcheck.c
>>> --- src/usr.sbin/ocspcheck/ocspcheck.c 8 May 2017 20:15:34 -
>>> 1.21
>>> +++ src/usr.sbin/ocspcheck/ocspcheck.c 2 Sep 2017 17:09:00 -
>>> @@ -368,7 +368,7 @@ validate_response(char *buf, size_t size
>>> {
>>> ASN1_GENERALIZEDTIME *revtime = NULL, *thisupd = NULL, *nextupd
>>> =
>>> NULL;
>>> const unsigned char **p = (const unsigned char **)&buf;
>>> - int status, cert_status=0, crl_reason=0;
>>> + int status, cert_status=0, crl_reason=0, next_update=0;
>>> time_t now, rev_t = -1, this_t, next_t;
>>> OCSP_RESPONSE *resp;
>>> OCSP_BASICRESP *bresp;
>>> @@ -447,12 +447,14 @@ validate_response(char *buf, size_t size
>>> return 0;
>>> }
>>> if ((next_t = parse_ocsp_time(nextupd)) == -1) {
>>> - warnx("unable to parse next update time in OCSP reply");
>>> - return 0;
>>> + if (verbose >= 2)
>>> + fprintf(stderr, "Optional timestamp for next
>>> update not included in OCSP reply\n");
>>> }
>>> + else
>>> + next_update = 1;
>>>
>>> /* Don't allow this update to precede next update */
>>> - if (this_t >= next_t) {
>>> + if (next_update == 1 && this_t >= next_t) {
>>> warnx("Invalid OCSP reply: this update >= next update");
>>> return 0;
>>> }
>>> @@ -481,7 +483,7 @@ validate_response(char *buf, size_t size
>>> /*
>>> * Check that next update is still valid
>>> */
>>> - if (next_t < now - JITTER_SEC) {
>>> + if (next_update == 1 && next_t < now - JITTER_SEC) {
>>> warnx("Invalid OCSP reply: reply has expired (%s)",
>>> ctime(&next_t));
>>> return 0;
>>> @@ -489,7 +491,8 @@ validate_response(char *buf, size_t size
>>>
>>> vspew("OCSP response validated from %s\n", host);
>>> vspew("This Update: %s", ctime(&this_t));
>>> - vspew("Next Update: %s", ctime(&next_t));
>>> + if (next_update == 1)
>>> + vspew("Next Update: %s", ctime(&next_t));
>>> return 1;
>>> }
>>>
>>>
>>>
>>
>
Re: [patch] ocspcheck: nextUpdate is optional according to RFC 6960
Andreas where are you seeing this as being a real issue - who is shipping
out OCSP responses without a next update field?
On Sat, Sep 2, 2017 at 11:28 AM, Andreas Bartelt wrote:
> ocspcheck effectively treats a missing nextUpdate like an error, i.e., it
> always provides a warning and no staplefile is written out. According to
> RFC 6960, the nextUpdate field is optional. The following patch should
> handle this case more gracefully and include a suitable debug message only
> in case -vv is specified.
>
> OK?
>
> Index: src/usr.sbin/ocspcheck/ocspcheck.c
> ===
> RCS file: /cvs/src/usr.sbin/ocspcheck/ocspcheck.c,v
> retrieving revision 1.21
> diff -u -p -u -r1.21 ocspcheck.c
> --- src/usr.sbin/ocspcheck/ocspcheck.c 8 May 2017 20:15:34 -
> 1.21
> +++ src/usr.sbin/ocspcheck/ocspcheck.c 2 Sep 2017 17:09:00 -
> @@ -368,7 +368,7 @@ validate_response(char *buf, size_t size
> {
> ASN1_GENERALIZEDTIME *revtime = NULL, *thisupd = NULL, *nextupd =
> NULL;
> const unsigned char **p = (const unsigned char **)&buf;
> - int status, cert_status=0, crl_reason=0;
> + int status, cert_status=0, crl_reason=0, next_update=0;
> time_t now, rev_t = -1, this_t, next_t;
> OCSP_RESPONSE *resp;
> OCSP_BASICRESP *bresp;
> @@ -447,12 +447,14 @@ validate_response(char *buf, size_t size
> return 0;
> }
> if ((next_t = parse_ocsp_time(nextupd)) == -1) {
> - warnx("unable to parse next update time in OCSP reply");
> - return 0;
> + if (verbose >= 2)
> + fprintf(stderr, "Optional timestamp for next
> update not included in OCSP reply\n");
> }
> + else
> + next_update = 1;
>
> /* Don't allow this update to precede next update */
> - if (this_t >= next_t) {
> + if (next_update == 1 && this_t >= next_t) {
> warnx("Invalid OCSP reply: this update >= next update");
> return 0;
> }
> @@ -481,7 +483,7 @@ validate_response(char *buf, size_t size
> /*
> * Check that next update is still valid
> */
> - if (next_t < now - JITTER_SEC) {
> + if (next_update == 1 && next_t < now - JITTER_SEC) {
> warnx("Invalid OCSP reply: reply has expired (%s)",
> ctime(&next_t));
> return 0;
> @@ -489,7 +491,8 @@ validate_response(char *buf, size_t size
>
> vspew("OCSP response validated from %s\n", host);
> vspew("This Update: %s", ctime(&this_t));
> - vspew("Next Update: %s", ctime(&next_t));
> + if (next_update == 1)
> + vspew("Next Update: %s", ctime(&next_t));
> return 1;
> }
>
>
Re: [PATCH 0/2] SMALL_TIME_T follow-ups (was Re: [PATCH] allow notAfter after 2038 with 32-bit time_t)
> > With the new define (SMALL_TIME_T) enabled, a 32-bit time_t build > using "openssl s_client -connect" can successfully connect to a server > and verify its certificate chain when one or more notAfter dates after > 2038 are present. > > However, using "nc -c" fails to connect to the same server. > > The reason being that libtls also needs to clamp the notAfter date. I've just committed a change that enables libtls to use it. so nc and libtls should work on broken computers now. I still think karma for me doing this is I will be killed by an embedded 32 bit linux device in 2038... -Bob
Re: [PATCH] allow notAfter after 2038 with 32-bit time_t
https://github.com/openbsd/src/commit/b943944faeecf3a978bf3f57df1b35335ffecbec On Tue, Jul 11, 2017 at 4:23 AM, Stuart Henderson wrote: > On 2017/07/11 01:55, Kyle J. McKay wrote: > > 2) 32-bit systems are going to be around for many years still; 32-bit ARM > > platforms are everywhere > .. > > 4) 32-bit time_t has potentially still got over 20 years of life left in > it > > Yes. The gamble is whether 32-bit systems will still be around then. > I don't see why they _wouldn't_ be. The sooner that OS adapt, the less > likely there are to still be operational systems when 2038 comes around. > > > 3) 32-bit systems typically have 32-bit time_t values (I'm not aware of > > anything in the standard preventing use of a 64-bit time_t on a 32-bit > > system but that doesn't seem to be happening, at least not yet) > > This depends on the OS. Linux's general avoidance of ABI breaks is > certainly a problem in this area. NetBSD has used 64-bit time_t on all > architectures since 6.0 (2012) and OpenBSD since 5.5 (2014). I haven't > looked recently but IIRC FreeBSD has 64-bit time_t on 32-bit ARM > (and of course on 64-bit systems). > > Plenty of software still needs patching to fix operation on a 32-bit > arch with 64-bit time_t - we have a lot of these in the ports tree > (with mixed success feeding such changes upstream - "Linux doesn't > need it"...). > >
Re: [PATCH] allow notAfter after 2038 with 32-bit time_t
On Thu, May 18, 2017 at 7:31 AM, Kyle J. McKay wrote:
> RFC 5280 section 4.1.2.5 states:
>
> To indicate that a certificate has no well-defined expiration date,
> the notAfter SHOULD be assigned the GeneralizedTime value of
> 1231235959Z.
>
>
True enough.
> Unfortunately, if sizeof(time_t) == 4, -12-31T23:59:59Z cannot be
> represented as a time_t value causing valid certificates to be rejected
> just because the notAfter value is after 2038-01-19T03:14:07Z.
>
Correct
So, I'll ask - what is the platform you are using that needs this? OpenBSD
does not, nor do most modern unix systems - So what platform are we doing
this for?
I'm not asking it to be nasty, but to put it in a slightly different
context "OMG, windows 98 does not support this, but if you add this code I
can support windows 98". You and I both know what your answer would
probably be for the person with Windows 98, which is "Here's a nickel kid,
get a modern operating system".
We ripped out support for a lot of moribund platforms for a reason. What
platform are you using with 32 bit time where this is an issue? I'm not
saying no, I'm saying "convince me this matters for anything relevant
please", and I am open to being convinced.
Fix this problem by disabling the restriction in the X509_cmp_time function
> and "wrap" far in the future notAfter values to 2038-01-19T03:14:07Z in the
> tls_get_peer_cert_times function.
>
> With both of these changes certificates with "no well-defined expiration
> date" as specified by RFC 5280 are again accepted on platforms where the
> sizeof(time_t) == 4.
>
> In general, there's no reason that a notAfter value should not be wrapped
> to 2038-01-19T03:14:07Z on a system with a 32-bit time_t. The system
> itself
> can never have a time after 2038-01-19T03:14:07Z because of the size of the
> time_t type and so wrapping a notAfter date that is after
> 2038-01-19T03:14:07Z
> to 2038-01-19T03:14:07Z can never result in any additional certificates
> being
> accepted on such a system.
>
> Signed-off-by: Kyle J. McKay
> ---
>
> For those using the libressl-2.5.4.tar.gz distribution, an equivalent
> patch that updates the tarball files instead can be found here:
>
> https://gist.github.com/7d4d59bbae9e4d18444b86aa79d6f350
>
> Without this patch (or an equivalent), libressl-portable is not a viable
> alternative to OpenSSL on systems with a 32-bit time_t.
>
> It rejects valid TLS connections to any site that contains a notAfter date
> after 2038-01-19T03:14:07Z in any certificate in the chain.
>
> Besides the special case date mentioned above, there are many root
> certificates
> already in use today that have notAfter dates beyond 2038-01-19.
>
> These patches have been tested with libressl-portable 2.5.4 on a system
> with
> a 32-bit time_t. With both of these patches the "nc -c" command
> successfully
> connects to sites with certificates that include notAfter dates after
> 2038-01-19.
>
> If either patch is omitted, "nc -c" fails to connect.
>
> src/lib/libcrypto/x509/x509_vfy.c | 3 ++-
> src/lib/libtls/tls_conninfo.c | 8 ++--
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/src/lib/libcrypto/x509/x509_vfy.c
> b/src/lib/libcrypto/x509/x509_vfy.c
> index d8c09a12..c59bd258 100644
> --- a/src/lib/libcrypto/x509/x509_vfy.c
> +++ b/src/lib/libcrypto/x509/x509_vfy.c
> @@ -1882,7 +1882,8 @@ X509_cmp_time(const ASN1_TIME *ctm, time_t *cmp_time)
> * a time_t. A time_t must be sane if you care about times after
> * Jan 19 2038.
> */
> - if ((time1 = timegm(&tm1)) == -1)
> + if (((time1 = timegm(&tm1)) == -1) &&
> + ((sizeof(time_t) != 4) || tm1.tm_year < 138))
> goto out;
>
> if (gmtime_r(&time2, &tm2) == NULL)
> diff --git a/src/lib/libtls/tls_conninfo.c b/src/lib/libtls/tls_conninfo.c
> index 5cdd0f77..a59b4ba2 100644
> --- a/src/lib/libtls/tls_conninfo.c
> +++ b/src/lib/libtls/tls_conninfo.c
> @@ -142,8 +142,12 @@ tls_get_peer_cert_times(struct tls *ctx, time_t
> *notbefore,
> goto err;
> if ((*notbefore = timegm(&before_tm)) == -1)
> goto err;
> - if ((*notafter = timegm(&after_tm)) == -1)
> - goto err;
> + if ((*notafter = timegm(&after_tm)) == -1) {
> + if (sizeof(time_t) == 4 && after_tm.tm_year >= 138)
> + *notafter = 2147483647;
> + else
> + goto err;
> + }
>
> return (0);
>
> ---
>
>
Re: Better handling of short reads
> As you all might have gathered by now Amit has jumped the gun
> but was wrong to do so. His setup is not affected by this change.
> That was expected so please don't get distracted by this as I'm
> still looking forward to replies to the original set of changes.
> beck@?
>
> > diff --git sys/kern/vfs_bio.c sys/kern/vfs_bio.c
> > index 95bc80bc0e6..9316e6e0eb2 100644
> > --- sys/kern/vfs_bio.c
> > +++ sys/kern/vfs_bio.c
> > @@ -534,10 +534,27 @@ bread_cluster_callback(struct buf *bp)
> > */
> > buf_fix_mapping(bp, newsize);
> > bp->b_bcount = newsize;
> > }
> >
> > + /* Invalidate read-ahead buffers if read short */
> > + if (bp->b_resid > 0) {
> > + printf("read %ld resid %ld\n", bp->b_bcount, bp->b_resid);
Should the printf actually be here? I'm not thinking this thing
spewing dmesg like a banshee if we get short reads is really going
to help anything
> > + for (i = 0; xbpp[i] != NULL; i++)
> > + continue;
> > + for (i = i - 1; i != 0; i--) {
> > + if (xbpp[i]->b_bufsize <= bp->b_resid) {
> > + bp->b_resid -= xbpp[i]->b_bufsize;
> > + SET(xbpp[i]->b_flags, B_INVAL);
> > + } else if (bp->b_resid > 0) {
> > + bp->b_resid = 0;
> > + SET(xbpp[i]->b_flags, B_INVAL);
> > + } else
> > + break;
> > + }
> > + }
> > +
> > for (i = 1; xbpp[i] != 0; i++) {
> > if (ISSET(bp->b_flags, B_ERROR))
> > SET(xbpp[i]->b_flags, B_INVAL | B_ERROR);
> > biodone(xbpp[i]);
> > }
>
Re: Better handling of short reads
- ok mike, I'm looking at it.. Allow me a short while to beat my
head against a wall for a bit to get it into readahead mode...
On Wed, Jun 14, 2017 at 3:56 AM, Mike Belopuhov wrote:
> On Thu, Jun 08, 2017 at 11:55 +0200, Mike Belopuhov wrote:
> > On Wed, Jun 07, 2017 at 23:04 -0500, Amit Kulkarni wrote:
> > > On Wed, 7 Jun 2017 21:27:27 -0500
> > > Amit Kulkarni wrote:
> > >
> > > > On Thu, 8 Jun 2017 01:57:25 +0200
> > > > Mike Belopuhov wrote:
> > > >
> > > > > On Wed, Jun 07, 2017 at 18:35 -0500, Amit Kulkarni wrote:
> > > > > > Wow, please get this in!!!
> > > > > >
> > > > > > This fixes cvs update on hard disks, to go much much faster.
> When I am
> > > > > > updating the entire set of cvs trees: www, src, xenocara, ports,
> I can
> > > > > > still use firefox and have it perfectly usable. There's a night
> and
> > > > > > day improvement, before and after. Thanks for debugging and
> fixing
> > > > > > this.
> > > > > >
> > > > >
> > > > > What kind of broken hardware do you have that this diff helps you?
> > > > > Can you show us your dmesg?
> > > > >
> > >
> > > Please ignore previous dmesg, it was incomplete.
> > >
> >
> > Are you 100% sure this diff changes anything for you?
> > Can you please try the one below. It adds a printf.
> >
>
> As you all might have gathered by now Amit has jumped the gun
> but was wrong to do so. His setup is not affected by this change.
> That was expected so please don't get distracted by this as I'm
> still looking forward to replies to the original set of changes.
> beck@?
>
> > diff --git sys/kern/vfs_bio.c sys/kern/vfs_bio.c
> > index 95bc80bc0e6..9316e6e0eb2 100644
> > --- sys/kern/vfs_bio.c
> > +++ sys/kern/vfs_bio.c
> > @@ -534,10 +534,27 @@ bread_cluster_callback(struct buf *bp)
> >*/
> > buf_fix_mapping(bp, newsize);
> > bp->b_bcount = newsize;
> > }
> >
> > + /* Invalidate read-ahead buffers if read short */
> > + if (bp->b_resid > 0) {
> > + printf("read %ld resid %ld\n", bp->b_bcount, bp->b_resid);
> > + for (i = 0; xbpp[i] != NULL; i++)
> > + continue;
> > + for (i = i - 1; i != 0; i--) {
> > + if (xbpp[i]->b_bufsize <= bp->b_resid) {
> > + bp->b_resid -= xbpp[i]->b_bufsize;
> > + SET(xbpp[i]->b_flags, B_INVAL);
> > + } else if (bp->b_resid > 0) {
> > + bp->b_resid = 0;
> > + SET(xbpp[i]->b_flags, B_INVAL);
> > + } else
> > + break;
> > + }
> > + }
> > +
> > for (i = 1; xbpp[i] != 0; i++) {
> > if (ISSET(bp->b_flags, B_ERROR))
> > SET(xbpp[i]->b_flags, B_INVAL | B_ERROR);
> > biodone(xbpp[i]);
> > }
>
>
Re: ocspcheck size_t printing
You are correct.
Patch committed. Thanks!
-Bob
On Mon, May 08, 2017 at 08:20:57PM +0200, Jonas 'Sortie' Termansen wrote:
> Hi,
>
> When upgrading to libressl-2.5.4 I noticed a couple -Wformat errors due
> to this code assuming size_t is of type long when it was actually int on
> this 32-bit system. Here's a patch against cvs that fixes the issue and
> also prints the variableas unsigned type.
>
> Jonas
>
> Index: ocspcheck.c
> ===
> RCS file: /cvs/src/usr.sbin/ocspcheck/ocspcheck.c,v
> retrieving revision 1.20
> diff -u -r1.20 ocspcheck.c
> --- ocspcheck.c 27 Mar 2017 23:59:08 - 1.20
> +++ ocspcheck.c 8 May 2017 18:15:51 -
> @@ -564,7 +564,7 @@
> if ((request = ocsp_request_new_from_cert(certfile, nonce)) == NULL)
> exit(1);
>
> - dspew("Built an %ld byte ocsp request\n", request->size);
> + dspew("Built an %zu byte ocsp request\n", request->size);
>
> if ((host = url2host(request->url, &port, &path)) == NULL)
> errx(1, "Invalid OCSP url %s from %s", request->url,
> @@ -601,7 +601,7 @@
> dspew("Server at %s returns:\n", host);
> for (i = 0; i < httphsz; i++)
> dspew(" [%s]=[%s]\n", httph[i].key, httph[i].val);
> - dspew(" [Body]=[%ld bytes]\n", hget->bodypartsz);
> + dspew(" [Body]=[%zu bytes]\n", hget->bodypartsz);
> if (hget->bodypartsz <= 0)
> errx(1, "No body in reply from %s", host);
>
>
Official OpenBSD 6.1 CD !
So. There *Is* an official OpenBSD 6.1 CD Just One. If you are interested, please bid on ebay : http://www.ebay.com/itm/The-only-Official-OpenBSD-6-1-CD-set-to-be-made-For-auction-for-the-project-/252910718452?hash=item3ae2a74df4:g:SJQAAOSwrhBZBqkd (It's a pretty cool little CD set!)
Re: explicit_bzero after readpassphrase
On Mon, May 01, 2017 at 04:07:27PM -0600, Theo de Raadt wrote: > > Let me stop here and ask if the pattern is: "always explicit_bzero > a password field once it is used"? It might make sense, but some > of these are heading straight to exit immediately. Is it too much > to do it then, or is the worry these code patterns might get copied > elsewhere? > I would fall on the side of "It could get copied elsewhere or hoisted for other reasons (like pledge)" so do it anyway.
Re: patch: mv(1): Add -p flag to preserve time stamps for moved directories
> Note that I have noatime on this FS. then turn that off, or understand that things will not behave as you expect them to with it on.
Re: httpd/libtls: TLS client certificate revocation checking
There will be some libtls api additions post 6.1 to get the peer cert in PEM format In the meantime, testing snaps prior to 6.1 should be the priority. not a talkathon. On Sat, Apr 1, 2017 at 10:49 Joerg Sonnenberger wrote: > On Sat, Apr 01, 2017 at 07:53:05PM +1030, Jack Burton wrote: > > One common example of that happening is when a cert gets revoked because > > its private key has been lost/stolen and the user needs a new cert > > associated with the same identity. An even more common example is when > > a cert expires & gets renewed. > > If you are using certificate revocation, I think you should do the check > as early as possible. That means in httpd in this case. Nothing later in > the stack should have to care about expired or revoked certificates -- > it just adds complexity and the danger of someone forgetting about it. > > Which mechanisms to support (i.e. CRLs or OCSP) is a completely > different topic. > > Joerg > >
Re: regarding OpenSSL License change
On Thu, Mar 23, 2017 at 17:48 Bob Beck wrote: > Honestly, anyone who gets one of these should say no > > what would you all think if people quietly took derived works of software > licensed under one license and took silence as assent to relicense > > Does this mean that with an unanswered email i can now release my re > licensed as ISC version of gcc? or the linux kernel? > > This sort of action just means that any software you write can be > plagiarized against your will if you did not find out about it in time. > > thats really not cool > > If you write software this is not a world you want to live in. Even if > it does mean a anyone who can fork a github repo could get rid of the GPL > after a period of non response from an author (dont go on vacation). As > much as I might not agree with the GPL personally, I respect someones right > to release thier work under a license and have it respected. without having > to constantly answer emails and click web links telling people no > > > > On Thu, Mar 23, 2017 at 10:58 Theo de Raadt wrote: > > > > The start suggests they want to privately collect sufficient consensus > > > to pass their agenda. They appear to be considering all actions in > > > the tree (including mine) on equal grounds. > > > > I already sent them a clear "NO, i explicitly object to relicensing > > any of my contributions." > > > > If any of you care about the possibility of merging future OpenSSL > > improvements to LibreSSL and OpenBSD, i suggest you do the same. > > > > Similarly, if any of you dislike publishing their own code under Apache > 2. > > There has been no discussion amongst the greater community of > developers as to which license to take. Apache 2 has come as an edict > from Rich Salz. > > There has also been no statement from the original authorship that this > is the way to go. > > I suspect there is a lack of approval from some, and manufacturing > consent in volume is the approach being taken. > > > Apparently lawyers are being paid to help them push this through. Is > that being paid for by donations people gave after Heartbleed? Is > this why people donated? > >
Re: regarding OpenSSL License change
Honestly, anyone who gets one of these should say no what would you all think if people quietly took derived works of software licensed under one license and took silence as assent to relicense Does this mean that with an unanswered email i can now release my re licensed as ISC version of gcc? or the linux kernel? This sort of action just means that any software you write can be plagiarized against your will if you did not find out about it in time. thats really not cool If you write software this is not a world you want to live in. Even if it does mean a anyone who can fork a github repo could get rid of the GPL after a period of non response from an author (dont go on vacation). As much as I might not agree with the GPL personally, I respect someones right to release thier work under a license and have it respected. without having to constantly answer emails and click web links telling people no On Thu, Mar 23, 2017 at 10:58 Theo de Raadt wrote: > > > The start suggests they want to privately collect sufficient consensus > > > to pass their agenda. They appear to be considering all actions in > > > the tree (including mine) on equal grounds. > > > > I already sent them a clear "NO, i explicitly object to relicensing > > any of my contributions." > > > > If any of you care about the possibility of merging future OpenSSL > > improvements to LibreSSL and OpenBSD, i suggest you do the same. > > > > Similarly, if any of you dislike publishing their own code under Apache > 2. > > There has been no discussion amongst the greater community of > developers as to which license to take. Apache 2 has come as an edict > from Rich Salz. > > There has also been no statement from the original authorship that this > is the way to go. > > I suspect there is a lack of approval from some, and manufacturing > consent in volume is the approach being taken. > > > Apparently lawyers are being paid to help them push this through. Is > that being paid for by donations people gave after Heartbleed? Is > this why people donated? > >
Re: tlsv1 alert decrypt error
And as joel mentioned, a fix is already arriving for this - there was a bug in SSLv2 compatible handshake initiation, and Paypal still has it enabled... (yeeuch) On Mon, Mar 6, 2017 at 3:48 PM, Bob Beck wrote: > > Move it to tech@ from misc.. not libressl.. libressl is not special ;) > > On Mon, Mar 6, 2017 at 3:21 PM, Kirill Miazine wrote: > >> Moving to libressl@ from misc@, as it's a LibreSSL issue. >> >> * Joel Sing [2017-03-05 23:01]: >> >> On Thursday 02 March 2017 13:28:08 Kirill Miazine wrote: >>> >>>> Recently I've noticed a number of error messages in my Exim mail log: >>>> >>>> TLS error on connection from mx1.slc.paypal.com (mx0.slc.paypal.com >>>> ) >>>> [173.0.84.226] \ (SSL_accept): error:1403741B:SSL >>>> routines:ACCEPT_SR_KEY_EXCH:tlsv1 alert decrypt error TLS client >>>> disconnected cleanly (rejected our certificate?) >>>> >>> >>> This is most likely the same issue as that reported on the libressl@ >>> mailing >>> list a day or so ago - expect a fix to arrive shortly. >>> >> >> I rebuilt exim on latest snapshot (OpenBSD 6.1-beta (GENERIC.MP) #213: >> Mon Mar 6 12:31:59 MST 2017) and the error looks different now: >> >> TLS error on connection from mx0.phx.paypal.com [66.211.168.230] \ >>(SSL_accept): error:14039119:SSL routines:ACCEPT_SR_CERT_VRFY:decryption >> \ >>failed or bad record mac >> >> >> -- >>-- Kirill Miazine >> >> >
Re: tlsv1 alert decrypt error
Move it to tech@ from misc.. not libressl.. libressl is not special ;) On Mon, Mar 6, 2017 at 3:21 PM, Kirill Miazine wrote: > Moving to libressl@ from misc@, as it's a LibreSSL issue. > > * Joel Sing [2017-03-05 23:01]: > > On Thursday 02 March 2017 13:28:08 Kirill Miazine wrote: >> >>> Recently I've noticed a number of error messages in my Exim mail log: >>> >>> TLS error on connection from mx1.slc.paypal.com (mx0.slc.paypal.com) >>> [173.0.84.226] \ (SSL_accept): error:1403741B:SSL >>> routines:ACCEPT_SR_KEY_EXCH:tlsv1 alert decrypt error TLS client >>> disconnected cleanly (rejected our certificate?) >>> >> >> This is most likely the same issue as that reported on the libressl@ >> mailing >> list a day or so ago - expect a fix to arrive shortly. >> > > I rebuilt exim on latest snapshot (OpenBSD 6.1-beta (GENERIC.MP) #213: > Mon Mar 6 12:31:59 MST 2017) and the error looks different now: > > TLS error on connection from mx0.phx.paypal.com [66.211.168.230] \ >(SSL_accept): error:14039119:SSL routines:ACCEPT_SR_CERT_VRFY:decryption > \ >failed or bad record mac > > > -- >-- Kirill Miazine > >
Re: Scheduler ping-pong with preempt()
Go for it mpi.. move forward. ok beck@ On Mon, Feb 6, 2017 at 7:48 AM, Martin Pieuchot wrote: > On 24/01/17(Tue) 13:35, Martin Pieuchot wrote: > > Userland threads are preempt()'d when hogging a CPU or when processing > > an AST. Currently when such a thread is preempted the scheduler looks > > for an idle CPU and puts it on its run queue. That means the number of > > involuntary context switch often result in a migration. > > > > This is not a problem per se and one could argue that if another CPU > > is idle it makes sense to move. However with the KERNEL_LOCK() moving > > to another CPU won't necessarily allows the preempt()'d thread to run. > > It's even worse, it increases contention. > > > > If you add to this behavior the fact that sched_choosecpu() prefers idle > > CPUs in a linear order, meaning CPU0 > CPU1 > .. > CPUN, you'll > > understand that the set of idle CPUs will change every time preempt() is > > called. > > > > I believe this behavior affects kernel threads by side effect, since > > the set of idle CPU changes every time a thread is preempted. With this > > diff the 'softnet' thread didn't move on a 2 CPUs machine during simple > > benchmarks. Without, it plays ping-pong between CPU. > > > > The goal of this diff is to reduce the number of migrations. You > > can compare the value of 'sched_nomigrations' and 'sched_nmigrations' > > with and without it. > > > > As usual, I'd like to know what's the impact of this diff on your > > favorite benchmark. Please test and report back. > > I only got positive test results so I'd like to commit the diff below. > > ok? > > Index: kern/sched_bsd.c > === > RCS file: /cvs/src/sys/kern/sched_bsd.c,v > retrieving revision 1.44 > diff -u -p -r1.44 sched_bsd.c > --- kern/sched_bsd.c25 Jan 2017 06:15:50 - 1.44 > +++ kern/sched_bsd.c6 Feb 2017 14:47:28 - > @@ -329,7 +329,6 @@ preempt(struct proc *newp) > SCHED_LOCK(s); > p->p_priority = p->p_usrpri; > p->p_stat = SRUN; > - p->p_cpu = sched_choosecpu(p); > setrunqueue(p); > p->p_ru.ru_nivcsw++; > mi_switch(); > >
Re: Password corruption in adduser
ok beck@ On Sun, Feb 5, 2017 at 22:53 Theo Buehler wrote: > On Sun, Feb 05, 2017 at 09:47:35PM -0800, Philip Guenther wrote: > > On Sun, 5 Feb 2017, John McGuigan wrote: > > > I've noticed something strange in adduser -- when attempting to add a > > > user completely though command line argument it seems to corrupt the > > > entry in /etc/master.passwd. > > > > > > Example: > > > > > > $ echo "HorseBatteryStaple" | encrypt > > > $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2 > > > > > > # adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh \ > > > -message no -batch some.user "" "Some User" \ > > > $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2 > > > Added user ``some.user'' > > ... > > > some.user:b/bin/ksh9/9uoOrbTRaf//3ZprAb9k.hOpfe9vYVqjf1a:5000:5000:: \ > > > 0:0:Some User:/home/some.user:/bin/ksh > > > > > > As you can see the password entry gets corrupted with a 'b/bin/ksh...' > > > > Let's see what the adduser command is seeing by passing that all to > 'echo' > > instead: > > > > # echo \ > > > adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh \ > > > -message no -batch some.user "" "Some User" \ > > > $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2 > > adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh > -message no -batch some.user Some User b/bin/ksh9/FGXw.9oNjr3BLTS7DJp5n4M2 > > # > > > > Ah, so the expansion is happening *outside* of adduser...in the shell. > > Yes, the shell does variable expansion even if the dollar-sign is in the > > middle of a word, so it's expanding the variables > > $2 --> "" > > $0 --> "/bin/ksh" > > $ssZSLC6laHsTS7O2FwJ4Mufw6mSS --> "" > > > > > > > Behavior *is* present when hash is wrapped in " > > > > Sure, because double-quotes only stop file-globbing and field splitting > > and not variable expansion. You need single quotes for that: > > > > # echo \ > > > adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh \ > > > -message no -batch some.user "" "Some User" \ > > > '$2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2' > > adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh > -message no -batch some.user Some User > $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2 > > # > > The adduser.8 manual page has an example with no quotes in it, so we > should fix that. Also, let's use a new hash using $2b$ instead of $2a$. > > Index: adduser.8 > === > RCS file: /var/cvs/src/usr.sbin/adduser/adduser.8,v > retrieving revision 1.44 > diff -u -p -r1.44 adduser.8 > --- adduser.8 24 Dec 2015 16:54:37 - 1.44 > +++ adduser.8 6 Feb 2017 05:49:00 - > @@ -373,7 +373,7 @@ The password has been created using > .Xr encrypt 1 : > .Bd -literal -offset indent > # adduser -batch falken guest,staff,beer 'Prof. Falken' \e > -$2a$06$1Sdjxjoxg4cNmT6zAxriGOLgdLXQ3HdJ2dKBbzEk68jSrO1EtLJ3C > +'$2b$10$aOadQNznQ1YJFnqNaRRneOvYvZAEO7atYiTND3EsLf6afHT5t1UIK' > .Ed > .Pp > Create user > >
Re: netcat: IPv6 address support for proxy
ok beck@
On Sun, Feb 05, 2017 at 12:27:19AM +0100, Jeremie Courreges-Anglas wrote:
>
> The colons used in IPv6 addresses conflicts with the proxy port
> specification. Do the right thing for -x ::1:8080, [::1] and
> [::1]:8080.
>
> ok?
>
>
> Index: netcat.c
> ===
> RCS file: /d/cvs/src/usr.bin/nc/netcat.c,v
> retrieving revision 1.171
> diff -u -p -p -u -r1.171 netcat.c
> --- netcat.c 30 Nov 2016 07:56:23 - 1.171
> +++ netcat.c 4 Feb 2017 23:24:42 -
> @@ -148,8 +148,8 @@ main(int argc, char *argv[])
> struct servent *sv;
> socklen_t len;
> struct sockaddr_storage cliaddr;
> - char *proxy;
> - const char *errstr, *proxyhost = "", *proxyport = NULL;
> + char *proxy, *proxyport = NULL;
> + const char *errstr;
> struct addrinfo proxyhints;
> char unix_dg_tmp_socket_buf[UNIX_DG_TMP_SOCKET_SIZE];
> struct tls_config *tls_cfg = NULL;
> @@ -426,15 +426,29 @@ main(int argc, char *argv[])
> if (family == AF_UNIX)
> errx(1, "no proxy support for unix sockets");
>
> - /* XXX IPv6 transport to proxy would probably work */
> - if (family == AF_INET6)
> - errx(1, "no proxy support for IPv6");
> -
> if (sflag)
> errx(1, "no proxy support for local source address");
>
> - proxyhost = strsep(&proxy, ":");
> - proxyport = proxy;
> + if (*proxy == '[') {
> + ++proxy;
> + proxyport = strchr(proxy, ']');
> + if (proxyport == NULL)
> + errx(1, "missing closing bracket in proxy");
> + *proxyport++ = '\0';
> + if (*proxyport == '\0')
> + /* Use default proxy port. */
> + proxyport = NULL;
> + else {
> + if (*proxyport == ':')
> + ++proxyport;
> + else
> + errx(1, "garbage proxy port delimiter");
> + }
> + } else {
> + proxyport = strrchr(proxy, ':');
> + if (proxyport != NULL)
> + *proxyport++ = '\0';
> + }
>
> memset(&proxyhints, 0, sizeof(struct addrinfo));
> proxyhints.ai_family = family;
> @@ -617,7 +631,7 @@ main(int argc, char *argv[])
> }
> if (xflag)
> s = socks_connect(host, portlist[i], hints,
> - proxyhost, proxyport, proxyhints, socksv,
> + proxy, proxyport, proxyhints, socksv,
> Pflag);
> else
> s = remote_connect(host, portlist[i], hints);
> --
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
>
Re: Update for US Holidays.
On Sat, Feb 04, 2017 at 01:52:14PM -0700, Bob Beck wrote: > > Presented without further comment. > > ok? > Or maybe this is more appropriate: Index: calendar.history === RCS file: /cvs/src/usr.bin/calendar/calendars/calendar.history,v retrieving revision 1.80 diff -u -p -u -p -r1.80 calendar.history --- calendar.history19 Nov 2016 12:41:22 - 1.80 +++ calendar.history4 Feb 2017 21:28:55 - @@ -475,6 +475,7 @@ 12/12 First wireless message sent across Atlantic by Marconi, 1901 12/13 Dartmouth College chartered, 1769 12/14 Portugal joins the United Nations, 1955 +12/14 World totals the cost of running embedded 32 bit Linux, 1901 12/15 Argo Merchant oil spill, 1976 12/15 James Naismith invents basketball, Canada, 1891 12/16 Pokemon episode (Electric Soldier Porygon) triggers attacks of
Re: Update for US Holidays.
On Sat, Feb 04, 2017 at 12:59:53PM -0800, Philip Guenther wrote: > On Sat, Feb 4, 2017 at 12:52 PM, Bob Beck wrote: > > > > Presented without further comment. > > > > ok? > > NACK. Obsolete 32bit time_t OSes can track their own damn holidays. But how will I remember to be appropriately devotional if my embedded linux pacemaker didn't kill me the previous day? :)
Update for US Holidays.
Presented without further comment. ok? Index: calendar.usholiday === RCS file: /cvs/src/usr.bin/calendar/calendars/calendar.usholiday,v retrieving revision 1.9 diff -u -p -u -p -r1.9 calendar.usholiday --- calendar.usholiday 19 Jan 2015 18:07:47 - 1.9 +++ calendar.usholiday 4 Feb 2017 20:50:41 - @@ -33,6 +33,7 @@ 11/SunFirstDaylight Saving Time ends; clocks move back (1st Sunday in November) 11/11 Veterans' Day 11/ThuFourth Thanksgiving Day (4th Thursday in November) +12/14 "National Day of Patriotic Devotion" (After 1901, for 32 bit time_t systems) 12/21* Winter Solstice 12/24 Christmas Eve 12/25 Christmas
Re: specify curves via ecdhe statement in httpd.conf
try connecting with openbsd nc rather than s-client On Sat, Feb 4, 2017 at 09:13 Bob Beck wrote: > > On Sat, Feb 4, 2017 at 07:51 Andreas Bartelt wrote: > > On 02/04/17 05:26, Joel Sing wrote: > > On Wednesday 01 February 2017 15:41:29 Andreas Bartelt wrote: > >> Hello, > >> > >> after reading the LibreSSL accouncement from today, I assumed that > >> specifying ecdhe "auto" in /etc/httpd.conf would enable X25519, P-256 > >> and P-384 on current. > > > > This is correct. > > > >> I've noticed that "auto" enables only curves x25519 and P-256 (which is > >> what I'd want to use - but somehow unexpected with regard to the > >> announcement). > > > > Why do you believe this is the case? > > > > Tested with a build of today's current: > - httpd started with ecdhe "auto" in /etc/httpd.conf > - then trying to connect via openssl s_client with -groups P-384 option > doesn't negotiate a cipher suite. > > However, specifying -groups P-256 works. I don't know how to specify > x25519 with OpenBSD's openssl s_client (it's not yet listed in openssl > ecparam -list_curves output) but SSL Labs successfully negotiates via > x25519 and P-256 (but not P-384). P-384 doesn't seem to be enabled with > "auto". > > Another confusing test result: > - httpd started with ecdhe "secp384r1" (P-384) > - then trying to connect via openssl s_client with -groups P-384 option > also doesn't negotiate a cipher suite! > > However, SSL Labs successfully connects to httpd and confirms support > for secp384r1. > > Can you reproduce this? > > >> Diff is attached which clarifies the meaning of "auto" in httpd.conf.5. > > > > There are some documentation improvements that could be used here, > however the > > meaning of auto for httpd.conf.5 needs to refer to the meaning of "auto" > for > > libtls (currently tls_config_set_ecdhecurve()). Otherwise libtls changes > and > > httpd becomes out of date. > > > >> There currently seems to be no way to explicitly specify x25519, or to > >> specify multiple colon separated curves with the ecdhe statement. Would > >> it make sense to change semantics and make the ecdhe statement in > >> httpd.conf consistent with the recent changes to openssl s_client > >> -groups (e.g., to also allow more common names like P-256 instead of > >> prime256v1)? > > > > Yes - tls_config_set_ecdhecurve() needs to change to accept the same > colon > > separate list of priority ordered curve names, that > SSL_set1_curves_list() > > accepts. > > > > > >
OpenBSD errata, Jan 31, 2017
An issue has been identified whereby httpd(8) could be subject to a denial of service attack. Repeated crafted requests could be made from a client using file-range requests, making the server consume excessive amounts of memory. This issue has been fixed in current. For 5.9 and 6.0 the following errata will disable range header processing in httpd(8) to prevent the problem. Thanks to Pierre Kim for reporting the issue. https://ftp.openbsd.org/pub/OpenBSD/patches/6.0/common/017_httpd.patch.sig https://ftp.openbsd.org/pub/OpenBSD/patches/5.9/common/034_httpd.patch.sig
err with multiple TLS sites but one OCSP?
Sooo..
Pretty sure mlucas has uncovered a problem with the ocsp interface.
Basically I didn't attach it to the keypair, (yes Joel, I think you
told me so) so it only works with the master keypair.. OK, but the
problem is that it also returns the staple for other keypairs which is
wrong.
This attaches the ocsp staple to the keypair, rather than the config.
It does not yet add a way to change it for keypairs other than the
master - that will require an API change - but with this change
it should not return an incorrect ocsp staple for the non primary
keypair. I'll deal with the API change separately after pestering
joel about it a bit.
ok?
Index: tls_config.c
===
RCS file: /cvs/src/lib/libtls/tls_config.c,v
retrieving revision 1.34
diff -u -p -u -p -r1.34 tls_config.c
--- tls_config.c24 Jan 2017 01:48:05 - 1.34
+++ tls_config.c28 Jan 2017 21:40:14 -
@@ -101,6 +101,26 @@ tls_keypair_set_key_mem(struct tls_keypa
return set_mem(&keypair->key_mem, &keypair->key_len, key, len);
}
+static int
+tls_keypair_set_ocsp_staple_file(struct tls_keypair *keypair,
+struct tls_error *error, const char *ocsp_file)
+{
+ if (keypair->ocsp_staple != NULL)
+ explicit_bzero(keypair->ocsp_staple, keypair->ocsp_staple_len);
+ return tls_config_load_file(error, "ocsp", ocsp_file,
+ &keypair->ocsp_staple, &keypair->ocsp_staple_len);
+}
+
+static int
+tls_keypair_set_ocsp_staple_mem(struct tls_keypair *keypair,
+const uint8_t *staple, size_t len)
+{
+ if (keypair->ocsp_staple != NULL)
+ explicit_bzero(keypair->ocsp_staple, keypair->ocsp_staple_len);
+ return set_mem(&keypair->ocsp_staple, &keypair->ocsp_staple_len, staple,
+ len);
+}
+
static void
tls_keypair_clear(struct tls_keypair *keypair)
{
@@ -241,7 +261,6 @@ tls_config_free(struct tls_config *confi
free((char *)config->ca_mem);
free((char *)config->ca_path);
free((char *)config->ciphers);
- free(config->ocsp_staple);
free(config);
}
@@ -664,14 +683,14 @@ tls_config_verify_client_optional(struct
int
tls_config_set_ocsp_staple_file(struct tls_config *config, const char
*staple_file)
{
- return tls_config_load_file(&config->error, "OCSP", staple_file,
- &config->ocsp_staple, &config->ocsp_staple_len);
+ return tls_keypair_set_ocsp_staple_file(config->keypair, &config->error,
+ staple_file);
}
int
tls_config_set_ocsp_staple_mem(struct tls_config *config, char *staple, size_t
len)
{
- return set_mem(&config->ocsp_staple, &config->ocsp_staple_len, staple,
len);
+ return tls_keypair_set_ocsp_staple_mem(config->keypair, staple, len);
}
int
Index: tls_internal.h
===
RCS file: /cvs/src/lib/libtls/tls_internal.h,v
retrieving revision 1.52
diff -u -p -u -p -r1.52 tls_internal.h
--- tls_internal.h 26 Jan 2017 12:56:37 - 1.52
+++ tls_internal.h 28 Jan 2017 21:07:25 -
@@ -51,6 +51,8 @@ struct tls_keypair {
size_t cert_len;
char *key_mem;
size_t key_len;
+ char *ocsp_staple;
+ size_t ocsp_staple_len;
};
#define TLS_MIN_SESSION_TIMEOUT (4)
@@ -83,8 +85,6 @@ struct tls_config {
int ecdhecurve;
struct tls_keypair *keypair;
int ocsp_require_stapling;
- char *ocsp_staple;
- size_t ocsp_staple_len;
uint32_t protocols;
unsigned char session_id[TLS_MAX_SESSION_ID_LENGTH];
int session_lifetime;
Index: tls_ocsp.c
===
RCS file: /cvs/src/lib/libtls/tls_ocsp.c,v
retrieving revision 1.10
diff -u -p -u -p -r1.10 tls_ocsp.c
--- tls_ocsp.c 27 Jan 2017 07:03:27 - 1.10
+++ tls_ocsp.c 28 Jan 2017 21:42:22 -
@@ -332,17 +332,19 @@ tls_ocsp_stapling_cb(SSL *ssl, void *arg
if ((ctx = SSL_get_app_data(ssl)) == NULL)
goto err;
- if (ctx->config->ocsp_staple == NULL ||
- ctx->config->ocsp_staple_len == 0)
+ if (ctx->config->keypair->ocsp_staple == NULL ||
+ ctx->config->keypair->ocsp_staple == NULL ||
+ ctx->config->keypair->ocsp_staple_len == 0)
return SSL_TLSEXT_ERR_NOACK;
- if ((ocsp_staple = malloc(ctx->config->ocsp_staple_len)) == NULL)
+ if ((ocsp_staple = malloc(ctx->config->keypair->ocsp_staple_len)) ==
+ NULL)
goto err;
- memcpy(ocsp_staple, ctx->config->ocsp_staple,
- ctx->config->ocsp_staple_len);
+ memcpy(ocsp_staple, ctx->config->keypair->ocsp_staple,
+ ctx->config->keypair->ocsp_staple_len);
if (SSL_set_tlsext_status_ocsp_resp(ctx->ssl_conn, ocsp_staple,
- ctx->config->ocsp_staple_len) != 1)
+ ctx->config->keypair->ocsp_staple_len) != 1)
goto err;
Re: err with multiple TLS sites but one OCSP?
On Fri, Jan 27, 2017 at 15:23 Stuart Henderson wrote: > On 2017/01/27 22:09, Bob Beck wrote: > > > I think you have more issues than ocsp. if thats the same host you can't > > > have two different tls certs on the same ip. and you have them both on > > > *443 > > > > > > try using a separate ip for each > > > > Wasn't SNI support added to httpd already? > > hmmm. right. but i bet itll work with explicit separate ip's. stapling on > the other hand will be per config. so thats probably whats fighting. > separate ip would confirm that. > im tired. ill look at it tomorrow unless someone else does > > >
Re: err with multiple TLS sites but one OCSP?
I think you have more issues than ocsp. if thats the same host you can't
have two different tls certs on the same ip. and you have them both on
*443
try using a separate ip for each
On Fri, Jan 27, 2017 at 15:03 Michael W. Lucas
wrote:
> On Fri, Jan 27, 2017 at 09:53:25PM +0000, Bob Beck wrote:
>
> >On Fri, Jan 27, 2017 at 14:12 Michael W. Lucas
>
> > Or a misconfiguration. Â show configs
>
>
>
>
>
> Configs follow.
>
>
>
> # cat /etc/httpd.conf
>
> include "/etc/sites/www3.conf"
>
> include "/etc/sites/www4.conf"
>
>
>
> www3.conf:
>
>
>
> server "www3.mwlucas.org" {
>
>listen on * port 80
>
>block return 302 "https://$SERVER_NAME$REQUEST_URI";
>
> }
>
>
>
>
>
> server "www3.mwlucas.org" {
>
> alias tarpit.mwlucas.org
>
> listen on * tls port 443
>
> hsts
>
> # TLS certificate and key files created with acme-client(1)
>
> tls certificate "/etc/ssl/acme/www3/www3.fullchain.pem"
>
> tls key "/etc/ssl/acme/www3/www3.key"
>
> tls ocsp "/etc/ssl/acme/www3/www3.der"
>
> tcp nodelay
>
>
>
>location "/.well-known/acme-challenge/*" {
>
>root "/acme"
>
>root strip 2
>
>}
>
> }
>
>
>
>
>
> www4:
>
>
>
> server "www4.mwlucas.org" {
>
> alias bill.mwlucas.org
>
> alias auction.mwlucas.org
>
> listen on * port 80
>
>
>
>location "/.well-known/acme-challenge/*" {
>
>root "/acme"
>
>root strip 2
>
>}
>
>
>
>
>
> block return 301 "https://$DOCUMENT_URI";
>
> }
>
>
>
> server "www4.mwlucas.org" {
>
> alias bill.mwlucas.org
>
> alias auction.mwlucas.org
>
> root "/www4"
>
> listen on * tls port 443
>
> hsts
>
> # TLS certificate and key files created with acme-client(1)
>
> tls certificate "/etc/ssl/acme/www4/www4.fullchain.pem"
>
> tls key "/etc/ssl/acme/www4/www4.key"
>
> # tls ocsp "/etc/ssl/acme/www4/www4.der"
>
> tcp nodelay
>
>location "/.well-known/acme-challenge/*" {
>
>root "/acme"
>
>root strip 2
>
>}
>
>
>
> }
>
>
>
>
>
>
>
>
>
> --
>
> Michael W. LucasTwitter @mwlauthor
>
> nonfiction: https://www.michaelwlucas.com/
>
> fiction: https://www.michaelwarrenlucas.com/
>
> blog: http://blather.michaelwlucas.com/
>
>
Re: err with multiple TLS sites but one OCSP?
On Fri, Jan 27, 2017 at 14:12 Michael W. Lucas wrote: > On Fri, Jan 27, 2017 at 02:50:29PM -0500, Michael W. Lucas wrote: > > > On Fri, Jan 27, 2017 at 06:49:06PM +, Stuart Henderson wrote: > > > > That looks like a web server bug, it shouldn't return a staple > > > Or a misconfiguration. show configs > > > > > in that case. What software are you using for that? > > > > > > > > > > > > OpenBSD httpd, of course. amd64 snapshot downloaded yesterday from > > > ftp3.usa.openbsd.org. > > > > To be clear, that's a "How the hell could I forget to include that?" > > facepalm, not anything about Stuart asking the question... > > > > -- > > Michael W. LucasTwitter @mwlauthor > > nonfiction: https://www.michaelwlucas.com/ > > fiction: https://www.michaelwarrenlucas.com/ > > blog: http://blather.michaelwlucas.com/ > > > >
Re: Allow install from https server w/ self signed cert
On Sat, Jan 07, 2017 at 03:52:04PM -0700, Theo de Raadt wrote: > > What workarounds would be reasonable and approriate? and does it > > make sense for OpenBSD to support such scenarios out-of-the-box to > > promote wider adoption of better software? > > If you want buy the OpenBSD-installer-for-drones, contact me offline. > That featureset didn't make it into the free software version. But out GeoLocation shit in the installer will just find that it's located on a netblock that is registered 5000 feet over Pakistan and direct it to a local public mirror! Definately have to disable that feature from the free software version.
Re: Allow install from https server w/ self signed cert
On Sat, Jan 07, 2017 at 05:42:24PM -0500, Jacob L. Leifman wrote: > Most of the time I agree with this particular attitude and it is indeed > appropriate for the OP case. However, there some major networks such as > various governments (or for example .mil) that do not participate in > the public PKI but run their own PKI infrastructure. What effect will > the installer's checks have in that environment? What workarounds would > be reasonable and approriate? and does it make sense for OpenBSD to > support such scenarios out-of-the-box to promote wider adoption of > better software? That's not a good reason, since in the case of what I am describing they would *still be depending on the public PKI infrastrucure* to publish their own list of signers.. No.. they aren't going to do that.. Sorry, Unless you're mailing me from a .mil address I think you might be imagining this usage case. they're probably building from source, or have the wherewithall to roll their own bsd.rd if they care about doing this.
Re: Allow install from https server w/ self signed cert
On Fri, Jan 06, 2017 at 10:48:37AM -0500, RD Thrush wrote: > On 01/06/17 06:28, Stuart Henderson wrote: > > Related to this (and particularly thinking about autoinstalls), > > would it make sense to allow explicit protocols in the hostname? > > > > some.host -> https with http fallback > > http://some.host/ -> http only > > https://some.host/ -> https only, no fallback > > That would totally work for my install problem. > > FWIW, instead of running a patched install.sub, "rm /etc/ssl/cert.pem" makes > the install bypass the https attempt. > Note, if you're upgrading or otherwise have a way to et a cert.pem bundle onto there to *replace* the default, you could always drop the signer for your private self-signed server into the cert.pem bundle, at which point it would be accepted as trusted. of course if you're just installing you have an interesting chicken and egg problem, unless you put it somewhere on an https site that does have a real certificate, drop out of the installer and do ftp -o /tmp/mysigner.pem https://my.secure.site/mysigner.pem cat /tmp/mysigner.pem >> /etc/ssl/cert.pem then continue the install, and you're good. Almost wonder if it's worth an extra question in the installer to ask for an https address to retrieve a certficiate bundle to be appended to cert.pem for the install...
Re: acme-client use configuration file [1 of 5]
No objection in principle.. although since some of us depend on this we
might either need warning and/or a small period of overlap where the old
stuff works and then we can move to the new stuff without things blowing
up.
On Sun, Jan 1, 2017 at 1:59 PM, Sebastian Benoit wrote:
> start using the configuration file and delete command line arguments:
>
> -a agreement-> agreement url ...
> -c certdir -> domain certificate "path"
> -f accountkey -> account key "path"
> -k domainkey-> domain key "path"
> -s authority-> sign with "name"
>
> new argument:
> -f configfile
>
> the changes needed to use the new configuration are local to main.c for
> now.
> While the configuration could be passed directly to netproc(), keyproc()
> etc,
> the diff is smaller this way.
>
> This also removes the multidir (-m) mode for now - specify different paths
> in
> each domain {} block instead.
>
> diff --git usr.sbin/acme-client/Makefile usr.sbin/acme-client/Makefile
> index 55e0b0e..eae13ed 100644
> --- usr.sbin/acme-client/Makefile
> +++ usr.sbin/acme-client/Makefile
> @@ -13,6 +13,6 @@ CFLAGS+= -W -Wall -I${.CURDIR}
> CFLAGS+= -Wstrict-prototypes -Wmissing-prototypes
> CFLAGS+= -Wmissing-declarations
> CFLAGS+= -Wshadow -Wpointer-arith
> -CFLAGS+= -Wsign-compare
> +CFLAGS+= -Wsign-compare -Wunused
>
> .include
> diff --git usr.sbin/acme-client/acme-client.1 usr.sbin/acme-client/acme-
> client.1
> index 526c11f..6f38573 100644
> --- usr.sbin/acme-client/acme-client.1
> +++ usr.sbin/acme-client/acme-client.1
> @@ -22,15 +22,10 @@
> .Nd ACME client
> .Sh SYNOPSIS
> .Nm acme-client
> -.Op Fl bFmNnrv
> -.Op Fl a Ar agreement
> +.Op Fl bFNnrv
> .Op Fl C Ar challengedir
> -.Op Fl c Ar certdir
> -.Op Fl f Ar accountkey
> -.Op Fl k Ar domainkey
> -.Op Fl s Ar authority
> +.Op Fl f Ar configfile
> .Ar domain
> -.Op Ar altnames
> .Sh DESCRIPTION
> The
> .Nm
> @@ -39,8 +34,6 @@ Automatic Certificate Management Environment (ACME)
> client.
> .Pp
> The options are as follows:
> .Bl -tag -width Ds
> -.It Fl a Ar agreement
> -Use an alternative user agreement URL.
> .It Fl b
> Back up all certificates in the certificate directory.
> This only happens if a remove or replace operation is possible.
> @@ -58,67 +51,21 @@ Any given backup uses the same Epoch time for all
> three certificates.
> If there are no certificates in place, this option does nothing.
> .It Fl C Ar challengedir
> The directory to register challenges.
> -.It Fl c Ar certdir
> -The directory to store public certificates.
> .It Fl F
> Force updating the certificate signature even if it's too soon.
> -.It Fl f Ar accountkey
> -The account private key.
> -This was either made with a previous client or with
> -.Fl n .
> -.It Fl k Ar domainkey
> -The private key for the domain.
> -This may also be created with
> -.Fl N .
> -.It Fl m
> -Append
> -.Ar domain
> -to all default paths except the challenge path
> -.Pq i.e. those that are overridden by Fl c , k , f .
> -Thus,
> -.Ar foo.com
> -as the initial domain would make the default domain private key into
> -.Pa /etc/ssl/acme/private/foo.com/privkey.pem .
> -This is useful in setups with multiple domain sets.
> +.It Fl f Ar configfile
> +Specify an alternative configuration file.
> .It Fl N
> Create a new RSA domain key if one does not already exist.
> .It Fl n
> Create a new RSA account key if one does not already exist.
> .It Fl r
> Revoke the X509 certificate found in the certificates.
> -.It Fl s Ar authority
> -ACME
> -.Ar authority
> -to talk to.
> -Currently the following authorities are available:
> -.Pp
> -.Bl -tag -width "letsencrypt-staging" -compact
> -.It Cm letsencrypt
> -Let's Encrypt authority
> -.It Cm letsencrypt-staging
> -Let's Encrypt staging authority
> -.El
> -.Pp
> -The default is
> -.Cm letsencrypt .
> .It Fl v
> Verbose operation.
> Specify twice to also trace communication and data transfers.
> .It Ar domain
> The domain name.
> -The only difference between this and
> -.Ar altnames
> -is that it's put into the certificate's
> -.Li CN
> -field and it uses the
> -.Qq main
> -domain when specifying
> -.Fl m .
> -.It Ar altnames
> -Alternative names
> -.Pq Dq SAN
> -for the domain name.
> -The number of SAN entries is limited to 100 or so.
> .El
> .Pp
> Public certificates are by default placed in
> @@ -175,7 +122,7 @@ as in the
> .Sx Challenges
> section:
> .Pp
> -.Dl # acme-client -vNn foo.com www.foo.com smtp.foo.com
> +.Dl # acme-client -vNn www.foo.com
> .Pp
> A daily
> .Xr cron 8
> @@ -183,7 +130,7 @@ job can renew the certificates:
> .Bd -literal -offset indent
> #! /bin/sh
>
> -acme-client foo.com www.foo.com smtp.foo.com
> +acme-client www.foo.com
>
> if [ $? -eq 0 ]
> then
> diff --git usr.sbin/acme-client/chngproc.c usr.sbin/acme-client/chngproc.c
> index 4cb7f33..3e931da 100644
> --- usr.sbin/acme-client/chngproc.c
> +++ usr.sbin/acme-client/chngproc.c
> @@ -27,7 +27,7 @@
> #include "extern.h"
Re: libtls syslogd pledge abort
> Or do not call tls_configure_ssl_verify() if verification is turned
> off.
This makes sense to me.
>
> Index: lib/libtls/tls_client.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/lib/libtls/tls_client.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 tls_client.c
> --- lib/libtls/tls_client.c 26 Dec 2016 16:20:58 - 1.38
> +++ lib/libtls/tls_client.c 29 Dec 2016 22:56:23 -
> @@ -195,7 +195,9 @@ tls_connect_common(struct tls *ctx, cons
> }
> }
>
> - if (tls_configure_ssl_verify(ctx, ctx->ssl_ctx, SSL_VERIFY_PEER) == -1)
> + if (ctx->config->verify_cert &&
> + (tls_configure_ssl_verify(ctx, ctx->ssl_ctx,
> + SSL_VERIFY_PEER) == -1))
> goto err;
>
> if (SSL_CTX_set_tlsext_status_cb(ctx->ssl_ctx, tls_ocsp_verify_cb) !=
> 1) {
>
ok beck@
> I would prefer the fix in libtls as
> - this problem may also affect other daemons
> - avoid to do unnecsessary stuff
> - syslogd could run on a system without cert.pem
>
> comments? ok?
>
> bluhm
Re: httpd(8)/proc.c: use less fds on startup
This is now working on www.openbsd.org. I upgraded my
6.0 system to current today off the latest snap and httpd would
not start, same problem.
This diff lets current httpd start again.
ok beck@
On Tue, Oct 04, 2016 at 11:54:37PM +0200, Rafael Zalamena wrote:
> On Tue, Oct 04, 2016 at 07:46:52PM +0200, Rafael Zalamena wrote:
> > This diff makes proc.c daemons to use less file descriptors on startup,
> > this way we increase the number of child we can have considerably. This
> > also improves the solution on a bug reported in bugs@
> > "httpd errors out with 'too many open files'".
> >
> > To achieve that I delayed the socket distribution and made a minimal
> > socket allocation in proc_init(), so only the necessary children socket
> > are allocated and passed with proc_exec(). After the event_init() is called
> > we call proc_connect() which creates the socketpair() and immediatly after
> > each call we already sends them without accumulating.
> >
> > Note: We still have to calculate how many fds we will want to have and
> > then limit the daemon prefork configuration.
> >
> > ok?
> >
>
> Paul de Weerd still found problems with the diff, because the httpd(8)
> would not exit successfully with '-n' flag. It happened because the
> new proc_connect() code tried to write fds to children process that
> did not start caused by the ps_noaction flag. (thanks Paul!)
>
> This new diff just adds a check for ps_noaction in proc_init() and
> proc_connect() so it doesn't try to do anything if we are not really
> going to start the daemon. Also I removed the ps_noaction from proc_run()
> since we are not going to get to this point anymore.
>
> ok?
>
>
> Index: proc.c
> ===
> RCS file: /home/obsdcvs/src/usr.sbin/httpd/proc.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 proc.c
> --- proc.c28 Sep 2016 12:01:04 - 1.27
> +++ proc.c4 Oct 2016 21:50:43 -
> @@ -36,8 +36,6 @@
>
> void proc_exec(struct privsep *, struct privsep_proc *, unsigned int,
> int, char **);
> -void proc_connectpeer(struct privsep *, enum privsep_procid, int,
> - struct privsep_pipes *);
> void proc_setup(struct privsep *, struct privsep_proc *, unsigned int);
> void proc_open(struct privsep *, int, int);
> void proc_accept(struct privsep *, int, enum privsep_procid,
> @@ -147,72 +145,18 @@ proc_exec(struct privsep *ps, struct pri
> }
>
> void
> -proc_connectpeer(struct privsep *ps, enum privsep_procid id, int inst,
> -struct privsep_pipes *pp)
> -{
> - unsigned int i, j;
> - struct privsep_fdpf;
> -
> - for (i = 0; i < PROC_MAX; i++) {
> - /* Parent is already connected with everyone. */
> - if (i == PROC_PARENT)
> - continue;
> -
> - for (j = 0; j < ps->ps_instances[i]; j++) {
> - /* Don't send socket to child itself. */
> - if (i == (unsigned int)id &&
> - j == (unsigned int)inst)
> - continue;
> - if (pp->pp_pipes[i][j] == -1)
> - continue;
> -
> - pf.pf_procid = i;
> - pf.pf_instance = j;
> - proc_compose_imsg(ps, id, inst, IMSG_CTL_PROCFD,
> - -1, pp->pp_pipes[i][j], &pf, sizeof(pf));
> - pp->pp_pipes[i][j] = -1;
> - }
> - }
> -}
> -
> -/* Inter-connect all process except with ourself. */
> -void
> proc_connect(struct privsep *ps)
> {
> - unsigned int src, i, j;
> - struct privsep_pipes*pp;
> - struct imsgev *iev;
> -
> - /* Listen on appropriate pipes. */
> - src = privsep_process;
> - pp = &ps->ps_pipes[src][ps->ps_instance];
> -
> - for (i = 0; i < PROC_MAX; i++) {
> - /* Don't listen to ourself. */
> - if (i == src)
> - continue;
> -
> - for (j = 0; j < ps->ps_instances[i]; j++) {
> - if (pp->pp_pipes[i][j] == -1)
> - continue;
> -
> - iev = &ps->ps_ievs[i][j];
> - imsg_init(&iev->ibuf, pp->pp_pipes[i][j]);
> - event_set(&iev->ev, iev->ibuf.fd, iev->events,
> - iev->handler, iev->data);
> - event_add(&iev->ev, NULL);
> - }
> - }
> + unsigned int src, dst;
>
> - /* Exchange pipes between process. */
> - for (i = 0; i < PROC_MAX; i++) {
> - /* Parent is already connected with everyone. */
> - if (i == PROC_PARENT)
> - continue;
> + /* Don't distribute any sockets if we are not really going to run. */
> + if (ps->ps_noaction)
> + return;
>
> - for (j = 0; j < ps->ps_instances[i]; j++)
Re: rebound quantum entanglement
BTW I'm not picking on you.. my DNS setup blew up this week for local
resolution and I've been dealing with the fallout - so the topic
is relatively near and dear to my heart.
On Wed, Sep 14, 2016 at 10:07 PM, Bob Beck wrote:
>
> Yep. and now you need to solve the problem that when prepending
> 127.0.0.1, and hitting rebound, which in turn is going to only grab the
> first dns server from my resolv.conf instead of all of them, that it now
> doubles my failure time when the first dns server doesn't respond (once for
> libc asking rebound, which asks only the first server it found, which
> fails) then falls back to libc asking resolv.conf which again... asks the
> first server, and fails again.
>
> So the problem here is that it's going to be great when things are working
> but become a failure multiplier when something breaks.
>
> Of course - perhaps you could query them all in parallel to mitigate this?
> Rebound might need to become a real boy if we're going to do something
> like this. If rebound were a "real boy" it would be it easier to say "just
> use rebound or if it's not there just use resolv.conf normally
>
> then nothing changes at *all* when it's not there.
>
>
> On Wed, Sep 14, 2016 at 8:39 PM, Ted Unangst wrote:
>
>> Ted Unangst wrote:
>> > Bob Beck wrote:
>> > > how is rebound going to handle a change in resolv.conf? thats still a
>> > > problem here
>> >
>> > oh, that's easy. it watches the file for changes. i never quite got
>> around to
>> > that, but it's another five lines.
>>
>> ok, so it's a net +15 lines, including blanks.
>>
>> Index: rebound.8
>> ===
>> RCS file: /cvs/src/usr.sbin/rebound/rebound.8,v
>> retrieving revision 1.4
>> diff -u -p -r1.4 rebound.8
>> --- rebound.8 4 Dec 2015 04:50:43 - 1.4
>> +++ rebound.8 15 Sep 2016 00:57:21 -
>> @@ -33,9 +33,7 @@ The options are as follows:
>> .Bl -tag -width Ds
>> .It Fl c Ar config
>> Specify an alternative configuration file, instead of the default
>> -.Pa /etc/rebound.conf .
>> -At present, the config file consists of a single line containing the next
>> -hop DNS server.
>> +.Pa /etc/resolv.conf .
>> .Nm
>> will reload the configuration file when sent a SIGHUP signal.
>> .It Fl d
>> @@ -46,8 +44,8 @@ does not
>> into the background.
>> .El
>> .Sh FILES
>> -.Bl -tag -width "/etc/rebound.confXX" -compact
>> -.It Pa /etc/rebound.conf
>> +.Bl -tag -width "/etc/resolv.confXX" -compact
>> +.It Pa /etc/resolv.conf
>> Default
>> .Nm
>> configuration file.
>> Index: rebound.c
>> ===
>> RCS file: /cvs/src/usr.sbin/rebound/rebound.c,v
>> retrieving revision 1.70
>> diff -u -p -r1.70 rebound.c
>> --- rebound.c 1 Sep 2016 10:57:24 - 1.70
>> +++ rebound.c 15 Sep 2016 02:30:46 -
>> @@ -33,10 +33,12 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> #include
>> +#include
>>
>> #define MINIMUM(a,b) (((a)<(b))?(a):(b))
>>
>> @@ -455,34 +457,51 @@ fail:
>> }
>>
>> static int
>> -readconfig(FILE *conf, union sockun *remoteaddr)
>> +readconfig(int conffd, union sockun *remoteaddr)
>> {
>> + const char ns[] = "nameserver";
>> char buf[1024];
>> + char *p;
>> struct sockaddr_in *sin = &remoteaddr->i;
>> struct sockaddr_in6 *sin6 = &remoteaddr->i6;
>> + FILE *conf;
>> + int rv = -1;
>>
>> - if (fgets(buf, sizeof(buf), conf) == NULL)
>> - return -1;
>> - buf[strcspn(buf, "\n")] = '\0';
>> + conf = fdopen(conffd, "r");
>>
>> - memset(remoteaddr, 0, sizeof(*remoteaddr));
>> - if (inet_pton(AF_INET, buf, &sin->sin_addr) == 1) {
>> - sin->sin_len = sizeof(*sin);
>> - sin->sin_family = AF_INET;
>> - sin->sin_port = htons(53);
>> - return AF_INET;
>> - } else if (inet_pton(AF_INET6, buf, &sin6->sin6_addr) == 1) {
>> - sin6->sin6_len = sizeof(*sin6);
>> - sin6->sin6_family = AF_INET6;
>> - sin6->sin6_port = htons(53);
>> -
Re: rebound quantum entanglement
Yep. and now you need to solve the problem that when prepending 127.0.0.1,
and hitting rebound, which in turn is going to only grab the first dns
server from my resolv.conf instead of all of them, that it now doubles my
failure time when the first dns server doesn't respond (once for libc
asking rebound, which asks only the first server it found, which fails)
then falls back to libc asking resolv.conf which again... asks the first
server, and fails again.
So the problem here is that it's going to be great when things are working
but become a failure multiplier when something breaks.
Of course - perhaps you could query them all in parallel to mitigate this?
Rebound might need to become a real boy if we're going to do something
like this. If rebound were a "real boy" it would be it easier to say "just
use rebound or if it's not there just use resolv.conf normally
then nothing changes at *all* when it's not there.
On Wed, Sep 14, 2016 at 8:39 PM, Ted Unangst wrote:
> Ted Unangst wrote:
> > Bob Beck wrote:
> > > how is rebound going to handle a change in resolv.conf? thats still a
> > > problem here
> >
> > oh, that's easy. it watches the file for changes. i never quite got
> around to
> > that, but it's another five lines.
>
> ok, so it's a net +15 lines, including blanks.
>
> Index: rebound.8
> ===
> RCS file: /cvs/src/usr.sbin/rebound/rebound.8,v
> retrieving revision 1.4
> diff -u -p -r1.4 rebound.8
> --- rebound.8 4 Dec 2015 04:50:43 - 1.4
> +++ rebound.8 15 Sep 2016 00:57:21 -
> @@ -33,9 +33,7 @@ The options are as follows:
> .Bl -tag -width Ds
> .It Fl c Ar config
> Specify an alternative configuration file, instead of the default
> -.Pa /etc/rebound.conf .
> -At present, the config file consists of a single line containing the next
> -hop DNS server.
> +.Pa /etc/resolv.conf .
> .Nm
> will reload the configuration file when sent a SIGHUP signal.
> .It Fl d
> @@ -46,8 +44,8 @@ does not
> into the background.
> .El
> .Sh FILES
> -.Bl -tag -width "/etc/rebound.confXX" -compact
> -.It Pa /etc/rebound.conf
> +.Bl -tag -width "/etc/resolv.confXX" -compact
> +.It Pa /etc/resolv.conf
> Default
> .Nm
> configuration file.
> Index: rebound.c
> ===
> RCS file: /cvs/src/usr.sbin/rebound/rebound.c,v
> retrieving revision 1.70
> diff -u -p -r1.70 rebound.c
> --- rebound.c 1 Sep 2016 10:57:24 - 1.70
> +++ rebound.c 15 Sep 2016 02:30:46 -
> @@ -33,10 +33,12 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> #include
> +#include
>
> #define MINIMUM(a,b) (((a)<(b))?(a):(b))
>
> @@ -455,34 +457,51 @@ fail:
> }
>
> static int
> -readconfig(FILE *conf, union sockun *remoteaddr)
> +readconfig(int conffd, union sockun *remoteaddr)
> {
> + const char ns[] = "nameserver";
> char buf[1024];
> + char *p;
> struct sockaddr_in *sin = &remoteaddr->i;
> struct sockaddr_in6 *sin6 = &remoteaddr->i6;
> + FILE *conf;
> + int rv = -1;
>
> - if (fgets(buf, sizeof(buf), conf) == NULL)
> - return -1;
> - buf[strcspn(buf, "\n")] = '\0';
> + conf = fdopen(conffd, "r");
>
> - memset(remoteaddr, 0, sizeof(*remoteaddr));
> - if (inet_pton(AF_INET, buf, &sin->sin_addr) == 1) {
> - sin->sin_len = sizeof(*sin);
> - sin->sin_family = AF_INET;
> - sin->sin_port = htons(53);
> - return AF_INET;
> - } else if (inet_pton(AF_INET6, buf, &sin6->sin6_addr) == 1) {
> - sin6->sin6_len = sizeof(*sin6);
> - sin6->sin6_family = AF_INET6;
> - sin6->sin6_port = htons(53);
> - return AF_INET6;
> - } else {
> - return -1;
> + while (fgets(buf, sizeof(buf), conf) != NULL) {
> + buf[strcspn(buf, "\n")] = '\0';
> +
> + if (strncmp(buf, ns, strlen(ns)) != 0)
> + continue;
> + p = buf + strlen(ns) + 1;
> + while (isspace((unsigned char)*p))
> + p++;
> +
> + /* this will not end well */
> + if (strcmp(p, "127.0.0.1") == 0)
> + continue;
> +
> + memset(remoteaddr, 0, sizeof(*remoteaddr));
> +
Re: rebound quantum entanglement
wont this also mean if it is not running i have to wait for the localhost attempt to fail before the resolver moves on? (ASR_STATE_NEXT_NS, etc) so i slow everything down for a timeout? dont get me wrong, it is an interesting direction, but I think maybe get the rest of the five line changes into rebound to make it useful and then look at libc which might need slightly more cleverness than just adding localhost unconditionally. On Wednesday, 14 September 2016, Ted Unangst wrote: > Bob Beck wrote: > > how is rebound going to handle a change in resolv.conf? thats still a > > problem here > > oh, that's easy. it watches the file for changes. i never quite got around > to > that, but it's another five lines. >
Re: rebound quantum entanglement
how is rebound going to handle a change in resolv.conf? thats still a
problem here
On Wednesday, 14 September 2016, Ted Unangst wrote:
> So the plan is for rebound to be the 'system' resolver, with libc talking
> to
> rbeound and rebound talking to the cloud. The main wrinkle is how does
> rebound
> find the cloud? rebound.conf, but dhclient doesn't know anything about
> rebound.conf, preferring to edit resolv.conf. But if rebound reads
> resolv.conf, what does libc read? This has been a bit of a tangle until
> now,
> especially in scenarios like upgrades where rebound may not even be
> running.
>
> And so I present the following diff to enable a smooth transition. It's
> 'quantum' because it works whether or not rebound is running. No need to
> open
> the box.
>
> 1. rebound reads resolv.conf. This remains the config file for upstream
> DNS.
>
> 2. libc now prepends its nameserver list with localhost, thus always
> searching
> for rebound. If it's not running, we just continue down the list.
>
> This covers the basic use case, where enabling rebound now requires no
> additional work. No need to edit dhclient.conf, etc. It also works on
> ramdisks. It also works with a mix of old and new binaries. Once you flip
> resolv.conf back to upstream, old binaries will bypass rebound, but that's
> ok.
> The new rebound checks to make sure it's not stuck in a time loop, which is
> never good.
>
> I also note this improves the situation for people who have been using
> unbound
> as a local cache, too. Just enable unbound and libc will use it
> automatically.
>
> Particular edge case: if resolv.conf has no nameservers, then the localhost
> default is not prepended. So libc won't try talking to rebound if it's
> specifically configured not to (chroot).
>
>
> Index: lib/libc/asr/asr.c
> ===
> RCS file: /cvs/src/lib/libc/asr/asr.c,v
> retrieving revision 1.54
> diff -u -p -r1.54 asr.c
> --- lib/libc/asr/asr.c 18 Jun 2016 15:25:28 - 1.54
> +++ lib/libc/asr/asr.c 15 Sep 2016 00:42:30 -
> @@ -549,6 +549,15 @@ pass0(char **tok, int n, struct asr_ctx
> return;
> if (n != 2)
> return;
> + /* prepend localhost to list */
> + if (ac->ac_nscount == 0) {
> + if (asr_parse_nameserver((struct sockaddr *)&ss,
> "127.0.0.1"))
> + return;
> + if ((ac->ac_ns[ac->ac_nscount] = calloc(1,
> ss.ss_len)) == NULL)
> + return;
> + memmove(ac->ac_ns[ac->ac_nscount], &ss,
> ss.ss_len);
> + ac->ac_nscount += 1;
> + }
> if (asr_parse_nameserver((struct sockaddr *)&ss, tok[1]))
> return;
> if ((ac->ac_ns[ac->ac_nscount] = calloc(1, ss.ss_len)) ==
> NULL)
> Index: usr.sbin/rebound/rebound.8
> ===
> RCS file: /cvs/src/usr.sbin/rebound/rebound.8,v
> retrieving revision 1.4
> diff -u -p -r1.4 rebound.8
> --- usr.sbin/rebound/rebound.8 4 Dec 2015 04:50:43 - 1.4
> +++ usr.sbin/rebound/rebound.8 15 Sep 2016 00:57:21 -
> @@ -33,9 +33,7 @@ The options are as follows:
> .Bl -tag -width Ds
> .It Fl c Ar config
> Specify an alternative configuration file, instead of the default
> -.Pa /etc/rebound.conf .
> -At present, the config file consists of a single line containing the next
> -hop DNS server.
> +.Pa /etc/resolv.conf .
> .Nm
> will reload the configuration file when sent a SIGHUP signal.
> .It Fl d
> @@ -46,8 +44,8 @@ does not
> into the background.
> .El
> .Sh FILES
> -.Bl -tag -width "/etc/rebound.confXX" -compact
> -.It Pa /etc/rebound.conf
> +.Bl -tag -width "/etc/resolv.confXX" -compact
> +.It Pa /etc/resolv.conf
> Default
> .Nm
> configuration file.
> Index: usr.sbin/rebound/rebound.c
> ===
> RCS file: /cvs/src/usr.sbin/rebound/rebound.c,v
> retrieving revision 1.70
> diff -u -p -r1.70 rebound.c
> --- usr.sbin/rebound/rebound.c 1 Sep 2016 10:57:24 - 1.70
> +++ usr.sbin/rebound/rebound.c 15 Sep 2016 00:53:26 -
> @@ -37,6 +37,7 @@
> #include
> #include
> #include
> +#include
>
> #define MINIMUM(a,b) (((a)<(b))?(a):(b))
>
> @@ -457,28 +458,41 @@ fail:
> static int
> readconfig(FILE *conf, union sockun *remoteaddr)
> {
> + const char ns[] = "nameserver";
> char buf[1024];
> + char *p;
> struct sockaddr_in *sin = &remoteaddr->i;
> struct sockaddr_in6 *sin6 = &remoteaddr->i6;
>
> - if (fgets(buf, sizeof(buf), conf) == NULL)
> - return -1;
> - buf[strcspn(buf, "\n")] = '\0';
> + while (fgets(buf, sizeof(buf), conf) != NULL) {
> + buf[strcspn(buf, "\n")] = '\0';
>
> - memset(remoteaddr, 0, sizeof(*remoteadd
Re: reduce double caching in mfs
I really dislike "CHEAP".
and it almost seems like these should actually be NOCACHE.. why the heck
can't they be?
On Thu, Sep 8, 2016 at 7:49 PM, Ted Unangst wrote:
> Currently, the bufcache doesn't know that mfs is backed by memory. All i/o
> to
> mfs ends up being double cached, once in the userland process and again in
> the
> kernel bufcache. This is wasteful. In particular, it means one can't use
> mfs
> to increase the effective size of the buffer cache. Reading or writing to
> mfs
> will push out buffers used for disk caching. (I think you can even end up
> with
> triple buffering when mfs starts swapping...)
>
> This is mostly inherent to the design of mfs. But with a small tweak to the
> buffer cache, we can improve the situation. This introduces the concept of
> 'cheap' buffers, a hint to the buffer cache that they are easily replaced.
> (There's a 'nocache' flag already, but it's not suitable here.) When mfs
> finishes with a buf, it marks it cheap. Then it goes onto a special queue
> that
> gets chewed up before we start looking at the regular caches. We still
> cache
> some number of cheap buffers to prevent constant memory copying.
>
> With this diff, I've confirmed that reading/writing large files to mfs
> doesn't
> flush the cache, but performance appears about the same. (Of particular
> note,
> my bufcache is big enough to cache all of src/, but not src/ and obj/. With
> obj/ on mfs, src never gets flushed.)
>
>
> Index: kern/vfs_bio.c
> ===
> RCS file: /cvs/src/sys/kern/vfs_bio.c,v
> retrieving revision 1.176
> diff -u -p -r1.176 vfs_bio.c
> --- kern/vfs_bio.c 4 Sep 2016 10:51:24 - 1.176
> +++ kern/vfs_bio.c 8 Sep 2016 18:31:52 -
> @@ -93,7 +93,10 @@ int bd_req; /* Sleep point for cleaner
>
> #define NUM_CACHES 2
> #define DMA_CACHE 0
> +#define CHEAP_LIMIT 256
> struct bufcache cleancache[NUM_CACHES];
> +struct bufqueue cheapqueue;
> +u_int cheapqueuelen;
> struct bufqueue dirtyqueue;
>
> void
> @@ -1297,6 +1300,7 @@ bufcache_init(void)
> TAILQ_INIT(&cleancache[i].coldqueue);
> TAILQ_INIT(&cleancache[i].warmqueue);
> }
> + TAILQ_INIT(&cheapqueue);
> TAILQ_INIT(&dirtyqueue);
> }
>
> @@ -1329,6 +1333,12 @@ bufcache_getcleanbuf(int cachenum, int d
>
> splassert(IPL_BIO);
>
> + /* try cheap queue if over limit */
> + if (discard || cheapqueuelen > CHEAP_LIMIT) {
> + if ((bp = TAILQ_FIRST(&cheapqueue)))
> + return bp;
> + }
> +
> /* try cold queue */
> while ((bp = TAILQ_FIRST(&cache->coldqueue))) {
> if ((!discard) &&
> @@ -1356,6 +1366,8 @@ bufcache_getcleanbuf(int cachenum, int d
> /* buffer is cold - give it up */
> return bp;
> }
> + if ((bp = TAILQ_FIRST(&cheapqueue)))
> + return bp;
> if ((bp = TAILQ_FIRST(&cache->warmqueue)))
> return bp;
> if ((bp = TAILQ_FIRST(&cache->hotqueue)))
> @@ -1410,6 +1422,13 @@ bufcache_take(struct buf *bp)
> pages = atop(bp->b_bufsize);
> struct bufcache *cache = &cleancache[bp->cache];
> if (!ISSET(bp->b_flags, B_DELWRI)) {
> + if (ISSET(bp->b_flags, B_CHEAP)) {
> + TAILQ_REMOVE(&cheapqueue, bp, b_freelist);
> + cheapqueuelen--;
> + CLR(bp->b_flags, B_CHEAP);
> + return;
> + }
> +
> if (ISSET(bp->b_flags, B_COLD)) {
> queue = &cache->coldqueue;
> } else if (ISSET(bp->b_flags, B_WARM)) {
> @@ -1462,11 +1481,17 @@ bufcache_release(struct buf *bp)
> struct bufqueue *queue;
> int64_t pages;
> struct bufcache *cache = &cleancache[bp->cache];
> +
> pages = atop(bp->b_bufsize);
> KASSERT(ISSET(bp->b_flags, B_BC));
> KASSERT((ISSET(bp->b_flags, B_DMA) && bp->cache == 0)
> || ((!ISSET(bp->b_flags, B_DMA)) && bp->cache > 0));
> if (!ISSET(bp->b_flags, B_DELWRI)) {
> + if (ISSET(bp->b_flags, B_CHEAP)) {
> + TAILQ_INSERT_TAIL(&cheapqueue, bp, b_freelist);
> + cheapqueuelen++;
> + return;
> + }
> int64_t *queuepages;
> if (ISSET(bp->b_flags, B_WARM | B_COLD)) {
> SET(bp->b_flags, B_WARM);
> Index: sys/buf.h
> ===
> RCS file: /cvs/src/sys/sys/buf.h,v
> retrieving revision 1.103
> diff -u -p -r1.103 buf.h
> --- sys/buf.h 23 May 2016 09:31:28 - 1.103
> +++ sys/buf.h 8 Sep 2016 17:20:12 -
> @@ -221,12 +221,14 @@ struct bufcache {
> #defineB_COLD 0x0100 /* buffer is on the cold
> queue */
> #
Re: [PATCH] Callback-based interface to libtls
I am in agreement in principle, but please coordinate with bcook@ and/or
jsing@ who were possibly doing
some related adjustments.
On Mon, Sep 5, 2016 at 4:44 AM, Ted Unangst wrote:
> Bob Beck wrote:
> > >
> > > Agreed, I was also a bit unclear on payload at first (though it grew on
> > > me over time, so I didn't change it). Here's an update with the
> > > parameter renamed and better documented.
> > >
> > > ok?
> >
> > Yeah. I'm good with this
> >
> > IMO get it in so we can tweak it in tree.
>
> first tweak: the context has a type: struct tls *, so use it.
>
> Index: tls.h
> ===
> RCS file: /cvs/src/lib/libtls/tls.h,v
> retrieving revision 1.37
> diff -u -p -r1.37 tls.h
> --- tls.h 4 Sep 2016 14:15:44 - 1.37
> +++ tls.h 5 Sep 2016 10:42:50 -
> @@ -44,9 +44,9 @@ extern "C" {
> struct tls;
> struct tls_config;
>
> -typedef ssize_t (*tls_read_cb)(void *_ctx, void *_buf, size_t _buflen,
> +typedef ssize_t (*tls_read_cb)(struct tls *_ctx, void *_buf, size_t
> _buflen,
> void *_cb_arg);
> -typedef ssize_t (*tls_write_cb)(void *_ctx, const void *_buf,
> +typedef ssize_t (*tls_write_cb)(struct tls *_ctx, const void *_buf,
> size_t _buflen, void *_cb_arg);
>
> int tls_init(void);
> Index: tls_init.3
> ===
> RCS file: /cvs/src/lib/libtls/tls_init.3,v
> retrieving revision 1.71
> diff -u -p -r1.71 tls_init.3
> --- tls_init.3 4 Sep 2016 16:37:18 - 1.71
> +++ tls_init.3 5 Sep 2016 10:43:43 -
> @@ -189,13 +189,13 @@
> .Ft "int"
> .Fn tls_connect_socket "struct tls *ctx" "int s" "const char *servername"
> .Ft "int"
> -.Fn tls_connect_cbs "struct tls *ctx" "ssize_t (*tls_read_cb)(void *ctx,
> void *buf, size_t buflen, void *cb_arg)" "ssize_t (*tls_write_cb)(void
> *ctx, const void *buf, size_t buflen, void *cb_arg)" "void *cb_arg" "const
> char *servername"
> +.Fn tls_connect_cbs "struct tls *ctx" "ssize_t (*tls_read_cb)(struct tls
> *ctx, void *buf, size_t buflen, void *cb_arg)" "ssize_t
> (*tls_write_cb)(struct tls *ctx, const void *buf, size_t buflen, void
> *cb_arg)" "void *cb_arg" "const char *servername"
> .Ft "int"
> .Fn tls_accept_fds "struct tls *tls" "struct tls **cctx" "int fd_read"
> "int fd_write"
> .Ft "int"
> .Fn tls_accept_socket "struct tls *tls" "struct tls **cctx" "int socket"
> .Ft "int"
> -.Fn tls_accept_cbs "struct tls *ctx" "struct tls **cctx" "ssize_t
> (*tls_read_cb)(void *ctx, void *buf, size_t buflen, void *cb_arg)" "ssize_t
> (*tls_write_cb)(void *ctx, const void *buf, size_t buflen, void *cb_arg)"
> "void *cb_arg"
> +.Fn tls_accept_cbs "struct tls *ctx" "struct tls **cctx" "ssize_t
> (*tls_read_cb)(struct *ctx, void *buf, size_t buflen, void *cb_arg)"
> "ssize_t (*tls_write_cb)(struct tls *ctx, const void *buf, size_t buflen,
> void *cb_arg)" "void *cb_arg"
> .Ft "int"
> .Fn tls_handshake "struct tls *ctx"
> .Ft "ssize_t"
>
Re: hexdump(1): strlen + calloc + snprintf == asprintf
ok beck@ On Sun, Sep 4, 2016 at 9:54 AM, Theo Buehler wrote: > use the libc interface instead of rolling it by hand. > > Index: parse.c > === > RCS file: /var/cvs/src/usr.bin/hexdump/parse.c,v > retrieving revision 1.21 > diff -u -p -r1.21 parse.c > --- parse.c 24 Aug 2016 03:13:45 - 1.21 > +++ parse.c 24 Aug 2016 05:23:47 - > @@ -147,8 +147,7 @@ add(const char *fmt) > for (savep = ++p; *p != '"';) > if (*p++ == 0) > badfmt(fmt); > - tfu->fmt = strndup(savep, p - savep); > - if (tfu->fmt == NULL) > + if ((tfu->fmt = strndup(savep, p - savep)) == NULL) > err(1, NULL); > escape(tfu->fmt); > p++; > @@ -219,7 +218,6 @@ rewrite(FS *fs) > char *p1, *p2; > char savech, *fmtp, cs[4]; > int nconv, prec; > - size_t len; > > nextpr = NULL; > prec = 0; > @@ -408,10 +406,8 @@ rewrite(FS *fs) > */ > savech = *p2; > p1[0] = '\0'; > - len = strlen(fmtp) + strlen(cs) + 1; > - if ((pr->fmt = calloc(1, len)) == NULL) > + if (asprintf(&pr->fmt, "%s%s", fmtp, cs) == -1) > err(1, NULL); > - snprintf(pr->fmt, len, "%s%s", fmtp, cs); > *p2 = savech; > pr->cchar = pr->fmt + (p1 - fmtp); > fmtp = p2; > >
