Re: Explicitly check PF_TRANS_RULESET
On Thu, Jan 18, 2018 at 11:12:59PM -0500, Lawrence Teo wrote: > The pf(4) DIOCX{BEGIN,COMMIT,ROLLBACK} calls support two ruleset types: > PF_TRANS_RULESET and PF_TRANS_TABLE. > > However, their switch statements in pf_ioctl.c only check for > PF_TRANS_TABLE and do not check PF_TRANS_RULESET at all. > > This diff adds explicit checks for PF_TRANS_RULESET to those switch > statements. > > ok? > OK sashan@
Re: Zap PF_TRANS_ALTQ
> > On Fri, Jan 19, 2018 at 08:24:23PM +, Stuart Henderson wrote: > > > To be honest though, unless it's in the way of something, I'm not sure > > > it's > > > worth removing. > > > > If those constatns are in ports and require revision bumps, we can > > leave it in the header. Rebuilding base is one thing, but doing > > manual work with ports is not worth it. > > This seems overblown. > > The PF API is frozen. > > It is only an ABI between the kernel and pfctl -- that is the sticky > and tricky upgrade position. Everything else can must be able to > handle change. I meant to say: the PF API is *NOT* frozen. We can change it at any time! It is just a tiny bit uncomfortable to incompatibly change the bsd:pfctl interface. The timing just has to be right. ports? Who cares!
Re: Zap PF_TRANS_ALTQ
> On Fri, Jan 19, 2018 at 08:24:23PM +, Stuart Henderson wrote: > > To be honest though, unless it's in the way of something, I'm not sure it's > > worth removing. > > If those constatns are in ports and require revision bumps, we can > leave it in the header. Rebuilding base is one thing, but doing > manual work with ports is not worth it. This seems overblown. The PF API is frozen. It is only an ABI between the kernel and pfctl -- that is the sticky and tricky upgrade position. Everything else can must be able to handle change.
Re: Zap PF_TRANS_ALTQ
On 2018/01/19 21:46, Alexander Bluhm wrote: > On Fri, Jan 19, 2018 at 08:24:23PM +, Stuart Henderson wrote: > > To be honest though, unless it's in the way of something, I'm not sure it's > > worth removing. > > If those constatns are in ports and require revision bumps, we can > leave it in the header. Rebuilding base is one thing, but doing > manual work with ports is not worth it. > > bluhm > Nothing showed up in ports (my unpacked source is from May 30 2017 but nothing relevant comes to mind that might have been added since then). So it looks like just one non-ports thing that's providing an interface, but probably nothing actually uses that interface anyway. In which case, should be safe.
Re: Zap PF_TRANS_ALTQ
On Fri, Jan 19, 2018 at 08:24:23PM +, Stuart Henderson wrote: > To be honest though, unless it's in the way of something, I'm not sure it's > worth removing. If those constatns are in ports and require revision bumps, we can leave it in the header. Rebuilding base is one thing, but doing manual work with ports is not worth it. bluhm
Re: Zap PF_TRANS_ALTQ
On 2018/01/19 15:07, Alexandr Nedvedicky wrote: > On Thu, Jan 18, 2018 at 11:15:41PM -0500, Lawrence Teo wrote: > > Nothing uses PF_TRANS_ALTQ anymore, so zap it. > > > > ok? > > I'm fine with removing it. I'm just concerned about potential impact > on port tree as there might be some tool/library still trying to use > PF_TRANS_ALTQ. I just don't know. I'm doing a search for PF_TRANS_ALTQ or PF_TRANS_TABLE now. (Anything using the latter needs to be forced to udpate if the enum changes). Results in a bit. It will cause some pain for https://github.com/dotpy/py-pf/ (not in ports..) To be honest though, unless it's in the way of something, I'm not sure it's worth removing.
Building clang on sparc64
I just committed the bits to start building clang on sparc64. While it is possible to bootstrap clang yourself using the same procedure as we used for other architectures, I really recommend upgrading to a snapshot. Snapshots with clang should become available in a couple of days. Cheers, Mark
Re: Zap PF_TRANS_ALTQ
> On Thu, Jan 18, 2018 at 11:15:41PM -0500, Lawrence Teo wrote: > > Nothing uses PF_TRANS_ALTQ anymore, so zap it. > > > > ok? > > I'm fine with removing it. I'm just concerned about potential impact > on port tree as there might be some tool/library still trying to use > PF_TRANS_ALTQ. I just don't know. > > you might want to ask sthen@ for 'better OK'. The proposal is a binary flag day. Ducks need to be aligned. I'm short on time in the next two weeks. Can we do this later?
Re: Zap PF_TRANS_ALTQ
On Thu, Jan 18, 2018 at 11:15:41PM -0500, Lawrence Teo wrote: > Nothing uses PF_TRANS_ALTQ anymore, so zap it. > > ok? I'm fine with removing it. I'm just concerned about potential impact on port tree as there might be some tool/library still trying to use PF_TRANS_ALTQ. I just don't know. you might want to ask sthen@ for 'better OK'. regards sashan
Re: libc softfloat diff
> From: Jeremie Courreges-Anglas > Date: Fri, 19 Jan 2018 15:28:20 +0100 > > On Fri, Jan 19 2018, Mark Kettenis wrote: > > I thought I had built a snap with armv7, but apparently I didn't. Or > > at least I didn't since I made changes to the whole symbol mess. > > Anyway, the issue is that when building ramdisk code the difference > > between GCC-style inline and ISO-style inline rears its ugly head > > again. The solution is to switch to using "static inline". > > > > ok? > > ok jca@ Sorry, committed it before seeing your mail. > Probably not an issue, but maybe estimateDiv64To32, estimateSqrt32 and > countLeadingZeros32 should be marked __inline as well. Maybe. This code won't be used on armv7 anymore in the future though once we switch to hardfloat. > > Index: lib/libc/softfloat/softfloat-macros.h > > === > > RCS file: /cvs/src/lib/libc/softfloat/softfloat-macros.h,v > > retrieving revision 1.1 > > diff -u -p -r1.1 softfloat-macros.h > > --- lib/libc/softfloat/softfloat-macros.h 6 Nov 2006 15:11:37 - > > 1.1 > > +++ lib/libc/softfloat/softfloat-macros.h 19 Jan 2018 13:25:13 - > > @@ -39,7 +39,7 @@ result will be either 0 or 1, depending > > The result is stored in the location pointed to by `zPtr'. > > > > --- > > */ > > -__inline void shift32RightJamming( bits32 a, int16 count, bits32 *zPtr ) > > +static __inline void shift32RightJamming( bits32 a, int16 count, bits32 > > *zPtr ) > > { > > bits32 z; > > > > @@ -65,7 +65,7 @@ than 64, the result will be 0. The resu > > which are stored at the locations pointed to by `z0Ptr' and `z1Ptr'. > > > > --- > > */ > > -__inline void > > +static __inline void > > shift64Right( > > bits32 a0, bits32 a1, int16 count, bits32 *z0Ptr, bits32 *z1Ptr ) > > { > > @@ -101,7 +101,7 @@ nonzero. The result is broken into two > > the locations pointed to by `z0Ptr' and `z1Ptr'. > > > > --- > > */ > > -__inline void > > +static __inline void > > shift64RightJamming( > > bits32 a0, bits32 a1, int16 count, bits32 *z0Ptr, bits32 *z1Ptr ) > > { > > @@ -153,7 +153,7 @@ corrupted as described above, and is ret > > `z2Ptr'.) > > > > --- > > */ > > -__inline void > > +static __inline void > > shift64ExtraRightJamming( > > bits32 a0, > > bits32 a1, > > @@ -212,7 +212,7 @@ of `count' must be less than 32. The re > > pieces which are stored at the locations pointed to by `z0Ptr' and `z1Ptr'. > > > > --- > > */ > > -__inline void > > +static __inline void > > shortShift64Left( > > bits32 a0, bits32 a1, int16 count, bits32 *z0Ptr, bits32 *z1Ptr ) > > { > > @@ -232,7 +232,7 @@ The value of `count' must be less than 3 > > `z1Ptr', and `z2Ptr'. > > > > --- > > */ > > -__inline void > > +static __inline void > > shortShift96Left( > > bits32 a0, > > bits32 a1, > > @@ -268,7 +268,7 @@ any carry out is lost. The result is br > > are stored at the locations pointed to by `z0Ptr' and `z1Ptr'. > > > > --- > > */ > > -__inline void > > +static __inline void > > add64( > > bits32 a0, bits32 a1, bits32 b0, bits32 b1, bits32 *z0Ptr, bits32 > > *z1Ptr ) > > { > > @@ -289,7 +289,7 @@ modulo 2^96, so any carry out is lost. > > `z1Ptr', and `z2Ptr'. > > > > --- > > */ > > -__inline void > > +static __inline void > > add96( > > bits32 a0, > > bits32 a1, > > @@ -328,7 +328,7 @@ Subtracts the 64-bit value formed by con > > `z1Ptr'. > > > > --- > > */ > > -__inline void > > +static __inline void > > sub64( > > bits32 a0, bits32 a1, bits32 b0, bits32 b1, bits32 *z0Ptr, bits32 > > *z1Ptr ) > > { > > @@ -347,7 +347,7 @@ into three 32-bit pieces which are store > > `z0Ptr', `z1Ptr', and `z2Ptr'. > > > > --- > > */ > > -__inline void > > +static __inline void > > sub96( > > bits32 a0, > > bits32 a1, > > @@ -384,7 +384,8 @@ into two 32-bit pieces which are stored > > `z0Ptr' and `z1Ptr'. > > > > --- > > */ > > -__inline void mul32To64( bits32 a, bits32 b, bits32 *z0Ptr, bits32 *z1Ptr ) > > +static __inline void > > + mul32To64( bits32 a, bits3
Patch to libcrypto's X509_check_(host|email) functions to behave like documented
Hi, I originally created this on github[0], but as it's not part of the portable bits, I've asked to post this on this OpenBSD tech mailing list, I hope this is the correct way of addressing this. The attached patch aims to fix an API difference between OpenSSL and LibreSSL for X509_check_host/X509_check_email and also makes them behave like described in the respective man page. According to the X509_check_host(3) (src/lib/libcrypto/man/X509_check_host.3), "The namelen argument must be the number of characters in the name string or zero, in which case the length is calculated with strlen(name)" for both functions mentioned above. Even though I think this not a good API (I would always make the called pass the exact length and call it a day), this causes compatibility problems with code written for OpenSSL, a good example is MariaDB's SSL certificate hostname check found here: https://github.com/MariaDB/server/blob/10.3/sql-common/client.c#L1825 This change uses the code from OpenSSL's implementation, which on top of calculating the string length also tolerates the caller including terminating NUL in string length. This way, LibreSSL's code should be API compatible with OpenSSL's interface (even though I don't get why it's necessary for an API to correct invalid input, but that's a different story I guess). Quick and dirty example code demonstrating the problem: #include #include #include int main() { X509_NAME *name = X509_NAME_new(); X509 *cert = X509_new(); const char* cn = "test.example.org"; X509_NAME_add_entry_by_NID(name, NID_commonName, MBSTRING_ASC, (unsigned char *)cn, -1, -1, 0); X509_set_subject_name(cert, name); X509_NAME_free(name); printf("Check host without strlen for %s results in %i\n", cn, X509_check_host(cert, cn, 0, 0, 0)); printf("Check host with strlen for %s results in %i\n", cn, X509_check_host(cert, cn, strlen(cn), 0, 0)); } outputs Check host without strlen for test.example.org results in 0 Check host with strlen for test.example.org results in 1 After the patch (as expected): Check host without strlen for test.example.org results in 1 Check host with strlen for test.example.org results in 1 Best, Michael [0]https://github.com/libressl-portable/openbsd/pull/87 -- Michael Gmelin diff --git a/src/lib/libcrypto/x509v3/v3_utl.c b/src/lib/libcrypto/x509v3/v3_utl.c index 04c789922b..58020214fc 100644 --- a/src/lib/libcrypto/x509v3/v3_utl.c +++ b/src/lib/libcrypto/x509v3/v3_utl.c @@ -1015,8 +1015,17 @@ int X509_check_host(X509 *x, const char *chk, size_t chklen, { if (chk == NULL) return -2; - if (memchr(chk, '\0', chklen)) + /* + * Embedded NULs are disallowed, except as the last character of a + * string of length 2 or more (tolerate caller including terminating + * NUL in string length). + */ + if (chklen == 0) + chklen = strlen(chk); + else if (memchr(chk, '\0', chklen > 1 ? chklen - 1 : chklen)) return -2; + if (chklen > 1 && chk[chklen - 1] == '\0') + --chklen; return do_x509_check(x, chk, chklen, flags, GEN_DNS, peername); } @@ -1025,8 +1034,17 @@ int X509_check_email(X509 *x, const char *chk, size_t chklen, { if (chk == NULL) return -2; - if (memchr(chk, '\0', chklen)) + /* + * Embedded NULs are disallowed, except as the last character of a + * string of length 2 or more (tolerate caller including terminating + * NUL in string length). + */ + if (chklen == 0) + chklen = strlen(chk); + else if (memchr(chk, '\0', chklen > 1 ? chklen - 1 : chklen)) return -2; + if (chklen > 1 && chk[chklen - 1] == '\0') + --chklen; return do_x509_check(x, chk, chklen, flags, GEN_EMAIL, NULL); }
Re: httpd: mess with PATH_INFO (again)
On Thu, Jan 18, 2018 at 10:18:29AM +, Stuart Henderson wrote: > I think it should skip redirecting for "". You can't actually issue an > HTTP request for http://example.com (without the trailing slash). > > 'A PATH_INFO of "/" represents a single void path segment.' > > I think that is "http://example.com//";. afaict httpd is doing the > right thing here. > Welp, the maintainer obviously disagrees[1]. From the looks of it, both values, "" or "/", are considered valid by standards. Could we get this or something similar in, so Flask works with httpd? It's still the second biggest Python web framework. [1]: https://github.com/pallets/werkzeug/issues/1240
Re: Zap PF_TRANS_ALTQ
On Thu, Jan 18, 2018 at 11:15:41PM -0500, Lawrence Teo wrote: > Nothing uses PF_TRANS_ALTQ anymore, so zap it. > > ok? This means that people have to recompile their pfctl and some other tools together with the kernel. But that should not prevent code cleanup. OK bluhm@ > Index: pfvar.h > === > RCS file: /cvs/src/sys/net/pfvar.h,v > retrieving revision 1.470 > diff -u -p -r1.470 pfvar.h > --- pfvar.h 29 Dec 2017 17:05:25 - 1.470 > +++ pfvar.h 19 Jan 2018 03:42:56 - > @@ -68,7 +68,7 @@ enum{ PF_INOUT, PF_IN, PF_OUT, PF_FWD } > enum { PF_PASS, PF_DROP, PF_SCRUB, PF_NOSCRUB, PF_NAT, PF_NONAT, > PF_BINAT, PF_NOBINAT, PF_RDR, PF_NORDR, PF_SYNPROXY_DROP, PF_DEFER, > PF_MATCH, PF_DIVERT, PF_RT, PF_AFRT }; > -enum { PF_TRANS_RULESET, PF_TRANS_ALTQ, PF_TRANS_TABLE }; > +enum { PF_TRANS_RULESET, PF_TRANS_TABLE }; > enum { PF_OP_NONE, PF_OP_IRG, PF_OP_EQ, PF_OP_NE, PF_OP_LT, > PF_OP_LE, PF_OP_GT, PF_OP_GE, PF_OP_XRG, PF_OP_RRG }; > enum { PF_CHANGE_NONE, PF_CHANGE_ADD_HEAD, PF_CHANGE_ADD_TAIL,
Re: libc softfloat diff
On Fri, Jan 19 2018, Mark Kettenis wrote: > I thought I had built a snap with armv7, but apparently I didn't. Or > at least I didn't since I made changes to the whole symbol mess. > Anyway, the issue is that when building ramdisk code the difference > between GCC-style inline and ISO-style inline rears its ugly head > again. The solution is to switch to using "static inline". > > ok? ok jca@ Probably not an issue, but maybe estimateDiv64To32, estimateSqrt32 and countLeadingZeros32 should be marked __inline as well. > > Index: lib/libc/softfloat/softfloat-macros.h > === > RCS file: /cvs/src/lib/libc/softfloat/softfloat-macros.h,v > retrieving revision 1.1 > diff -u -p -r1.1 softfloat-macros.h > --- lib/libc/softfloat/softfloat-macros.h 6 Nov 2006 15:11:37 - > 1.1 > +++ lib/libc/softfloat/softfloat-macros.h 19 Jan 2018 13:25:13 - > @@ -39,7 +39,7 @@ result will be either 0 or 1, depending > The result is stored in the location pointed to by `zPtr'. > > --- > */ > -__inline void shift32RightJamming( bits32 a, int16 count, bits32 *zPtr ) > +static __inline void shift32RightJamming( bits32 a, int16 count, bits32 > *zPtr ) > { > bits32 z; > > @@ -65,7 +65,7 @@ than 64, the result will be 0. The resu > which are stored at the locations pointed to by `z0Ptr' and `z1Ptr'. > > --- > */ > -__inline void > +static __inline void > shift64Right( > bits32 a0, bits32 a1, int16 count, bits32 *z0Ptr, bits32 *z1Ptr ) > { > @@ -101,7 +101,7 @@ nonzero. The result is broken into two > the locations pointed to by `z0Ptr' and `z1Ptr'. > > --- > */ > -__inline void > +static __inline void > shift64RightJamming( > bits32 a0, bits32 a1, int16 count, bits32 *z0Ptr, bits32 *z1Ptr ) > { > @@ -153,7 +153,7 @@ corrupted as described above, and is ret > `z2Ptr'.) > > --- > */ > -__inline void > +static __inline void > shift64ExtraRightJamming( > bits32 a0, > bits32 a1, > @@ -212,7 +212,7 @@ of `count' must be less than 32. The re > pieces which are stored at the locations pointed to by `z0Ptr' and `z1Ptr'. > > --- > */ > -__inline void > +static __inline void > shortShift64Left( > bits32 a0, bits32 a1, int16 count, bits32 *z0Ptr, bits32 *z1Ptr ) > { > @@ -232,7 +232,7 @@ The value of `count' must be less than 3 > `z1Ptr', and `z2Ptr'. > > --- > */ > -__inline void > +static __inline void > shortShift96Left( > bits32 a0, > bits32 a1, > @@ -268,7 +268,7 @@ any carry out is lost. The result is br > are stored at the locations pointed to by `z0Ptr' and `z1Ptr'. > > --- > */ > -__inline void > +static __inline void > add64( > bits32 a0, bits32 a1, bits32 b0, bits32 b1, bits32 *z0Ptr, bits32 > *z1Ptr ) > { > @@ -289,7 +289,7 @@ modulo 2^96, so any carry out is lost. > `z1Ptr', and `z2Ptr'. > > --- > */ > -__inline void > +static __inline void > add96( > bits32 a0, > bits32 a1, > @@ -328,7 +328,7 @@ Subtracts the 64-bit value formed by con > `z1Ptr'. > > --- > */ > -__inline void > +static __inline void > sub64( > bits32 a0, bits32 a1, bits32 b0, bits32 b1, bits32 *z0Ptr, bits32 > *z1Ptr ) > { > @@ -347,7 +347,7 @@ into three 32-bit pieces which are store > `z0Ptr', `z1Ptr', and `z2Ptr'. > > --- > */ > -__inline void > +static __inline void > sub96( > bits32 a0, > bits32 a1, > @@ -384,7 +384,8 @@ into two 32-bit pieces which are stored > `z0Ptr' and `z1Ptr'. > > --- > */ > -__inline void mul32To64( bits32 a, bits32 b, bits32 *z0Ptr, bits32 *z1Ptr ) > +static __inline void > + mul32To64( bits32 a, bits32 b, bits32 *z0Ptr, bits32 *z1Ptr ) > { > bits16 aHigh, aLow, bHigh, bLow; > bits32 z0, zMiddleA, zMiddleB, z1; > @@ -415,7 +416,7 @@ which are stored at the locations pointe > `z2Ptr'. > > --- > */ > -__inline void > +static __inline void > mul64By32To96( > bits32 a0, > bits32 a1, > @@ -444,7 +445,7 @@ product. The product is broken into fou > the locations pointed to
Re: relayd and PUT
Hi! On Fri, 2018-01-05 at 00:12 +0100, Alexander Bluhm wrote: > I have commited more regression tests that check the timeout with > unidirectional traffic flow. I could not find an error. In theory > when we have an idle timeout in one direction, relayd checks wheter > there is trafic flowing in the other direction. The tests set the > timeout to 2 seconds and send 5 bytes while sleeping one second > between each byte. The timeout does not trigger. > So it seems that you encounter some corner case. I need more > information. Yes, its a bit harder to trigger. First, currently relayd opens server connection only after first client request finishes. If the first request is long PUT relayd buffers the PUT and the problem doesn't appear. But it triggers another problem, depending on the request size you run out of memory. I have another patch for this. It opens the relay>server socket earlyer and makes the timeout problem even easyer to trigger. I will send it separately. So to get the server connection opened and the timeout to happen you need to do some small GET(or whatever) request and keep the connection open. In my test case I use "GET /; PUT /largefile" > - Do you use http or https? both have the problem > - Do you use persistent connections? yes > - Do you use chunked encoding? no > - Does it only occur with http or also with plain tcp? only http > - Does disabling socket splicing help? the problem happens when libevent code is in use either splicing is disabled or not available(https) > - Does it happen when the connect to the server is slow? > > While testing I saw that with socket splicing the timeout is handled > twice. We get an wakeup from the idle splicing and from libevent > timeout. I think it is sufficient to only use the idle splicing > if it is available. I noticed it too, but it doesn't seem to make things worse. > Does this diff help? This diff doesn't change things. Rivo
Re: VMD: vioscsi - fix large iso support
On Fri, Jan 19, 2018 at 07:35:08PM +1000, David Gwynne wrote: > > > > On 19 Jan 2018, at 4:32 pm, Carlos Cardenas wrote: > > > > Howdy. > > > > Attached is a patch to fix handling images > 4GB support in Linux. I > > confused image size vs blocks to determine when to send UINT32_MAX > > and trigger a READ_CAPACITY_16. > > > > Identified by mlarkin@ > > > > Comments? Ok? > > ok. > > after this you should make another fix though: READ CAPACITY 10 and 16 should > return the last addressable block, not than the number of blocks. > > in other words you need to take 1 from n_blocks before processing it for > scsi. a good exapmle of this is > http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/ic/nvme.c#rev1.55 and > http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/ic/nvme.c.diff?r1=1.54&r2=1.55 > > dlg That's already handled in READ_CAPACITY_* and GET_CONFIGURATION, i.e. _lto4b(dev->n_blocks - 1, r_cap_data->length); Along with checks in READ_* to ensure the requested lba is < n_blocks - 1. Yes...I was bitten by that earlier on in the development of the driver. :-| +--+ Carlos > > > > > +--+ > > Carlos > > Index: vioscsi.c > > === > > RCS file: /home/los/cvs/src/usr.sbin/vmd/vioscsi.c,v > > retrieving revision 1.3 > > diff -u -p -a -u -r1.3 vioscsi.c > > --- vioscsi.c 16 Jan 2018 06:10:45 - 1.3 > > +++ vioscsi.c 19 Jan 2018 06:26:14 - > > @@ -637,12 +637,12 @@ vioscsi_handle_read_capacity(struct vios > > __func__, dev->sz, dev->n_blocks); > > > > /* > > -* determine if size of iso image > UINT32_MAX > > +* determine if num blocks of iso image > UINT32_MAX > > * if it is, set addr to UINT32_MAX (0x) > > * indicating to hosts that READ_CAPACITY_16 should > > * be called to retrieve the full size > > */ > > - if (dev->sz >= UINT32_MAX) { > > + if (dev->n_blocks >= UINT32_MAX) { > > _lto4b(UINT32_MAX, r_cap_data->addr); > > _lto4b(VIOSCSI_BLOCK_SIZE_CDROM, r_cap_data->length); > > log_warnx("%s: ISO sz %lld is bigger than " > > @@ -1603,7 +1603,7 @@ vioscsi_handle_get_config(struct vioscsi > > config_random_read_desc->feature_code); > > config_random_read_desc->byte3 = CONFIG_RANDOM_READ_BYTE3; > > config_random_read_desc->length = CONFIG_RANDOM_READ_LENGTH; > > - if (dev->sz >= UINT32_MAX) > > + if (dev->n_blocks >= UINT32_MAX) > > _lto4b(UINT32_MAX, config_random_read_desc->block_size); > > else > > _lto4b(dev->n_blocks - 1, config_random_read_desc->block_size); >
Re: relayd and PUT
Hi! Please ingore this. Rivo On Fri, 2018-01-19 at 14:01 +, Rivo Nurges wrote: > On Fri, 2018-01-05 at 00:12 +0100, Alexander Bluhm wrote: > > On Wed, Dec 13, 2017 at 07:42:03AM +0100, Claudio Jeker wrote: > > > On Wed, Dec 13, 2017 at 12:25:39AM +, Rivo Nurges wrote: > > > > If you http PUT a "big" file through relayd, server<>relay read > > > > side > > > > will eventually get a EVBUFFER_TIMEOUT. Nothing comes back from > > > > the > > > > server until the PUT is done. I disabled server read timeouts > > > > for > > > > PUT > > > > requests. > > > > > > I have seen something similar and came to the conclusion that the > > > timeout > > > handling of relayd is not correct. As long as traffic is flowing > > > the > > > timeout should be reset (at least that is what every other > > > implementation > > > does). This is not really happening in relayd. I have seen this > > > on > > > GET > > > requests that are huge (timeout hits in the middle of the > > > transimit > > > and > > > kills the session). > > > > I have commited more regression tests that check the timeout with > > unidirectional traffic flow. I could not find an error. In theory > > when we have an idle timeout in one direction, relayd checks wheter > > there is trafic flowing in the other direction. The tests set the > > timeout to 2 seconds and send 5 bytes while sleeping one second > > between each byte. The timeout does not trigger. > > > > So it seems that you encounter some corner case. I need more > > information. > > > > - Do you use http or https? > > - Do you use persistent connections? > > - Do you use chunked encoding? > > - Does it only occur with http or also with plain tcp? > > - Does disabling socket splicing help? > > - Does it happen when the connect to the server is slow? > > > > While testing I saw that with socket splicing the timeout is > > handled > > twice. We get an wakeup from the idle splicing and from libevent > > timeout. I think it is sufficient to only use the idle splicing > > if it is available. > > > > Does this diff help? > > > > bluhm > > > > Index: relay.c > > === > > RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/relayd/relay.c,v > > retrieving revision 1.237 > > diff -u -p -r1.237 relay.c > > --- relay.c 27 Dec 2017 15:53:30 - 1.237 > > +++ relay.c 4 Jan 2018 22:44:20 - > > @@ -733,16 +733,21 @@ relay_connected(int fd, short sig, void > > if ((rlay->rl_conf.flags & F_TLSCLIENT) && (out->tls != > > NULL)) > > relay_tls_connected(out); > > > > - bufferevent_settimeout(bev, > > - rlay->rl_conf.timeout.tv_sec, rlay- > > > rl_conf.timeout.tv_sec); > > > > bufferevent_setwatermark(bev, EV_WRITE, > > RELAY_MIN_PREFETCHED * proto->tcpbufsiz, 0); > > bufferevent_enable(bev, EV_READ|EV_WRITE); > > if (con->se_in.bev) > > bufferevent_enable(con->se_in.bev, EV_READ); > > > > - if (relay_splice(&con->se_out) == -1) > > + switch (relay_splice(&con->se_out)) { > > + case 0: > > + bufferevent_settimeout(bev, > > + rlay->rl_conf.timeout.tv_sec, rlay- > > > rl_conf.timeout.tv_sec); > > > > + break; > > + case -1: > > relay_close(con, strerror(errno)); > > + break; > > + } > > } > > > > void > > @@ -784,14 +789,19 @@ relay_input(struct rsession *con) > > if ((rlay->rl_conf.flags & F_TLS) && con->se_in.tls != > > NULL) > > relay_tls_connected(&con->se_in); > > > > - bufferevent_settimeout(con->se_in.bev, > > - rlay->rl_conf.timeout.tv_sec, rlay- > > > rl_conf.timeout.tv_sec); > > > > bufferevent_setwatermark(con->se_in.bev, EV_WRITE, > > RELAY_MIN_PREFETCHED * proto->tcpbufsiz, 0); > > bufferevent_enable(con->se_in.bev, EV_READ|EV_WRITE); > > > > - if (relay_splice(&con->se_in) == -1) > > + switch (relay_splice(&con->se_in)) { > > + case 0: > > + bufferevent_settimeout(con->se_in.bev, > > + rlay->rl_conf.timeout.tv_sec, rlay- > > > rl_conf.timeout.tv_sec); > > > > + break; > > + case -1: > > relay_close(con, strerror(errno)); > > + break; > > + } > > } > > > > void
Re: relayd and PUT
On Fri, 2018-01-05 at 00:12 +0100, Alexander Bluhm wrote: > On Wed, Dec 13, 2017 at 07:42:03AM +0100, Claudio Jeker wrote: > > On Wed, Dec 13, 2017 at 12:25:39AM +, Rivo Nurges wrote: > > > If you http PUT a "big" file through relayd, server<>relay read > > > side > > > will eventually get a EVBUFFER_TIMEOUT. Nothing comes back from > > > the > > > server until the PUT is done. I disabled server read timeouts for > > > PUT > > > requests. > > > > I have seen something similar and came to the conclusion that the > > timeout > > handling of relayd is not correct. As long as traffic is flowing > > the > > timeout should be reset (at least that is what every other > > implementation > > does). This is not really happening in relayd. I have seen this on > > GET > > requests that are huge (timeout hits in the middle of the transimit > > and > > kills the session). > > I have commited more regression tests that check the timeout with > unidirectional traffic flow. I could not find an error. In theory > when we have an idle timeout in one direction, relayd checks wheter > there is trafic flowing in the other direction. The tests set the > timeout to 2 seconds and send 5 bytes while sleeping one second > between each byte. The timeout does not trigger. > > So it seems that you encounter some corner case. I need more > information. > > - Do you use http or https? > - Do you use persistent connections? > - Do you use chunked encoding? > - Does it only occur with http or also with plain tcp? > - Does disabling socket splicing help? > - Does it happen when the connect to the server is slow? > > While testing I saw that with socket splicing the timeout is handled > twice. We get an wakeup from the idle splicing and from libevent > timeout. I think it is sufficient to only use the idle splicing > if it is available. > > Does this diff help? > > bluhm > > Index: relay.c > === > RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/relayd/relay.c,v > retrieving revision 1.237 > diff -u -p -r1.237 relay.c > --- relay.c 27 Dec 2017 15:53:30 - 1.237 > +++ relay.c 4 Jan 2018 22:44:20 - > @@ -733,16 +733,21 @@ relay_connected(int fd, short sig, void > if ((rlay->rl_conf.flags & F_TLSCLIENT) && (out->tls != > NULL)) > relay_tls_connected(out); > > - bufferevent_settimeout(bev, > - rlay->rl_conf.timeout.tv_sec, rlay- > >rl_conf.timeout.tv_sec); > bufferevent_setwatermark(bev, EV_WRITE, > RELAY_MIN_PREFETCHED * proto->tcpbufsiz, 0); > bufferevent_enable(bev, EV_READ|EV_WRITE); > if (con->se_in.bev) > bufferevent_enable(con->se_in.bev, EV_READ); > > - if (relay_splice(&con->se_out) == -1) > + switch (relay_splice(&con->se_out)) { > + case 0: > + bufferevent_settimeout(bev, > + rlay->rl_conf.timeout.tv_sec, rlay- > >rl_conf.timeout.tv_sec); > + break; > + case -1: > relay_close(con, strerror(errno)); > + break; > + } > } > > void > @@ -784,14 +789,19 @@ relay_input(struct rsession *con) > if ((rlay->rl_conf.flags & F_TLS) && con->se_in.tls != NULL) > relay_tls_connected(&con->se_in); > > - bufferevent_settimeout(con->se_in.bev, > - rlay->rl_conf.timeout.tv_sec, rlay- > >rl_conf.timeout.tv_sec); > bufferevent_setwatermark(con->se_in.bev, EV_WRITE, > RELAY_MIN_PREFETCHED * proto->tcpbufsiz, 0); > bufferevent_enable(con->se_in.bev, EV_READ|EV_WRITE); > > - if (relay_splice(&con->se_in) == -1) > + switch (relay_splice(&con->se_in)) { > + case 0: > + bufferevent_settimeout(con->se_in.bev, > + rlay->rl_conf.timeout.tv_sec, rlay- > >rl_conf.timeout.tv_sec); > + break; > + case -1: > relay_close(con, strerror(errno)); > + break; > + } > } > > void
libc softfloat diff
I thought I had built a snap with armv7, but apparently I didn't. Or at least I didn't since I made changes to the whole symbol mess. Anyway, the issue is that when building ramdisk code the difference between GCC-style inline and ISO-style inline rears its ugly head again. The solution is to switch to using "static inline". ok? Index: lib/libc/softfloat/softfloat-macros.h === RCS file: /cvs/src/lib/libc/softfloat/softfloat-macros.h,v retrieving revision 1.1 diff -u -p -r1.1 softfloat-macros.h --- lib/libc/softfloat/softfloat-macros.h 6 Nov 2006 15:11:37 - 1.1 +++ lib/libc/softfloat/softfloat-macros.h 19 Jan 2018 13:25:13 - @@ -39,7 +39,7 @@ result will be either 0 or 1, depending The result is stored in the location pointed to by `zPtr'. --- */ -__inline void shift32RightJamming( bits32 a, int16 count, bits32 *zPtr ) +static __inline void shift32RightJamming( bits32 a, int16 count, bits32 *zPtr ) { bits32 z; @@ -65,7 +65,7 @@ than 64, the result will be 0. The resu which are stored at the locations pointed to by `z0Ptr' and `z1Ptr'. --- */ -__inline void +static __inline void shift64Right( bits32 a0, bits32 a1, int16 count, bits32 *z0Ptr, bits32 *z1Ptr ) { @@ -101,7 +101,7 @@ nonzero. The result is broken into two the locations pointed to by `z0Ptr' and `z1Ptr'. --- */ -__inline void +static __inline void shift64RightJamming( bits32 a0, bits32 a1, int16 count, bits32 *z0Ptr, bits32 *z1Ptr ) { @@ -153,7 +153,7 @@ corrupted as described above, and is ret `z2Ptr'.) --- */ -__inline void +static __inline void shift64ExtraRightJamming( bits32 a0, bits32 a1, @@ -212,7 +212,7 @@ of `count' must be less than 32. The re pieces which are stored at the locations pointed to by `z0Ptr' and `z1Ptr'. --- */ -__inline void +static __inline void shortShift64Left( bits32 a0, bits32 a1, int16 count, bits32 *z0Ptr, bits32 *z1Ptr ) { @@ -232,7 +232,7 @@ The value of `count' must be less than 3 `z1Ptr', and `z2Ptr'. --- */ -__inline void +static __inline void shortShift96Left( bits32 a0, bits32 a1, @@ -268,7 +268,7 @@ any carry out is lost. The result is br are stored at the locations pointed to by `z0Ptr' and `z1Ptr'. --- */ -__inline void +static __inline void add64( bits32 a0, bits32 a1, bits32 b0, bits32 b1, bits32 *z0Ptr, bits32 *z1Ptr ) { @@ -289,7 +289,7 @@ modulo 2^96, so any carry out is lost. `z1Ptr', and `z2Ptr'. --- */ -__inline void +static __inline void add96( bits32 a0, bits32 a1, @@ -328,7 +328,7 @@ Subtracts the 64-bit value formed by con `z1Ptr'. --- */ -__inline void +static __inline void sub64( bits32 a0, bits32 a1, bits32 b0, bits32 b1, bits32 *z0Ptr, bits32 *z1Ptr ) { @@ -347,7 +347,7 @@ into three 32-bit pieces which are store `z0Ptr', `z1Ptr', and `z2Ptr'. --- */ -__inline void +static __inline void sub96( bits32 a0, bits32 a1, @@ -384,7 +384,8 @@ into two 32-bit pieces which are stored `z0Ptr' and `z1Ptr'. --- */ -__inline void mul32To64( bits32 a, bits32 b, bits32 *z0Ptr, bits32 *z1Ptr ) +static __inline void + mul32To64( bits32 a, bits32 b, bits32 *z0Ptr, bits32 *z1Ptr ) { bits16 aHigh, aLow, bHigh, bLow; bits32 z0, zMiddleA, zMiddleB, z1; @@ -415,7 +416,7 @@ which are stored at the locations pointe `z2Ptr'. --- */ -__inline void +static __inline void mul64By32To96( bits32 a0, bits32 a1, @@ -444,7 +445,7 @@ product. The product is broken into fou the locations pointed to by `z0Ptr', `z1Ptr', `z2Ptr', and `z3Ptr'. --- */ -__inline void +static __inline void mul64To128( bits32 a0, bits32 a1, @@ -597,7 +598,7 @@ equal to the 64-bit value formed by conc returns 0. --- */ -__inline flag eq64( bits32 a0, bits32 a1, bits32 b0, bits32 b1 ) +static __inline flag eq64( bits32
Re: Explicitly check PF_TRANS_RULESET
On Thu, Jan 18, 2018 at 11:12:59PM -0500, Lawrence Teo wrote: > The pf(4) DIOCX{BEGIN,COMMIT,ROLLBACK} calls support two ruleset types: > PF_TRANS_RULESET and PF_TRANS_TABLE. > > However, their switch statements in pf_ioctl.c only check for > PF_TRANS_TABLE and do not check PF_TRANS_RULESET at all. > > This diff adds explicit checks for PF_TRANS_RULESET to those switch > statements. > > ok? OK bluhm@ > Index: pf_ioctl.c > === > RCS file: /cvs/src/sys/net/pf_ioctl.c,v > retrieving revision 1.326 > diff -u -p -U6 -r1.326 pf_ioctl.c > --- pf_ioctl.c28 Nov 2017 16:05:46 - 1.326 > +++ pf_ioctl.c19 Jan 2018 03:40:47 - > @@ -2244,21 +2244,27 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a > free(table, M_TEMP, sizeof(*table)); > free(ioe, M_TEMP, sizeof(*ioe)); > PF_UNLOCK(); > goto fail; > } > break; > - default: > + case PF_TRANS_RULESET: > if ((error = pf_begin_rules(&ioe->ticket, > ioe->anchor))) { > free(table, M_TEMP, sizeof(*table)); > free(ioe, M_TEMP, sizeof(*ioe)); > PF_UNLOCK(); > goto fail; > } > break; > + default: > + free(table, M_TEMP, sizeof(*table)); > + free(ioe, M_TEMP, sizeof(*ioe)); > + error = EINVAL; > + PF_UNLOCK(); > + goto fail; > } > if (copyout(ioe, io->array+i, sizeof(io->array[i]))) { > free(table, M_TEMP, sizeof(*table)); > free(ioe, M_TEMP, sizeof(*ioe)); > error = EFAULT; > PF_UNLOCK(); > @@ -2310,21 +2316,27 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a > free(table, M_TEMP, sizeof(*table)); > free(ioe, M_TEMP, sizeof(*ioe)); > PF_UNLOCK(); > goto fail; /* really bad */ > } > break; > - default: > + case PF_TRANS_RULESET: > if ((error = pf_rollback_rules(ioe->ticket, > ioe->anchor))) { > free(table, M_TEMP, sizeof(*table)); > free(ioe, M_TEMP, sizeof(*ioe)); > PF_UNLOCK(); > goto fail; /* really bad */ > } > break; > + default: > + free(table, M_TEMP, sizeof(*table)); > + free(ioe, M_TEMP, sizeof(*ioe)); > + error = EINVAL; > + PF_UNLOCK(); > + goto fail; /* really bad */ > } > } > free(table, M_TEMP, sizeof(*table)); > free(ioe, M_TEMP, sizeof(*ioe)); > PF_UNLOCK(); > break; > @@ -2370,25 +2382,31 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a > free(ioe, M_TEMP, sizeof(*ioe)); > error = EBUSY; > PF_UNLOCK(); > goto fail; > } > break; > - default: > + case PF_TRANS_RULESET: > rs = pf_find_ruleset(ioe->anchor); > if (rs == NULL || > !rs->rules.inactive.open || > rs->rules.inactive.ticket != > ioe->ticket) { > free(table, M_TEMP, sizeof(*table)); > free(ioe, M_TEMP, sizeof(*ioe)); > error = EBUSY; > PF_UNLOCK(); > goto fail; > } > break; > + default: > + free(table, M_TEMP, sizeof(*table)); > +
Re: Basic SHA3 support (cryptographic discussion)
On Wed, Jan 17, 2018 at 10:20:50PM +0100, Christian Weisgerber wrote: > What do you want to USE your SHA-3 implementation for? I would like to have a sha3 command line tool. Just to have it there and start using it. For example adding it to ports distfiles would be trivial. Yes, general protocol transition will be hard. And it will not happen before sha2 is broken. And we will live with broken hashes for a long time. But I think this is not an argument against sha3. Of course there is the question whether this should be in libc and /bin. But this is independent of adding sha3 and even more relevant for broken md5 and sha1. So I would add one sha3 tool there for consistency. If we feel it does not belong there and it hurts, all the hashes could be moved. bluhm
Re: VMD: vioscsi - fix large iso support
> On 19 Jan 2018, at 4:32 pm, Carlos Cardenas wrote: > > Howdy. > > Attached is a patch to fix handling images > 4GB support in Linux. I > confused image size vs blocks to determine when to send UINT32_MAX > and trigger a READ_CAPACITY_16. > > Identified by mlarkin@ > > Comments? Ok? ok. after this you should make another fix though: READ CAPACITY 10 and 16 should return the last addressable block, not than the number of blocks. in other words you need to take 1 from n_blocks before processing it for scsi. a good exapmle of this is http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/ic/nvme.c#rev1.55 and http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/ic/nvme.c.diff?r1=1.54&r2=1.55 dlg > > +--+ > Carlos > Index: vioscsi.c > === > RCS file: /home/los/cvs/src/usr.sbin/vmd/vioscsi.c,v > retrieving revision 1.3 > diff -u -p -a -u -r1.3 vioscsi.c > --- vioscsi.c 16 Jan 2018 06:10:45 - 1.3 > +++ vioscsi.c 19 Jan 2018 06:26:14 - > @@ -637,12 +637,12 @@ vioscsi_handle_read_capacity(struct vios > __func__, dev->sz, dev->n_blocks); > > /* > - * determine if size of iso image > UINT32_MAX > + * determine if num blocks of iso image > UINT32_MAX >* if it is, set addr to UINT32_MAX (0x) >* indicating to hosts that READ_CAPACITY_16 should >* be called to retrieve the full size >*/ > - if (dev->sz >= UINT32_MAX) { > + if (dev->n_blocks >= UINT32_MAX) { > _lto4b(UINT32_MAX, r_cap_data->addr); > _lto4b(VIOSCSI_BLOCK_SIZE_CDROM, r_cap_data->length); > log_warnx("%s: ISO sz %lld is bigger than " > @@ -1603,7 +1603,7 @@ vioscsi_handle_get_config(struct vioscsi > config_random_read_desc->feature_code); > config_random_read_desc->byte3 = CONFIG_RANDOM_READ_BYTE3; > config_random_read_desc->length = CONFIG_RANDOM_READ_LENGTH; > - if (dev->sz >= UINT32_MAX) > + if (dev->n_blocks >= UINT32_MAX) > _lto4b(UINT32_MAX, config_random_read_desc->block_size); > else > _lto4b(dev->n_blocks - 1, config_random_read_desc->block_size);
rasops(9): Fix underline rotation on CCW rotated screens
Hi tech@, Fix underline rotation on CCW (quarter counter-clockwise) rotated screens. Currently, the "underline" is actually drawn above text. Comments? OK? Index: sys/dev/rasops/rasops.c === RCS file: /cvs/src/sys/dev/rasops/rasops.c,v retrieving revision 1.49 diff -u -p -r1.49 rasops.c --- sys/dev/rasops/rasops.c 23 Dec 2017 10:30:25 - 1.49 +++ sys/dev/rasops/rasops.c 17 Jan 2018 17:35:49 - @@ -1247,6 +1247,8 @@ rasops_putchar_rotated(void *cookie, int /* Do rotated underline */ rp = ri->ri_bits + col * ri->ri_yscale + row * ri->ri_xscale; + if (ri->ri_flg & RI_ROTATE_CCW) + rp += (ri->ri_font->fontwidth - 1) * ri->ri_pelbytes; height = ri->ri_font->fontheight; /* XXX this assumes 16-bit color depth */
Re: httpd: mess with PATH_INFO (again)
The WSGI PEP states the following: > PATH_INFO > The remainder of the request URL's "path", designating the virtual > "location" of the request's target within the application. This may be > an empty string, if the request URL targets the application root and > does not have a trailing slash. Seems that both values are valid (at least within the CGI portion that WSGI uses). Ugh, well Werkzeug probably shouldn't redirect, either. I'll see if I can push the PR [2]. [1]: https://www.python.org/dev/peps/pep-/#environ-variables [2]: https://github.com/pallets/werkzeug/issues/1240
Re: VMD: vioscsi - fix large iso support
On Thu, Jan 18, 2018 at 10:32:31PM -0800, Carlos Cardenas wrote: > Howdy. > > Attached is a patch to fix handling images > 4GB support in Linux. I > confused image size vs blocks to determine when to send UINT32_MAX > and trigger a READ_CAPACITY_16. > > Identified by mlarkin@ > > Comments? Ok? > > +--+ > Carlos ok mlarkin > Index: vioscsi.c > === > RCS file: /home/los/cvs/src/usr.sbin/vmd/vioscsi.c,v > retrieving revision 1.3 > diff -u -p -a -u -r1.3 vioscsi.c > --- vioscsi.c 16 Jan 2018 06:10:45 - 1.3 > +++ vioscsi.c 19 Jan 2018 06:26:14 - > @@ -637,12 +637,12 @@ vioscsi_handle_read_capacity(struct vios > __func__, dev->sz, dev->n_blocks); > > /* > - * determine if size of iso image > UINT32_MAX > + * determine if num blocks of iso image > UINT32_MAX >* if it is, set addr to UINT32_MAX (0x) >* indicating to hosts that READ_CAPACITY_16 should >* be called to retrieve the full size >*/ > - if (dev->sz >= UINT32_MAX) { > + if (dev->n_blocks >= UINT32_MAX) { > _lto4b(UINT32_MAX, r_cap_data->addr); > _lto4b(VIOSCSI_BLOCK_SIZE_CDROM, r_cap_data->length); > log_warnx("%s: ISO sz %lld is bigger than " > @@ -1603,7 +1603,7 @@ vioscsi_handle_get_config(struct vioscsi > config_random_read_desc->feature_code); > config_random_read_desc->byte3 = CONFIG_RANDOM_READ_BYTE3; > config_random_read_desc->length = CONFIG_RANDOM_READ_LENGTH; > - if (dev->sz >= UINT32_MAX) > + if (dev->n_blocks >= UINT32_MAX) > _lto4b(UINT32_MAX, config_random_read_desc->block_size); > else > _lto4b(dev->n_blocks - 1, config_random_read_desc->block_size);
Re: httpd: mess with PATH_INFO (again)
On Fri, Jan 19, 2018 at 08:29:13AM +0100, Ve Telko wrote: > Hi, > > problem is in your framework, server handles this correctly. > > Using PHP, PATH_INFO and > REQUEST_URI is always "/" > > for URLs like > http://foo.bar > http://foo.bar/ > http://foo.bar// > > Ve. > It's not over here, and I logged it from httpd, so it's already empty on httpd's side.
Re: httpd: mess with PATH_INFO (again)
Hi, problem is in your framework, server handles this correctly. Using PHP, PATH_INFO and REQUEST_URI is always "/" for URLs like http://foo.bar http://foo.bar/ http://foo.bar// Ve.