Re: relayd - HTTP Desync Attacks
Patch inlined
Best regards
Index: usr.sbin/relayd/http.h
===
RCS file: /cvs/src/usr.sbin/relayd/http.h,v
retrieving revision 1.11
diff -u -p -u -r1.11 http.h
--- usr.sbin/relayd/http.h 8 May 2019 23:22:19 - 1.11
+++ usr.sbin/relayd/http.h 19 Aug 2019 19:35:54 -
@@ -239,7 +239,6 @@ struct http_descriptor {
char *http_host;
enum httpmethod http_method;
- int http_chunked;
char *http_version;
unsigned int http_status;
Index: usr.sbin/relayd/relay_http.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
retrieving revision 1.78
diff -u -p -u -r1.78 relay_http.c
--- usr.sbin/relayd/relay_http.c 13 Jul 2019 06:53:00 - 1.78
+++ usr.sbin/relayd/relay_http.c 19 Aug 2019 19:35:54 -
@@ -159,7 +159,7 @@ relay_read_http(struct bufferevent *bev,
int action, unique, ret;
const char *errstr;
size_t size, linelen;
- struct kv *hdr = NULL;
+ struct kv *hdr = NULL, lookup;
struct kv *upgrade = NULL, *upgrade_ws = NULL;
getmonotime(&con->se_tv_last);
@@ -219,8 +219,7 @@ relay_read_http(struct bufferevent *bev,
}
/* Append line to the last header, if present */
- if (kv_extend(&desc->http_headers,
-desc->http_lastheader, line) == NULL) {
+ if (kv_extend(desc->http_lastheader, line) == NULL) {
free(line);
goto fail;
}
@@ -279,7 +278,6 @@ relay_read_http(struct bufferevent *bev,
DPRINTF("http_version %s http_rescode %s "
"http_resmesg %s", desc->http_version,
desc->http_rescode, desc->http_resmesg);
- goto lookup;
} else if (cre->line == 1 && cre->dir == RELAY_DIR_REQUEST) {
if ((desc->http_method = relay_httpmethod_byname(key))
== HTTP_METHOD_NONE) {
@@ -360,11 +358,7 @@ relay_read_http(struct bufferevent *bev,
goto abort;
}
}
- lookup:
- if (strcasecmp("Transfer-Encoding", key) == 0 &&
-strcasecmp("chunked", value) == 0)
- desc->http_chunked = 1;
-
+
/* The following header should only occur once */
if (strcasecmp("Host", key) == 0) {
unique = 1;
@@ -516,10 +510,16 @@ relay_read_http(struct bufferevent *bev,
bev->readcb = relay_read_http;
break;
}
- if (desc->http_chunked) {
- /* Chunked transfer encoding */
- cre->toread = TOREAD_HTTP_CHUNK_LENGTH;
- bev->readcb = relay_read_httpchunks;
+
+ lookup.kv_key = "Transfer-Encoding";
+ hdr = kv_find(&desc->http_headers, &lookup);
+ if (hdr != NULL && hdr->kv_value != NULL) {
+ value = hdr->kv_value + strspn(hdr->kv_value, " \t\r\n");
+ if (strcasecmp("chunked", value) == 0) {
+ /* Chunked transfer encoding */
+ cre->toread = TOREAD_HTTP_CHUNK_LENGTH;
+ bev->readcb = relay_read_httpchunks;
+ }
}
/*
@@ -772,7 +772,6 @@ relay_reset_http(struct ctl_relay_event
relay_httpdesc_free(desc);
desc->http_method = 0;
- desc->http_chunked = 0;
cre->headerlen = 0;
cre->line = 0;
cre->done = 0;
Index: usr.sbin/relayd/relayd.c
===
RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
retrieving revision 1.180
diff -u -p -u -r1.180 relayd.c
--- usr.sbin/relayd/relayd.c 26 Jun 2019 12:13:47 - 1.180
+++ usr.sbin/relayd/relayd.c 19 Aug 2019 19:35:54 -
@@ -713,7 +713,7 @@ kv_delete(struct kvtree *keys, struct kv
}
struct kv *
-kv_extend(struct kvtree *keys, struct kv *kv, char *value)
+kv_extend(struct kv *kv, char *value)
{
char *newvalue;
Index: usr.sbin/relayd/relayd.h
===
RCS file: /cvs/src/usr.sbin/relayd/relayd.h,v
retrieving revision 1.259
diff -u -p -u -r1.259 relayd.h
--- usr.sbin/relayd/relayd.h 26 Jun 2019 12:13:47 - 1.259
+++ usr.sbin/relayd/relayd.h 19 Aug 2019 19:35:55 -
@@ -1349,7 +1349,7 @@ struct kv *kv_add(struct kvtree *, char
int kv_set(struct kv *, char *, ...);
int kv_setkey(struct kv *, char *, ...);
void kv_delete(struct kvtree *, struct kv *);
-struct kv *kv_extend(struct kvtree *, struct kv *, char *);
+struct kv *kv_extend(struct kv *, char *);
void kv_purge(struct kvtree *);
void kv_free(struct kv *);
struct kv *kv_inherit(struct kv *, struct kv *);
On Wed, Aug 14, 2019 at 12:40 PM Pablo Caballero wrote:
> "In order to achieve this, the function kv_extend should be modified in
> order to alter..." Wrong! The header (already added to desc->http_headers)
> gets modified correctly through the desc->http_lastheader pointer.
>
> I'm going to stop making assumptions... I have to get my hands on a
> working OBSD setup and make some real test.
>
> Best regards
>
> On Wed, Aug 14, 2019 at 2:27 AM Pablo Caballero wrote:
>
>> Hi guys! I've been reading about HTTP Desync Attacks lately so I took a
>> look to relayd's source
Re: relayd - HTTP Desync Attacks
"In order to achieve this, the function kv_extend should be modified in
order to alter..." Wrong! The header (already added to desc->http_headers)
gets modified correctly through the desc->http_lastheader pointer.
I'm going to stop making assumptions... I have to get my hands on a working
OBSD setup and make some real test.
Best regards
On Wed, Aug 14, 2019 at 2:27 AM Pablo Caballero wrote:
> Hi guys! I've been reading about HTTP Desync Attacks lately so I took a
> look to relayd's source code to check if it's likely to be exploited.
> I wasn't able to do a POC (I don't have a OBSD installation at the moment)
> but I think it is.
> I'm going to install a OpenBSD as soon as I can in order to test the
> current code and send a patch if needed but I want to tell you what I've
> seen so far.
> There I go:
>
> If a malicious user split a "Transfer-Encoding: chunked" http header in
> multiple lines along with a Content-Length header it would trick relayd to
> use the content length header while forwarding the
> Transfer-Encoding header downstream.
> The problem is in relay_read_http:
>
> if (strcasecmp("Transfer-Encoding", key) == 0 &&
> strcasecmp("chunked", value) == 0)
> desc->http_chunked = 1;
>
> The check for key == "Transfer-Encoding" && value == chunked isn't
> deferred until the full header is read and it should.
> Maybe we could defer the check until we are out of the loop.
>
> Replacing
> if (desc->http_chunked) {
> /* Chunked transfer encoding */
> cre->toread = TOREAD_HTTP_CHUNK_LENGTH;
> bev->readcb = relay_read_httpchunks;
> }
>
> by doing a lookup into desc->http_headers
>
> In order to achieve this, the function kv_extend should be modified in
> order to alter the structure pointed by the first parameter (today it does
> nothing with the first parameter)
>
> Also, It seems like the "lookup" label along with the goto lookup sentence
> could be removed.
>
> Payload
> POST ...
> ...
> Transfer-Encoding:
> chunked
> ...
> Content-Length: whatever greater than the chunk
>
> 10
> 1234567890123456
> POISON!
>
> If I'm right this payload would result in relayd honouring the
> Content-Length header (breaking RFC 2616) and probably poisoning the socket
> (assuming that the downstream server would honour the Transfer-Encoding
> header)
>
> PD: I haven't digged enough to realize if relayd reuse downstream
> connections among different clients so I can't say the real impact of this
> issue.
>
> Best regards
>
> Pablo
>
relayd - HTTP Desync Attacks
Hi guys! I've been reading about HTTP Desync Attacks lately so I took a
look to relayd's source code to check if it's likely to be exploited.
I wasn't able to do a POC (I don't have a OBSD installation at the moment)
but I think it is.
I'm going to install a OpenBSD as soon as I can in order to test the
current code and send a patch if needed but I want to tell you what I've
seen so far.
There I go:
If a malicious user split a "Transfer-Encoding: chunked" http header in
multiple lines along with a Content-Length header it would trick relayd to
use the content length header while forwarding the
Transfer-Encoding header downstream.
The problem is in relay_read_http:
if (strcasecmp("Transfer-Encoding", key) == 0 &&
strcasecmp("chunked", value) == 0)
desc->http_chunked = 1;
The check for key == "Transfer-Encoding" && value == chunked isn't deferred
until the full header is read and it should.
Maybe we could defer the check until we are out of the loop.
Replacing
if (desc->http_chunked) {
/* Chunked transfer encoding */
cre->toread = TOREAD_HTTP_CHUNK_LENGTH;
bev->readcb = relay_read_httpchunks;
}
by doing a lookup into desc->http_headers
In order to achieve this, the function kv_extend should be modified in
order to alter the structure pointed by the first parameter (today it does
nothing with the first parameter)
Also, It seems like the "lookup" label along with the goto lookup sentence
could be removed.
Payload
POST ...
...
Transfer-Encoding:
chunked
...
Content-Length: whatever greater than the chunk
10
1234567890123456
POISON!
If I'm right this payload would result in relayd honouring the
Content-Length header (breaking RFC 2616) and probably poisoning the socket
(assuming that the downstream server would honour the Transfer-Encoding
header)
PD: I haven't digged enough to realize if relayd reuse downstream
connections among different clients so I can't say the real impact of this
issue.
Best regards
Pablo
man uath(4) correctness
Index: uath.4 === RCS file: /cvs/src/share/man/man4/uath.4,v retrieving revision 1.24 diff -u -r1.24 uath.4 --- uath.4 3 Sep 2011 22:59:08 - 1.24 +++ uath.4 22 Oct 2011 19:11:06 - @@ -61,8 +61,6 @@ .Pp .Nm supports hardware WEP. -Wired Equivalent Privacy (WEP) is the de facto encryption standard -for wireless networks. It can be typically configured in one of three modes: no encryption; 40-bit encryption; or 104-bit encryption. Unfortunately, due to serious weaknesses in WEP protocol Regards
Re: Toshiba M30 ACPI support
On Tue, Jun 14, 2011 at 6:38 AM, Javier Vazquez wrote:
> Hello Everybody,
>
>I installed OpenBSD 4.9 current in my old Toshiba M30. Although,
>there was not LCD brightness control, so I started to do something
about
>it, taking into account the Toshiba acpi support of FreeBSD.
>http://fxr.watson.org/fxr/source/dev/acpi_support/acpi_toshiba.c
>
>Successfully, I can manage the LCD brightness level using
wsconsctl(8).
>
>wsconsctl display.brightness=100. ;) works nicely by command
>line as well by setting the brightness level in /etc/wsconsctl.conf
file.
>
>I attached the required patch files for testing. I hope can be
>useful for other guys using toshiba laptops as I added the
>Libretto and Dynabook models.
>
>Thanks to Paul for coaching me to provide a decent patch :).
>
> Greatings,
> Javier.
> /*-
> * Copyright (c) 2003 Hiroyuki Aizu
> * All rights reserved.
> *
> * Redistribution and use in source and binary forms, with or without
> * modification, are permitted provided that the following conditions
> * are met:
> * 1. Redistributions of source code must retain the above copyright
> *notice, this list of conditions and the following disclaimer.
> * 2. Redistributions in binary form must reproduce the above copyright
> *notice, this list of conditions and the following disclaimer in the
> *documentation and/or other materials provided with the distribution.
> *
> * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
PURPOSE
> * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
CONSEQUENTIAL
> * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
STRICT
> * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY
WAY
> * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> * SUCH DAMAGE.
> *
> */
> #include
> #include
> #include
>
> #include
> #include
> #include
> #include
> #include
>
> #include
> #include
>
> /*
> * Toshiba HCI interface definitions
> *
> * HCI is Toshiba's "Hardware Control Interface" which is supposed to
> * be uniform across all their models. Ideally we would just call
> * dedicated ACPI methods instead of using this primitive interface.
> * However, the ACPI methods seem to be incomplete in some areas (for
> * example they allow setting, but not reading, the LCD brightness
> * value), so this is still useful.
> */
> #define METHOD_HCI "GHCI"
> #define METHOD_HCI_ENABLE "ENAB"
>
> /* Operations */
> #define HCI_SET 0xFF00
> #define HCI_GET 0xFE00
>
> /* Functions */
> #define HCI_REG_SYSTEM_EVENT0x0016
> #define HCI_REG_VIDEO_OUTPUT0x001C
> #define HCI_REG_LCD_BRIGHTNESS 0x002A
>
> /* Field definitions */
> #define HCI_LCD_BRIGHTNESS_BITS 3
> #define HCI_LCD_BRIGHTNESS_SHIFT(16 - HCI_LCD_BRIGHTNESS_BITS)
> #define HCI_LCD_BRIGHTNESS_MAX ((1 << HCI_LCD_BRIGHTNESS_BITS) -
1)
> #define HCI_LCD_BRIGHTNESS_MIN 0
> #define HCI_VIDEO_OUTPUT_FLAG 0x0100
> #define HCI_VIDEO_OUTPUT_CYCLE_MIN 0
> #define HCI_VIDEO_OUTPUT_CYCLE_MAX 7
>
> /* HCI register definitions */
> #define HCI_WORDS 6 /* Number of register */
> #define HCI_REG_AX 0 /* Operation, then return value
*/
> #define HCI_REG_BX 1 /* Function */
> #define HCI_REG_CX 2 /* Argument (in or out) */
>
> /* Return codes */
> #define HCI_FAILURE-1
> #define HCI_SUCCESS 0
>
> /* Toshiba fn_keys events */
> #define FN_KEY_VIDEO_OUTPUT 0x01BF
> #define FN_KEY_BRIGHTNESS_DOWN 0x01C0
> #define FN_KEY_BRIGHTNESS_UP0x01C1
>
> struct acpitoshiba_softc {
>struct devicesc_dev;
>struct acpi_softc *sc_acpi;
>struct aml_node *sc_devnode;
> };
>
> int toshiba_enable_events(struct acpitoshiba_softc *);
> int toshiba_read_events(struct acpitoshiba_softc *);
> int toshiba_match(struct device *, void *, void *);
> voidtoshiba_attach(struct device *, struct device *, void *);
> int toshiba_hotkey(struct aml_node *, int, void *);
> int toshiba_get_brightness(struct acpitoshiba_softc *, u_int32_t *);
> int toshiba_set_brightness(struct acpitoshiba_softc *, u_int32_t *);
> int toshiba_get_video_output(struct acpitoshiba_softc *, u_int32_t *);
> int toshiba_set_video_ou
kill.c correctness
"if (!*argv || *ep)" should be "if (!**argv || *ep)" but !**argv is already granted by !isdigit(**argv) Index: kill.c === RCS file: /cvs/src/bin/kill/kill.c,v retrieving revision 1.9 diff -u -r1.9 kill.c --- kill.c 27 Oct 2009 23:59:21 - 1.9 +++ kill.c 6 Jun 2011 04:18:54 - @@ -65,7 +65,7 @@ if (!isdigit(**argv)) usage(); numsig = strtol(*argv, &ep, 10); - if (!*argv || *ep) + if (*ep) errx(1, "illegal signal number: %s", *argv); if (numsig >= 128) numsig -= 128;
