Re: relayd websocket

2019-05-08 Thread Sebastian Benoit


ok benno@

Reyk Floeter(r...@openbsd.org) on 2019.05.08 20:35:46 +0200:
> On Wed, May 08, 2019 at 07:07:43PM +0200, Reyk Floeter wrote:
> > On Wed, May 08, 2019 at 06:26:45PM +0200, Reyk Floeter wrote:
> > > On Wed, Mar 06, 2019 at 05:36:32PM +0100, Sebastian Benoit wrote:
> > > > Rivo Nurges(rivo.nur...@smit.ee) on 2019.03.05 22:42:13 +:
> > > > > Hi!
> > > > > 
> > > > > On 3/5/19 10:36 PM, Claudio Jeker wrote:
> > > > > > I guess that this would need strcasestr() instead of strcasecmp(), 
> > > > > > since you
> > > > > > are looking for the substring "Upgrade" in value. Maybe more is 
> > > > > > needed if
> > > > > > we want to be sure that 'Connection: Upgrade-maybe' does not match.
> > > > > 
> > > > > You are correct about strcasestr. "Connection: Upgrade-maybe" would 
> > > > > need 
> > > > > to have correct "Upgrade: websocket". Anyway, lets be strict.
> > > > > 
> > > > > Does something like this make sense?
> > > > 
> > > > i think the seperator list needs to include '\t' 
> > > > because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.
> > > > 
> > > > And i dont think you can mix "," with " \t" seperators,
> > > > because otherwise "Foo Upgrade, Bar" will match.
> > > > 
> > > > Something more is needed to parse elements of a header.
> > > >  
> > > 
> > > I would reshuffle the websocket handling a bit as I don't think that
> > > we need the http priv struct.  We can lookup the parsed headers later.
> > > 
> > > The attached diff moves it all into one places and introduces a new
> > > function kv_find_value() that is able to to match headers with
> > > multiple values (I think we might need it elsewhere.  And even if not,
> > > I would prefer to have this in a function intead of stuffing it into
> > > relay_read_http).
> > > 
> > > A minor question is if the lookup should be done before or after
> > > applying the filters (relay_test - my diff does it afterwards, the
> > > current code does it before).
> > > 
> > > Tests?  Comments?  Ok?
> > > 
> > 
> > I keep on replying to my own diffs...  the updated one below uses
> > strcasecmp instead of strcasestr in kv_find_value().  There is no need
> > for substring- or fn- matching in this function.
> > 
> 
> Next one...
> 
> benno@ pointed out that the RFC allows whitespace before the comma.
> We also don't have to strip \r\n as it is done by relayd's kv parser.
> 
> OK?
> 
> Reyk
> 
> Index: usr.sbin/relayd/http.h
> ===
> RCS file: /cvs/src/usr.sbin/relayd/http.h,v
> retrieving revision 1.10
> diff -u -p -u -p -r1.10 http.h
> --- usr.sbin/relayd/http.h4 Mar 2019 21:25:03 -   1.10
> +++ usr.sbin/relayd/http.h8 May 2019 18:34:09 -
> @@ -251,10 +251,4 @@ struct http_descriptor {
>   struct kvtreehttp_headers;
>  };
>  
> -struct relay_http_priv {
> -#define HTTP_CONNECTION_UPGRADE  0x01
> -#define HTTP_UPGRADE_WEBSOCKET   0x02
> - int  http_upgrade_req;
> -};
> -
>  #endif /* HTTP_H */
> Index: usr.sbin/relayd/relay.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
> retrieving revision 1.242
> diff -u -p -u -p -r1.242 relay.c
> --- usr.sbin/relayd/relay.c   4 Mar 2019 21:25:03 -   1.242
> +++ usr.sbin/relayd/relay.c   8 May 2019 18:34:09 -
> @@ -1410,13 +1410,7 @@ relay_session(struct rsession *con)
>   return;
>   }
>  
> - if (rlay->rl_proto->type == RELAY_PROTO_HTTP) {
> - if (relay_http_priv_init(con) == -1) {
> - relay_close(con,
> - "failed to allocate http session data", 1);
> - return;
> - }
> - } else {
> + if (rlay->rl_proto->type != RELAY_PROTO_HTTP) {
>   if (rlay->rl_conf.fwdmode == FWD_TRANS)
>   relay_bindanyreq(con, 0, IPPROTO_TCP);
>   else if (relay_connect(con) == -1) {
> Index: usr.sbin/relayd/relay_http.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
> retrieving revision 1.72
> diff -u -p -u -p -r1.72 relay_http.c
> --- usr.sbin/relayd/relay_http.c  4 Mar 2019 21:25:03 -   1.72
> +++ usr.sbin/relayd/relay_http.c  8 May 2019 18:34:09 -
> @@ -110,17 +110,6 @@ relay_http_init(struct relay *rlay)
>  }
>  
>  int
> -relay_http_priv_init(struct rsession *con)
> -{
> - struct relay_http_priv  *p;
> - if ((p = calloc(1, sizeof(struct relay_http_priv))) == NULL)
> - return (-1);
> -
> - con->se_priv = p;
> - return (0);
> -}
> -
> -int
>  relay_httpdesc_init(struct ctl_relay_event *cre)
>  {
>   struct http_descriptor  *desc;
> @@ -163,13 +152,13 @@ relay_read_http(struct bufferevent *bev,
>   struct relay*rlay = con->se_relay;
>   struct protocol *proto = rlay->rl_proto;
>   struct 

Re: relayd websocket

2019-05-08 Thread Rivo Nurges
Hi!

Seems to work fine.

Rivo

On 2019-05-08 21:37, Reyk Floeter wrote:
> On Wed, May 08, 2019 at 07:07:43PM +0200, Reyk Floeter wrote:
>> On Wed, May 08, 2019 at 06:26:45PM +0200, Reyk Floeter wrote:
>>> On Wed, Mar 06, 2019 at 05:36:32PM +0100, Sebastian Benoit wrote:
 Rivo Nurges(rivo.nur...@smit.ee) on 2019.03.05 22:42:13 +:
> Hi!
>
> On 3/5/19 10:36 PM, Claudio Jeker wrote:
>> I guess that this would need strcasestr() instead of strcasecmp(), since 
>> you
>> are looking for the substring "Upgrade" in value. Maybe more is needed if
>> we want to be sure that 'Connection: Upgrade-maybe' does not match.
>
> You are correct about strcasestr. "Connection: Upgrade-maybe" would need
> to have correct "Upgrade: websocket". Anyway, lets be strict.
>
> Does something like this make sense?

 i think the seperator list needs to include '\t'
 because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.

 And i dont think you can mix "," with " \t" seperators,
 because otherwise "Foo Upgrade, Bar" will match.

 Something more is needed to parse elements of a header.
   
>>>
>>> I would reshuffle the websocket handling a bit as I don't think that
>>> we need the http priv struct.  We can lookup the parsed headers later.
>>>
>>> The attached diff moves it all into one places and introduces a new
>>> function kv_find_value() that is able to to match headers with
>>> multiple values (I think we might need it elsewhere.  And even if not,
>>> I would prefer to have this in a function intead of stuffing it into
>>> relay_read_http).
>>>
>>> A minor question is if the lookup should be done before or after
>>> applying the filters (relay_test - my diff does it afterwards, the
>>> current code does it before).
>>>
>>> Tests?  Comments?  Ok?
>>>
>>
>> I keep on replying to my own diffs...  the updated one below uses
>> strcasecmp instead of strcasestr in kv_find_value().  There is no need
>> for substring- or fn- matching in this function.
>>
> 
> Next one...
> 
> benno@ pointed out that the RFC allows whitespace before the comma.
> We also don't have to strip \r\n as it is done by relayd's kv parser.
> 
> OK?
> 
> Reyk
> 
> Index: usr.sbin/relayd/http.h
> ===
> RCS file: /cvs/src/usr.sbin/relayd/http.h,v
> retrieving revision 1.10
> diff -u -p -u -p -r1.10 http.h
> --- usr.sbin/relayd/http.h4 Mar 2019 21:25:03 -   1.10
> +++ usr.sbin/relayd/http.h8 May 2019 18:34:09 -
> @@ -251,10 +251,4 @@ struct http_descriptor {
>   struct kvtreehttp_headers;
>   };
>   
> -struct relay_http_priv {
> -#define HTTP_CONNECTION_UPGRADE  0x01
> -#define HTTP_UPGRADE_WEBSOCKET   0x02
> - int  http_upgrade_req;
> -};
> -
>   #endif /* HTTP_H */
> Index: usr.sbin/relayd/relay.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
> retrieving revision 1.242
> diff -u -p -u -p -r1.242 relay.c
> --- usr.sbin/relayd/relay.c   4 Mar 2019 21:25:03 -   1.242
> +++ usr.sbin/relayd/relay.c   8 May 2019 18:34:09 -
> @@ -1410,13 +1410,7 @@ relay_session(struct rsession *con)
>   return;
>   }
>   
> - if (rlay->rl_proto->type == RELAY_PROTO_HTTP) {
> - if (relay_http_priv_init(con) == -1) {
> - relay_close(con,
> - "failed to allocate http session data", 1);
> - return;
> - }
> - } else {
> + if (rlay->rl_proto->type != RELAY_PROTO_HTTP) {
>   if (rlay->rl_conf.fwdmode == FWD_TRANS)
>   relay_bindanyreq(con, 0, IPPROTO_TCP);
>   else if (relay_connect(con) == -1) {
> Index: usr.sbin/relayd/relay_http.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
> retrieving revision 1.72
> diff -u -p -u -p -r1.72 relay_http.c
> --- usr.sbin/relayd/relay_http.c  4 Mar 2019 21:25:03 -   1.72
> +++ usr.sbin/relayd/relay_http.c  8 May 2019 18:34:09 -
> @@ -110,17 +110,6 @@ relay_http_init(struct relay *rlay)
>   }
>   
>   int
> -relay_http_priv_init(struct rsession *con)
> -{
> - struct relay_http_priv  *p;
> - if ((p = calloc(1, sizeof(struct relay_http_priv))) == NULL)
> - return (-1);
> -
> - con->se_priv = p;
> - return (0);
> -}
> -
> -int
>   relay_httpdesc_init(struct ctl_relay_event *cre)
>   {
>   struct http_descriptor  *desc;
> @@ -163,13 +152,13 @@ relay_read_http(struct bufferevent *bev,
>   struct relay*rlay = con->se_relay;
>   struct protocol *proto = rlay->rl_proto;
>   struct evbuffer *src = EVBUFFER_INPUT(bev);
> - struct relay_http_priv  *priv = con->se_priv;
>   char*line = NULL, *key, *value;
>  

Re: relayd websocket

2019-05-08 Thread Reyk Floeter
On Wed, May 08, 2019 at 07:07:43PM +0200, Reyk Floeter wrote:
> On Wed, May 08, 2019 at 06:26:45PM +0200, Reyk Floeter wrote:
> > On Wed, Mar 06, 2019 at 05:36:32PM +0100, Sebastian Benoit wrote:
> > > Rivo Nurges(rivo.nur...@smit.ee) on 2019.03.05 22:42:13 +:
> > > > Hi!
> > > > 
> > > > On 3/5/19 10:36 PM, Claudio Jeker wrote:
> > > > > I guess that this would need strcasestr() instead of strcasecmp(), 
> > > > > since you
> > > > > are looking for the substring "Upgrade" in value. Maybe more is 
> > > > > needed if
> > > > > we want to be sure that 'Connection: Upgrade-maybe' does not match.
> > > > 
> > > > You are correct about strcasestr. "Connection: Upgrade-maybe" would 
> > > > need 
> > > > to have correct "Upgrade: websocket". Anyway, lets be strict.
> > > > 
> > > > Does something like this make sense?
> > > 
> > > i think the seperator list needs to include '\t' 
> > > because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.
> > > 
> > > And i dont think you can mix "," with " \t" seperators,
> > > because otherwise "Foo Upgrade, Bar" will match.
> > > 
> > > Something more is needed to parse elements of a header.
> > >  
> > 
> > I would reshuffle the websocket handling a bit as I don't think that
> > we need the http priv struct.  We can lookup the parsed headers later.
> > 
> > The attached diff moves it all into one places and introduces a new
> > function kv_find_value() that is able to to match headers with
> > multiple values (I think we might need it elsewhere.  And even if not,
> > I would prefer to have this in a function intead of stuffing it into
> > relay_read_http).
> > 
> > A minor question is if the lookup should be done before or after
> > applying the filters (relay_test - my diff does it afterwards, the
> > current code does it before).
> > 
> > Tests?  Comments?  Ok?
> > 
> 
> I keep on replying to my own diffs...  the updated one below uses
> strcasecmp instead of strcasestr in kv_find_value().  There is no need
> for substring- or fn- matching in this function.
> 

Next one...

benno@ pointed out that the RFC allows whitespace before the comma.
We also don't have to strip \r\n as it is done by relayd's kv parser.

OK?

Reyk

Index: usr.sbin/relayd/http.h
===
RCS file: /cvs/src/usr.sbin/relayd/http.h,v
retrieving revision 1.10
diff -u -p -u -p -r1.10 http.h
--- usr.sbin/relayd/http.h  4 Mar 2019 21:25:03 -   1.10
+++ usr.sbin/relayd/http.h  8 May 2019 18:34:09 -
@@ -251,10 +251,4 @@ struct http_descriptor {
struct kvtreehttp_headers;
 };
 
-struct relay_http_priv {
-#define HTTP_CONNECTION_UPGRADE0x01
-#define HTTP_UPGRADE_WEBSOCKET 0x02
-   int  http_upgrade_req;
-};
-
 #endif /* HTTP_H */
Index: usr.sbin/relayd/relay.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.242
diff -u -p -u -p -r1.242 relay.c
--- usr.sbin/relayd/relay.c 4 Mar 2019 21:25:03 -   1.242
+++ usr.sbin/relayd/relay.c 8 May 2019 18:34:09 -
@@ -1410,13 +1410,7 @@ relay_session(struct rsession *con)
return;
}
 
-   if (rlay->rl_proto->type == RELAY_PROTO_HTTP) {
-   if (relay_http_priv_init(con) == -1) {
-   relay_close(con,
-   "failed to allocate http session data", 1);
-   return;
-   }
-   } else {
+   if (rlay->rl_proto->type != RELAY_PROTO_HTTP) {
if (rlay->rl_conf.fwdmode == FWD_TRANS)
relay_bindanyreq(con, 0, IPPROTO_TCP);
else if (relay_connect(con) == -1) {
Index: usr.sbin/relayd/relay_http.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
retrieving revision 1.72
diff -u -p -u -p -r1.72 relay_http.c
--- usr.sbin/relayd/relay_http.c4 Mar 2019 21:25:03 -   1.72
+++ usr.sbin/relayd/relay_http.c8 May 2019 18:34:09 -
@@ -110,17 +110,6 @@ relay_http_init(struct relay *rlay)
 }
 
 int
-relay_http_priv_init(struct rsession *con)
-{
-   struct relay_http_priv  *p;
-   if ((p = calloc(1, sizeof(struct relay_http_priv))) == NULL)
-   return (-1);
-
-   con->se_priv = p;
-   return (0);
-}
-
-int
 relay_httpdesc_init(struct ctl_relay_event *cre)
 {
struct http_descriptor  *desc;
@@ -163,13 +152,13 @@ relay_read_http(struct bufferevent *bev,
struct relay*rlay = con->se_relay;
struct protocol *proto = rlay->rl_proto;
struct evbuffer *src = EVBUFFER_INPUT(bev);
-   struct relay_http_priv  *priv = con->se_priv;
char*line = NULL, *key, *value;
char*urlproto, *host, *path;
int  action, unique, ret;

Re: relayd websocket

2019-05-08 Thread Reyk Floeter
On Wed, May 08, 2019 at 06:26:45PM +0200, Reyk Floeter wrote:
> On Wed, Mar 06, 2019 at 05:36:32PM +0100, Sebastian Benoit wrote:
> > Rivo Nurges(rivo.nur...@smit.ee) on 2019.03.05 22:42:13 +:
> > > Hi!
> > > 
> > > On 3/5/19 10:36 PM, Claudio Jeker wrote:
> > > > I guess that this would need strcasestr() instead of strcasecmp(), 
> > > > since you
> > > > are looking for the substring "Upgrade" in value. Maybe more is needed 
> > > > if
> > > > we want to be sure that 'Connection: Upgrade-maybe' does not match.
> > > 
> > > You are correct about strcasestr. "Connection: Upgrade-maybe" would need 
> > > to have correct "Upgrade: websocket". Anyway, lets be strict.
> > > 
> > > Does something like this make sense?
> > 
> > i think the seperator list needs to include '\t' 
> > because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.
> > 
> > And i dont think you can mix "," with " \t" seperators,
> > because otherwise "Foo Upgrade, Bar" will match.
> > 
> > Something more is needed to parse elements of a header.
> >  
> 
> I would reshuffle the websocket handling a bit as I don't think that
> we need the http priv struct.  We can lookup the parsed headers later.
> 
> The attached diff moves it all into one places and introduces a new
> function kv_find_value() that is able to to match headers with
> multiple values (I think we might need it elsewhere.  And even if not,
> I would prefer to have this in a function intead of stuffing it into
> relay_read_http).
> 
> A minor question is if the lookup should be done before or after
> applying the filters (relay_test - my diff does it afterwards, the
> current code does it before).
> 
> Tests?  Comments?  Ok?
> 

I keep on replying to my own diffs...  the updated one below uses
strcasecmp instead of strcasestr in kv_find_value().  There is no need
for substring- or fn- matching in this function.

Reyk

Index: usr.sbin/relayd/http.h
===
RCS file: /cvs/src/usr.sbin/relayd/http.h,v
retrieving revision 1.10
diff -u -p -u -p -r1.10 http.h
--- usr.sbin/relayd/http.h  4 Mar 2019 21:25:03 -   1.10
+++ usr.sbin/relayd/http.h  8 May 2019 16:55:59 -
@@ -251,10 +251,4 @@ struct http_descriptor {
struct kvtreehttp_headers;
 };
 
-struct relay_http_priv {
-#define HTTP_CONNECTION_UPGRADE0x01
-#define HTTP_UPGRADE_WEBSOCKET 0x02
-   int  http_upgrade_req;
-};
-
 #endif /* HTTP_H */
Index: usr.sbin/relayd/relay.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.242
diff -u -p -u -p -r1.242 relay.c
--- usr.sbin/relayd/relay.c 4 Mar 2019 21:25:03 -   1.242
+++ usr.sbin/relayd/relay.c 8 May 2019 16:56:00 -
@@ -1410,13 +1410,7 @@ relay_session(struct rsession *con)
return;
}
 
-   if (rlay->rl_proto->type == RELAY_PROTO_HTTP) {
-   if (relay_http_priv_init(con) == -1) {
-   relay_close(con,
-   "failed to allocate http session data", 1);
-   return;
-   }
-   } else {
+   if (rlay->rl_proto->type != RELAY_PROTO_HTTP) {
if (rlay->rl_conf.fwdmode == FWD_TRANS)
relay_bindanyreq(con, 0, IPPROTO_TCP);
else if (relay_connect(con) == -1) {
Index: usr.sbin/relayd/relay_http.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
retrieving revision 1.72
diff -u -p -u -p -r1.72 relay_http.c
--- usr.sbin/relayd/relay_http.c4 Mar 2019 21:25:03 -   1.72
+++ usr.sbin/relayd/relay_http.c8 May 2019 16:56:01 -
@@ -110,17 +110,6 @@ relay_http_init(struct relay *rlay)
 }
 
 int
-relay_http_priv_init(struct rsession *con)
-{
-   struct relay_http_priv  *p;
-   if ((p = calloc(1, sizeof(struct relay_http_priv))) == NULL)
-   return (-1);
-
-   con->se_priv = p;
-   return (0);
-}
-
-int
 relay_httpdesc_init(struct ctl_relay_event *cre)
 {
struct http_descriptor  *desc;
@@ -163,13 +152,13 @@ relay_read_http(struct bufferevent *bev,
struct relay*rlay = con->se_relay;
struct protocol *proto = rlay->rl_proto;
struct evbuffer *src = EVBUFFER_INPUT(bev);
-   struct relay_http_priv  *priv = con->se_priv;
char*line = NULL, *key, *value;
char*urlproto, *host, *path;
int  action, unique, ret;
const char  *errstr;
size_t   size, linelen;
struct kv   *hdr = NULL;
+   struct kv   *upgrade = NULL, *upgrade_ws = NULL;
 
getmonotime(>se_tv_last);
cre->timedout = 0;
@@ -398,17 +387,6 @@ relay_read_http(struct bufferevent 

Re: relayd websocket

2019-05-08 Thread Reyk Floeter
On Wed, Mar 06, 2019 at 05:36:32PM +0100, Sebastian Benoit wrote:
> Rivo Nurges(rivo.nur...@smit.ee) on 2019.03.05 22:42:13 +:
> > Hi!
> > 
> > On 3/5/19 10:36 PM, Claudio Jeker wrote:
> > > I guess that this would need strcasestr() instead of strcasecmp(), since 
> > > you
> > > are looking for the substring "Upgrade" in value. Maybe more is needed if
> > > we want to be sure that 'Connection: Upgrade-maybe' does not match.
> > 
> > You are correct about strcasestr. "Connection: Upgrade-maybe" would need 
> > to have correct "Upgrade: websocket". Anyway, lets be strict.
> > 
> > Does something like this make sense?
> 
> i think the seperator list needs to include '\t' 
> because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.
> 
> And i dont think you can mix "," with " \t" seperators,
> because otherwise "Foo Upgrade, Bar" will match.
> 
> Something more is needed to parse elements of a header.
>  

I would reshuffle the websocket handling a bit as I don't think that
we need the http priv struct.  We can lookup the parsed headers later.

The attached diff moves it all into one places and introduces a new
function kv_find_value() that is able to to match headers with
multiple values (I think we might need it elsewhere.  And even if not,
I would prefer to have this in a function intead of stuffing it into
relay_read_http).

A minor question is if the lookup should be done before or after
applying the filters (relay_test - my diff does it afterwards, the
current code does it before).

Tests?  Comments?  Ok?

Reyk

Index: usr.sbin/relayd/http.h
===
RCS file: /cvs/src/usr.sbin/relayd/http.h,v
retrieving revision 1.10
diff -u -p -u -p -r1.10 http.h
--- usr.sbin/relayd/http.h  4 Mar 2019 21:25:03 -   1.10
+++ usr.sbin/relayd/http.h  8 May 2019 16:17:38 -
@@ -251,10 +251,4 @@ struct http_descriptor {
struct kvtreehttp_headers;
 };
 
-struct relay_http_priv {
-#define HTTP_CONNECTION_UPGRADE0x01
-#define HTTP_UPGRADE_WEBSOCKET 0x02
-   int  http_upgrade_req;
-};
-
 #endif /* HTTP_H */
Index: usr.sbin/relayd/relay.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.242
diff -u -p -u -p -r1.242 relay.c
--- usr.sbin/relayd/relay.c 4 Mar 2019 21:25:03 -   1.242
+++ usr.sbin/relayd/relay.c 8 May 2019 16:17:39 -
@@ -1410,13 +1410,7 @@ relay_session(struct rsession *con)
return;
}
 
-   if (rlay->rl_proto->type == RELAY_PROTO_HTTP) {
-   if (relay_http_priv_init(con) == -1) {
-   relay_close(con,
-   "failed to allocate http session data", 1);
-   return;
-   }
-   } else {
+   if (rlay->rl_proto->type != RELAY_PROTO_HTTP) {
if (rlay->rl_conf.fwdmode == FWD_TRANS)
relay_bindanyreq(con, 0, IPPROTO_TCP);
else if (relay_connect(con) == -1) {
Index: usr.sbin/relayd/relay_http.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
retrieving revision 1.72
diff -u -p -u -p -r1.72 relay_http.c
--- usr.sbin/relayd/relay_http.c4 Mar 2019 21:25:03 -   1.72
+++ usr.sbin/relayd/relay_http.c8 May 2019 16:17:40 -
@@ -110,17 +110,6 @@ relay_http_init(struct relay *rlay)
 }
 
 int
-relay_http_priv_init(struct rsession *con)
-{
-   struct relay_http_priv  *p;
-   if ((p = calloc(1, sizeof(struct relay_http_priv))) == NULL)
-   return (-1);
-
-   con->se_priv = p;
-   return (0);
-}
-
-int
 relay_httpdesc_init(struct ctl_relay_event *cre)
 {
struct http_descriptor  *desc;
@@ -163,13 +152,13 @@ relay_read_http(struct bufferevent *bev,
struct relay*rlay = con->se_relay;
struct protocol *proto = rlay->rl_proto;
struct evbuffer *src = EVBUFFER_INPUT(bev);
-   struct relay_http_priv  *priv = con->se_priv;
char*line = NULL, *key, *value;
char*urlproto, *host, *path;
int  action, unique, ret;
const char  *errstr;
size_t   size, linelen;
struct kv   *hdr = NULL;
+   struct kv   *upgrade = NULL, *upgrade_ws = NULL;
 
getmonotime(>se_tv_last);
cre->timedout = 0;
@@ -398,17 +387,6 @@ relay_read_http(struct bufferevent *bev,
unique = 0;
 
if (cre->line != 1) {
-   if (cre->dir == RELAY_DIR_REQUEST) {
-   if (strcasecmp("Connection", key) == 0 &&
-   strcasecmp("Upgrade", value) == 0)
-   

Re: relayd websocket

2019-05-08 Thread Rivo Nurges
Hi!

Anyone?

Rivo

On 2019-03-12 21:50, Rivo Nurges wrote:
> Hi!
> 
> Bump...
> 
> Rivo
> 
> On 3/6/19 10:57 PM, Rivo Nurges wrote:
>> Hi!
>>
>>
>> On 3/6/19 10:20 PM, Rivo Nurges wrote:
>>> On 3/6/19 6:36 PM, Sebastian Benoit wrote:
> Does something like this make sense?

 i think the seperator list needs to include '\t'
 because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.

 And i dont think you can mix "," with " \t" seperators,
 because otherwise "Foo Upgrade, Bar" will match.

 Something more is needed to parse elements of a header.
>>>
>>> Oh yeah. I'll work on that.
>>
>> So here comes the next version. Works with both spaces and tabs.
>>
>> Index: usr.sbin/relayd/relay_http.c
>> ===
>> RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
>> retrieving revision 1.72
>> diff -u -p -r1.72 relay_http.c
>> --- usr.sbin/relayd/relay_http.c 4 Mar 2019 21:25:03 -   1.72
>> +++ usr.sbin/relayd/relay_http.c 6 Mar 2019 20:53:59 -
>> @@ -36,6 +36,7 @@
>> #include 
>> #include 
>> #include 
>> +#include 
>>
>> #include "relayd.h"
>> #include "http.h"
>> @@ -166,6 +167,7 @@ relay_read_http(struct bufferevent *bev,
>>  struct relay_http_priv  *priv = con->se_priv;
>>  char*line = NULL, *key, *value;
>>  char*urlproto, *host, *path;
>> +char*valuecopy, *valuepart;
>>  int  action, unique, ret;
>>  const char  *errstr;
>>  size_t   size, linelen;
>> @@ -399,10 +401,19 @@ relay_read_http(struct bufferevent *bev,
>>
>>  if (cre->line != 1) {
>>  if (cre->dir == RELAY_DIR_REQUEST) {
>> -if (strcasecmp("Connection", key) == 0 &&
>> -strcasecmp("Upgrade", value) == 0)
>> -priv->http_upgrade_req |=
>> -HTTP_CONNECTION_UPGRADE;
>> +if (strcasecmp("Connection", key) == 0) {
>> +valuecopy = strdup(value);
>> +while ((valuepart = strsep(,
>> +",")) != NULL) {
>> +while (isblank(*valuepart))
>> +valuepart = [1];
>> +if (strcasecmp("Upgrade", valuepart)
>> +== 0)
>> +priv->http_upgrade_req |=
>> +HTTP_CONNECTION_UPGRADE;
>> +}
>> +free(valuecopy);
>> +}
>>  if (strcasecmp("Upgrade", key) == 0 &&
>>  strcasecmp("websocket", value) == 0)
>>  priv->http_upgrade_req |=
>>
>>
>> begin-base64 644 websocket3.diff
>> SW5kZXg6IHVzci5zYmluL3JlbGF5ZC9yZWxheV9odHRwLmMKPT09PT09PT09PT09PT09PT09PT09
>> PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQpSQ1MgZmlsZTog
>> L2N2cy9zcmMvdXNyLnNiaW4vcmVsYXlkL3JlbGF5X2h0dHAuYyx2CnJldHJpZXZpbmcgcmV2aXNp
>> b24gMS43MgpkaWZmIC11IC1wIC1yMS43MiByZWxheV9odHRwLmMKLS0tIHVzci5zYmluL3JlbGF5
>> ZC9yZWxheV9odHRwLmMJNCBNYXIgMjAxOSAyMToyNTowMyAtMDAwMAkxLjcyCisrKyB1c3Iuc2Jp
>> bi9yZWxheWQvcmVsYXlfaHR0cC5jCTYgTWFyIDIwMTkgMjA6NTM6NTkgLTAwMDAKQEAgLTM2LDYg
>> KzM2LDcgQEAKICNpbmNsdWRlIDxzaXBoYXNoLmg+CiAjaW5jbHVkZSA8aW1zZy5oPgogI2luY2x1
>> ZGUgPHVuaXN0ZC5oPgorI2luY2x1ZGUgPGN0eXBlLmg+CiAKICNpbmNsdWRlICJyZWxheWQuaCIK
>> ICNpbmNsdWRlICJodHRwLmgiCkBAIC0xNjYsNiArMTY3LDcgQEAgcmVsYXlfcmVhZF9odHRwKHN0
>> cnVjdCBidWZmZXJldmVudCAqYmV2LAogCXN0cnVjdCByZWxheV9odHRwX3ByaXYJKnByaXYgPSBj
>> b24tPnNlX3ByaXY7CiAJY2hhcgkJCSpsaW5lID0gTlVMTCwgKmtleSwgKnZhbHVlOwogCWNoYXIJ
>> CQkqdXJscHJvdG8sICpob3N0LCAqcGF0aDsKKwljaGFyCQkJKnZhbHVlY29weSwgKnZhbHVlcGFy
>> dDsKIAlpbnQJCQkgYWN0aW9uLCB1bmlxdWUsIHJldDsKIAljb25zdCBjaGFyCQkqZXJyc3RyOwog
>> CXNpemVfdAkJCSBzaXplLCBsaW5lbGVuOwpAQCAtMzk5LDEwICs0MDEsMTkgQEAgcmVsYXlfcmVh
>> ZF9odHRwKHN0cnVjdCBidWZmZXJldmVudCAqYmV2LAogCiAJCWlmIChjcmUtPmxpbmUgIT0gMSkg
>> ewogCQkJaWYgKGNyZS0+ZGlyID09IFJFTEFZX0RJUl9SRVFVRVNUKSB7Ci0JCQkJaWYgKHN0cmNh
>> c2VjbXAoIkNvbm5lY3Rpb24iLCBrZXkpID09IDAgJiYKLQkJCQkgICAgc3RyY2FzZWNtcCgiVXBn
>> cmFkZSIsIHZhbHVlKSA9PSAwKQotCQkJCQlwcml2LT5odHRwX3VwZ3JhZGVfcmVxIHw9Ci0JCQkJ
>> CSAgICBIVFRQX0NPTk5FQ1RJT05fVVBHUkFERTsKKwkJCQlpZiAoc3RyY2FzZWNtcCgiQ29ubmVj
>> dGlvbiIsIGtleSkgPT0gMCkgeworCQkJCSAgICB2YWx1ZWNvcHkgPSBzdHJkdXAodmFsdWUpOwor
>> CQkJCSAgICB3aGlsZSAoKHZhbHVlcGFydCA9IHN0cnNlcCgmdmFsdWVjb3B5LAorCQkJCQkiLCIp
>> KSAhPSBOVUxMKSB7CisJCQkJCXdoaWxlIChpc2JsYW5rKCp2YWx1ZXBhcnQpKQorCQkJCQkgICAg
>> dmFsdWVwYXJ0ID0gJnZhbHVlcGFydFsxXTsKKwkJCQkgICAgCWlmIChzdHJjYXNlY21wKCJVcGdy
>> 

Re: relayd websocket

2019-03-12 Thread Rivo Nurges
Hi!

Bump...

Rivo

On 3/6/19 10:57 PM, Rivo Nurges wrote:
> Hi!
> 
> 
> On 3/6/19 10:20 PM, Rivo Nurges wrote:
>> On 3/6/19 6:36 PM, Sebastian Benoit wrote:
 Does something like this make sense?
>>>
>>> i think the seperator list needs to include '\t'
>>> because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.
>>>
>>> And i dont think you can mix "," with " \t" seperators,
>>> because otherwise "Foo Upgrade, Bar" will match.
>>>
>>> Something more is needed to parse elements of a header.
>>
>> Oh yeah. I'll work on that.
> 
> So here comes the next version. Works with both spaces and tabs.
> 
> Index: usr.sbin/relayd/relay_http.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
> retrieving revision 1.72
> diff -u -p -r1.72 relay_http.c
> --- usr.sbin/relayd/relay_http.c  4 Mar 2019 21:25:03 -   1.72
> +++ usr.sbin/relayd/relay_http.c  6 Mar 2019 20:53:59 -
> @@ -36,6 +36,7 @@
>#include 
>#include 
>#include 
> +#include 
> 
>#include "relayd.h"
>#include "http.h"
> @@ -166,6 +167,7 @@ relay_read_http(struct bufferevent *bev,
>   struct relay_http_priv  *priv = con->se_priv;
>   char*line = NULL, *key, *value;
>   char*urlproto, *host, *path;
> + char*valuecopy, *valuepart;
>   int  action, unique, ret;
>   const char  *errstr;
>   size_t   size, linelen;
> @@ -399,10 +401,19 @@ relay_read_http(struct bufferevent *bev,
> 
>   if (cre->line != 1) {
>   if (cre->dir == RELAY_DIR_REQUEST) {
> - if (strcasecmp("Connection", key) == 0 &&
> - strcasecmp("Upgrade", value) == 0)
> - priv->http_upgrade_req |=
> - HTTP_CONNECTION_UPGRADE;
> + if (strcasecmp("Connection", key) == 0) {
> + valuecopy = strdup(value);
> + while ((valuepart = strsep(,
> + ",")) != NULL) {
> + while (isblank(*valuepart))
> + valuepart = [1];
> + if (strcasecmp("Upgrade", valuepart)
> + == 0)
> + priv->http_upgrade_req |=
> + HTTP_CONNECTION_UPGRADE;
> + }
> + free(valuecopy);
> + }
>   if (strcasecmp("Upgrade", key) == 0 &&
>   strcasecmp("websocket", value) == 0)
>   priv->http_upgrade_req |=
> 
> 
> begin-base64 644 websocket3.diff
> SW5kZXg6IHVzci5zYmluL3JlbGF5ZC9yZWxheV9odHRwLmMKPT09PT09PT09PT09PT09PT09PT09
> PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQpSQ1MgZmlsZTog
> L2N2cy9zcmMvdXNyLnNiaW4vcmVsYXlkL3JlbGF5X2h0dHAuYyx2CnJldHJpZXZpbmcgcmV2aXNp
> b24gMS43MgpkaWZmIC11IC1wIC1yMS43MiByZWxheV9odHRwLmMKLS0tIHVzci5zYmluL3JlbGF5
> ZC9yZWxheV9odHRwLmMJNCBNYXIgMjAxOSAyMToyNTowMyAtMDAwMAkxLjcyCisrKyB1c3Iuc2Jp
> bi9yZWxheWQvcmVsYXlfaHR0cC5jCTYgTWFyIDIwMTkgMjA6NTM6NTkgLTAwMDAKQEAgLTM2LDYg
> KzM2LDcgQEAKICNpbmNsdWRlIDxzaXBoYXNoLmg+CiAjaW5jbHVkZSA8aW1zZy5oPgogI2luY2x1
> ZGUgPHVuaXN0ZC5oPgorI2luY2x1ZGUgPGN0eXBlLmg+CiAKICNpbmNsdWRlICJyZWxheWQuaCIK
> ICNpbmNsdWRlICJodHRwLmgiCkBAIC0xNjYsNiArMTY3LDcgQEAgcmVsYXlfcmVhZF9odHRwKHN0
> cnVjdCBidWZmZXJldmVudCAqYmV2LAogCXN0cnVjdCByZWxheV9odHRwX3ByaXYJKnByaXYgPSBj
> b24tPnNlX3ByaXY7CiAJY2hhcgkJCSpsaW5lID0gTlVMTCwgKmtleSwgKnZhbHVlOwogCWNoYXIJ
> CQkqdXJscHJvdG8sICpob3N0LCAqcGF0aDsKKwljaGFyCQkJKnZhbHVlY29weSwgKnZhbHVlcGFy
> dDsKIAlpbnQJCQkgYWN0aW9uLCB1bmlxdWUsIHJldDsKIAljb25zdCBjaGFyCQkqZXJyc3RyOwog
> CXNpemVfdAkJCSBzaXplLCBsaW5lbGVuOwpAQCAtMzk5LDEwICs0MDEsMTkgQEAgcmVsYXlfcmVh
> ZF9odHRwKHN0cnVjdCBidWZmZXJldmVudCAqYmV2LAogCiAJCWlmIChjcmUtPmxpbmUgIT0gMSkg
> ewogCQkJaWYgKGNyZS0+ZGlyID09IFJFTEFZX0RJUl9SRVFVRVNUKSB7Ci0JCQkJaWYgKHN0cmNh
> c2VjbXAoIkNvbm5lY3Rpb24iLCBrZXkpID09IDAgJiYKLQkJCQkgICAgc3RyY2FzZWNtcCgiVXBn
> cmFkZSIsIHZhbHVlKSA9PSAwKQotCQkJCQlwcml2LT5odHRwX3VwZ3JhZGVfcmVxIHw9Ci0JCQkJ
> CSAgICBIVFRQX0NPTk5FQ1RJT05fVVBHUkFERTsKKwkJCQlpZiAoc3RyY2FzZWNtcCgiQ29ubmVj
> dGlvbiIsIGtleSkgPT0gMCkgeworCQkJCSAgICB2YWx1ZWNvcHkgPSBzdHJkdXAodmFsdWUpOwor
> CQkJCSAgICB3aGlsZSAoKHZhbHVlcGFydCA9IHN0cnNlcCgmdmFsdWVjb3B5LAorCQkJCQkiLCIp
> KSAhPSBOVUxMKSB7CisJCQkJCXdoaWxlIChpc2JsYW5rKCp2YWx1ZXBhcnQpKQorCQkJCQkgICAg
> dmFsdWVwYXJ0ID0gJnZhbHVlcGFydFsxXTsKKwkJCQkgICAgCWlmIChzdHJjYXNlY21wKCJVcGdy
> YWRlIiwgdmFsdWVwYXJ0KQorCQkJCQkgICAgPT0gMCkKKwkJCQkJICAgIHByaXYtPmh0dHBfdXBn
> 

Re: relayd websocket

2019-03-06 Thread Rivo Nurges
Hi!


On 3/6/19 10:20 PM, Rivo Nurges wrote:
> On 3/6/19 6:36 PM, Sebastian Benoit wrote:
>>> Does something like this make sense?
>>
>> i think the seperator list needs to include '\t'
>> because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.
>>
>> And i dont think you can mix "," with " \t" seperators,
>> because otherwise "Foo Upgrade, Bar" will match.
>>
>> Something more is needed to parse elements of a header.
> 
> Oh yeah. I'll work on that.

So here comes the next version. Works with both spaces and tabs.

Index: usr.sbin/relayd/relay_http.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
retrieving revision 1.72
diff -u -p -r1.72 relay_http.c
--- usr.sbin/relayd/relay_http.c4 Mar 2019 21:25:03 -   1.72
+++ usr.sbin/relayd/relay_http.c6 Mar 2019 20:53:59 -
@@ -36,6 +36,7 @@
  #include 
  #include 
  #include 
+#include 

  #include "relayd.h"
  #include "http.h"
@@ -166,6 +167,7 @@ relay_read_http(struct bufferevent *bev,
struct relay_http_priv  *priv = con->se_priv;
char*line = NULL, *key, *value;
char*urlproto, *host, *path;
+   char*valuecopy, *valuepart;
int  action, unique, ret;
const char  *errstr;
size_t   size, linelen;
@@ -399,10 +401,19 @@ relay_read_http(struct bufferevent *bev,

if (cre->line != 1) {
if (cre->dir == RELAY_DIR_REQUEST) {
-   if (strcasecmp("Connection", key) == 0 &&
-   strcasecmp("Upgrade", value) == 0)
-   priv->http_upgrade_req |=
-   HTTP_CONNECTION_UPGRADE;
+   if (strcasecmp("Connection", key) == 0) {
+   valuecopy = strdup(value);
+   while ((valuepart = strsep(,
+   ",")) != NULL) {
+   while (isblank(*valuepart))
+   valuepart = [1];
+   if (strcasecmp("Upgrade", valuepart)
+   == 0)
+   priv->http_upgrade_req |=
+   HTTP_CONNECTION_UPGRADE;
+   }
+   free(valuecopy);
+   }
if (strcasecmp("Upgrade", key) == 0 &&
strcasecmp("websocket", value) == 0)
priv->http_upgrade_req |=


begin-base64 644 websocket3.diff
SW5kZXg6IHVzci5zYmluL3JlbGF5ZC9yZWxheV9odHRwLmMKPT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQpSQ1MgZmlsZTog
L2N2cy9zcmMvdXNyLnNiaW4vcmVsYXlkL3JlbGF5X2h0dHAuYyx2CnJldHJpZXZpbmcgcmV2aXNp
b24gMS43MgpkaWZmIC11IC1wIC1yMS43MiByZWxheV9odHRwLmMKLS0tIHVzci5zYmluL3JlbGF5
ZC9yZWxheV9odHRwLmMJNCBNYXIgMjAxOSAyMToyNTowMyAtMDAwMAkxLjcyCisrKyB1c3Iuc2Jp
bi9yZWxheWQvcmVsYXlfaHR0cC5jCTYgTWFyIDIwMTkgMjA6NTM6NTkgLTAwMDAKQEAgLTM2LDYg
KzM2LDcgQEAKICNpbmNsdWRlIDxzaXBoYXNoLmg+CiAjaW5jbHVkZSA8aW1zZy5oPgogI2luY2x1
ZGUgPHVuaXN0ZC5oPgorI2luY2x1ZGUgPGN0eXBlLmg+CiAKICNpbmNsdWRlICJyZWxheWQuaCIK
ICNpbmNsdWRlICJodHRwLmgiCkBAIC0xNjYsNiArMTY3LDcgQEAgcmVsYXlfcmVhZF9odHRwKHN0
cnVjdCBidWZmZXJldmVudCAqYmV2LAogCXN0cnVjdCByZWxheV9odHRwX3ByaXYJKnByaXYgPSBj
b24tPnNlX3ByaXY7CiAJY2hhcgkJCSpsaW5lID0gTlVMTCwgKmtleSwgKnZhbHVlOwogCWNoYXIJ
CQkqdXJscHJvdG8sICpob3N0LCAqcGF0aDsKKwljaGFyCQkJKnZhbHVlY29weSwgKnZhbHVlcGFy
dDsKIAlpbnQJCQkgYWN0aW9uLCB1bmlxdWUsIHJldDsKIAljb25zdCBjaGFyCQkqZXJyc3RyOwog
CXNpemVfdAkJCSBzaXplLCBsaW5lbGVuOwpAQCAtMzk5LDEwICs0MDEsMTkgQEAgcmVsYXlfcmVh
ZF9odHRwKHN0cnVjdCBidWZmZXJldmVudCAqYmV2LAogCiAJCWlmIChjcmUtPmxpbmUgIT0gMSkg
ewogCQkJaWYgKGNyZS0+ZGlyID09IFJFTEFZX0RJUl9SRVFVRVNUKSB7Ci0JCQkJaWYgKHN0cmNh
c2VjbXAoIkNvbm5lY3Rpb24iLCBrZXkpID09IDAgJiYKLQkJCQkgICAgc3RyY2FzZWNtcCgiVXBn
cmFkZSIsIHZhbHVlKSA9PSAwKQotCQkJCQlwcml2LT5odHRwX3VwZ3JhZGVfcmVxIHw9Ci0JCQkJ
CSAgICBIVFRQX0NPTk5FQ1RJT05fVVBHUkFERTsKKwkJCQlpZiAoc3RyY2FzZWNtcCgiQ29ubmVj
dGlvbiIsIGtleSkgPT0gMCkgeworCQkJCSAgICB2YWx1ZWNvcHkgPSBzdHJkdXAodmFsdWUpOwor
CQkJCSAgICB3aGlsZSAoKHZhbHVlcGFydCA9IHN0cnNlcCgmdmFsdWVjb3B5LAorCQkJCQkiLCIp
KSAhPSBOVUxMKSB7CisJCQkJCXdoaWxlIChpc2JsYW5rKCp2YWx1ZXBhcnQpKQorCQkJCQkgICAg
dmFsdWVwYXJ0ID0gJnZhbHVlcGFydFsxXTsKKwkJCQkgICAgCWlmIChzdHJjYXNlY21wKCJVcGdy
YWRlIiwgdmFsdWVwYXJ0KQorCQkJCQkgICAgPT0gMCkKKwkJCQkJICAgIHByaXYtPmh0dHBfdXBn
cmFkZV9yZXEgfD0KKwkJCQkJICAgIAlIVFRQX0NPTk5FQ1RJT05fVVBHUkFERTsKKwkJCQkgICAg
fQorCQkJCSAgICBmcmVlKHZhbHVlY29weSk7CisJCQkJfQogCQkJCWlmIChzdHJjYXNlY21wKCJV
cGdyYWRlIiwga2V5KSA9PSAwICYmCiAJCQkJICAgIHN0cmNhc2VjbXAoIndlYnNvY2tldCIsIHZh

Re: relayd websocket

2019-03-06 Thread Rivo Nurges
On 3/6/19 6:36 PM, Sebastian Benoit wrote:
>> Does something like this make sense?
> 
> i think the seperator list needs to include '\t'
> because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.
> 
> And i dont think you can mix "," with " \t" seperators,
> because otherwise "Foo Upgrade, Bar" will match.
> 
> Something more is needed to parse elements of a header.

Oh yeah. I'll work on that.

Rivo



Re: relayd websocket

2019-03-06 Thread Sebastian Benoit
Rivo Nurges(rivo.nur...@smit.ee) on 2019.03.05 22:42:13 +:
> Hi!
> 
> On 3/5/19 10:36 PM, Claudio Jeker wrote:
> > I guess that this would need strcasestr() instead of strcasecmp(), since you
> > are looking for the substring "Upgrade" in value. Maybe more is needed if
> > we want to be sure that 'Connection: Upgrade-maybe' does not match.
> 
> You are correct about strcasestr. "Connection: Upgrade-maybe" would need 
> to have correct "Upgrade: websocket". Anyway, lets be strict.
> 
> Does something like this make sense?

i think the seperator list needs to include '\t' 
because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.

And i dont think you can mix "," with " \t" seperators,
because otherwise "Foo Upgrade, Bar" will match.

Something more is needed to parse elements of a header.
 
> Index: usr.sbin/relayd/relay_http.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
> retrieving revision 1.72
> diff -u -p -r1.72 relay_http.c
> --- usr.sbin/relayd/relay_http.c  4 Mar 2019 21:25:03 -   1.72
> +++ usr.sbin/relayd/relay_http.c  5 Mar 2019 22:33:47 -
> @@ -166,6 +166,7 @@ relay_read_http(struct bufferevent *bev,
>   struct relay_http_priv  *priv = con->se_priv;
>   char*line = NULL, *key, *value;
>   char*urlproto, *host, *path;
> + char*valuecopy, *valuepart;
>   int  action, unique, ret;
>   const char  *errstr;
>   size_t   size, linelen;
> @@ -399,10 +400,18 @@ relay_read_http(struct bufferevent *bev,
> 
>   if (cre->line != 1) {
>   if (cre->dir == RELAY_DIR_REQUEST) {
> - if (strcasecmp("Connection", key) == 0 &&
> - strcasecmp("Upgrade", value) == 0)
> - priv->http_upgrade_req |=
> - HTTP_CONNECTION_UPGRADE;
> +
> +
> + if (strcasecmp("Connection", key) == 0) {
> + valuecopy = strdup(value);
> + while ((valuepart = strsep(, ", 
> ")) != NULL)
> + if (strcasecmp("Upgrade", valuepart) == 
> 0)
> + priv->http_upgrade_req |=
> + HTTP_CONNECTION_UPGRADE;
> + free(valuecopy);
> + }
> +
> +
>   if (strcasecmp("Upgrade", key) == 0 &&
>   strcasecmp("websocket", value) == 0)
>   priv->http_upgrade_req |=
> 
> 
> 
> begin-base64 644 websocket2.diff
> SW5kZXg6IHVzci5zYmluL3JlbGF5ZC9yZWxheV9odHRwLmMKPT09PT09PT09PT09PT09PT09PT09
> PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQpSQ1MgZmlsZTog
> L2N2cy9zcmMvdXNyLnNiaW4vcmVsYXlkL3JlbGF5X2h0dHAuYyx2CnJldHJpZXZpbmcgcmV2aXNp
> b24gMS43MgpkaWZmIC11IC1wIC1yMS43MiByZWxheV9odHRwLmMKLS0tIHVzci5zYmluL3JlbGF5
> ZC9yZWxheV9odHRwLmMJNCBNYXIgMjAxOSAyMToyNTowMyAtMDAwMAkxLjcyCisrKyB1c3Iuc2Jp
> bi9yZWxheWQvcmVsYXlfaHR0cC5jCTUgTWFyIDIwMTkgMjI6MzM6NDcgLTAwMDAKQEAgLTE2Niw2
> ICsxNjYsNyBAQCByZWxheV9yZWFkX2h0dHAoc3RydWN0IGJ1ZmZlcmV2ZW50ICpiZXYsCiAJc3Ry
> dWN0IHJlbGF5X2h0dHBfcHJpdgkqcHJpdiA9IGNvbi0+c2VfcHJpdjsKIAljaGFyCQkJKmxpbmUg
> PSBOVUxMLCAqa2V5LCAqdmFsdWU7CiAJY2hhcgkJCSp1cmxwcm90bywgKmhvc3QsICpwYXRoOwor
> CWNoYXIJCQkqdmFsdWVjb3B5LCAqdmFsdWVwYXJ0OwogCWludAkJCSBhY3Rpb24sIHVuaXF1ZSwg
> cmV0OwogCWNvbnN0IGNoYXIJCSplcnJzdHI7CiAJc2l6ZV90CQkJIHNpemUsIGxpbmVsZW47CkBA
> IC0zOTksMTAgKzQwMCwxOCBAQCByZWxheV9yZWFkX2h0dHAoc3RydWN0IGJ1ZmZlcmV2ZW50ICpi
> ZXYsCiAKIAkJaWYgKGNyZS0+bGluZSAhPSAxKSB7CiAJCQlpZiAoY3JlLT5kaXIgPT0gUkVMQVlf
> RElSX1JFUVVFU1QpIHsKLQkJCQlpZiAoc3RyY2FzZWNtcCgiQ29ubmVjdGlvbiIsIGtleSkgPT0g
> MCAmJgotCQkJCSAgICBzdHJjYXNlY21wKCJVcGdyYWRlIiwgdmFsdWUpID09IDApCi0JCQkJCXBy
> aXYtPmh0dHBfdXBncmFkZV9yZXEgfD0KLQkJCQkJICAgIEhUVFBfQ09OTkVDVElPTl9VUEdSQURF
> OworCisKKwkJCQlpZiAoc3RyY2FzZWNtcCgiQ29ubmVjdGlvbiIsIGtleSkgPT0gMCkgeworCQkJ
> CSAgICB2YWx1ZWNvcHkgPSBzdHJkdXAodmFsdWUpOworCQkJCSAgICB3aGlsZSAoKHZhbHVlcGFy
> dCA9IHN0cnNlcCgmdmFsdWVjb3B5LCAiLCAiKSkgIT0gTlVMTCkKKwkJCQkgICAgCWlmIChzdHJj
> YXNlY21wKCJVcGdyYWRlIiwgdmFsdWVwYXJ0KSA9PSAwKQorCQkJCQkgICAgcHJpdi0+aHR0cF91
> cGdyYWRlX3JlcSB8PQorCQkJCQkgICAgCUhUVFBfQ09OTkVDVElPTl9VUEdSQURFOworCQkJCSAg
> ICBmcmVlKHZhbHVlY29weSk7CisJCQkJfQorCisKIAkJCQlpZiAoc3RyY2FzZWNtcCgiVXBncmFk
> ZSIsIGtleSkgPT0gMCAmJgogCQkJCSAgICBzdHJjYXNlY21wKCJ3ZWJzb2NrZXQiLCB2YWx1ZSkg
> PT0gMCkKIAkJCQkJcHJpdi0+aHR0cF91cGdyYWRlX3JlcSB8PQo=
> 
> 
> 
> 
> 
> 
> 



Re: relayd websocket

2019-03-05 Thread Rivo Nurges
Hi!

On 3/5/19 10:36 PM, Claudio Jeker wrote:
> I guess that this would need strcasestr() instead of strcasecmp(), since you
> are looking for the substring "Upgrade" in value. Maybe more is needed if
> we want to be sure that 'Connection: Upgrade-maybe' does not match.

You are correct about strcasestr. "Connection: Upgrade-maybe" would need 
to have correct "Upgrade: websocket". Anyway, lets be strict.

Does something like this make sense?

Index: usr.sbin/relayd/relay_http.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
retrieving revision 1.72
diff -u -p -r1.72 relay_http.c
--- usr.sbin/relayd/relay_http.c4 Mar 2019 21:25:03 -   1.72
+++ usr.sbin/relayd/relay_http.c5 Mar 2019 22:33:47 -
@@ -166,6 +166,7 @@ relay_read_http(struct bufferevent *bev,
struct relay_http_priv  *priv = con->se_priv;
char*line = NULL, *key, *value;
char*urlproto, *host, *path;
+   char*valuecopy, *valuepart;
int  action, unique, ret;
const char  *errstr;
size_t   size, linelen;
@@ -399,10 +400,18 @@ relay_read_http(struct bufferevent *bev,

if (cre->line != 1) {
if (cre->dir == RELAY_DIR_REQUEST) {
-   if (strcasecmp("Connection", key) == 0 &&
-   strcasecmp("Upgrade", value) == 0)
-   priv->http_upgrade_req |=
-   HTTP_CONNECTION_UPGRADE;
+
+
+   if (strcasecmp("Connection", key) == 0) {
+   valuecopy = strdup(value);
+   while ((valuepart = strsep(, ", 
")) != NULL)
+   if (strcasecmp("Upgrade", valuepart) == 
0)
+   priv->http_upgrade_req |=
+   HTTP_CONNECTION_UPGRADE;
+   free(valuecopy);
+   }
+
+
if (strcasecmp("Upgrade", key) == 0 &&
strcasecmp("websocket", value) == 0)
priv->http_upgrade_req |=



begin-base64 644 websocket2.diff
SW5kZXg6IHVzci5zYmluL3JlbGF5ZC9yZWxheV9odHRwLmMKPT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQpSQ1MgZmlsZTog
L2N2cy9zcmMvdXNyLnNiaW4vcmVsYXlkL3JlbGF5X2h0dHAuYyx2CnJldHJpZXZpbmcgcmV2aXNp
b24gMS43MgpkaWZmIC11IC1wIC1yMS43MiByZWxheV9odHRwLmMKLS0tIHVzci5zYmluL3JlbGF5
ZC9yZWxheV9odHRwLmMJNCBNYXIgMjAxOSAyMToyNTowMyAtMDAwMAkxLjcyCisrKyB1c3Iuc2Jp
bi9yZWxheWQvcmVsYXlfaHR0cC5jCTUgTWFyIDIwMTkgMjI6MzM6NDcgLTAwMDAKQEAgLTE2Niw2
ICsxNjYsNyBAQCByZWxheV9yZWFkX2h0dHAoc3RydWN0IGJ1ZmZlcmV2ZW50ICpiZXYsCiAJc3Ry
dWN0IHJlbGF5X2h0dHBfcHJpdgkqcHJpdiA9IGNvbi0+c2VfcHJpdjsKIAljaGFyCQkJKmxpbmUg
PSBOVUxMLCAqa2V5LCAqdmFsdWU7CiAJY2hhcgkJCSp1cmxwcm90bywgKmhvc3QsICpwYXRoOwor
CWNoYXIJCQkqdmFsdWVjb3B5LCAqdmFsdWVwYXJ0OwogCWludAkJCSBhY3Rpb24sIHVuaXF1ZSwg
cmV0OwogCWNvbnN0IGNoYXIJCSplcnJzdHI7CiAJc2l6ZV90CQkJIHNpemUsIGxpbmVsZW47CkBA
IC0zOTksMTAgKzQwMCwxOCBAQCByZWxheV9yZWFkX2h0dHAoc3RydWN0IGJ1ZmZlcmV2ZW50ICpi
ZXYsCiAKIAkJaWYgKGNyZS0+bGluZSAhPSAxKSB7CiAJCQlpZiAoY3JlLT5kaXIgPT0gUkVMQVlf
RElSX1JFUVVFU1QpIHsKLQkJCQlpZiAoc3RyY2FzZWNtcCgiQ29ubmVjdGlvbiIsIGtleSkgPT0g
MCAmJgotCQkJCSAgICBzdHJjYXNlY21wKCJVcGdyYWRlIiwgdmFsdWUpID09IDApCi0JCQkJCXBy
aXYtPmh0dHBfdXBncmFkZV9yZXEgfD0KLQkJCQkJICAgIEhUVFBfQ09OTkVDVElPTl9VUEdSQURF
OworCisKKwkJCQlpZiAoc3RyY2FzZWNtcCgiQ29ubmVjdGlvbiIsIGtleSkgPT0gMCkgeworCQkJ
CSAgICB2YWx1ZWNvcHkgPSBzdHJkdXAodmFsdWUpOworCQkJCSAgICB3aGlsZSAoKHZhbHVlcGFy
dCA9IHN0cnNlcCgmdmFsdWVjb3B5LCAiLCAiKSkgIT0gTlVMTCkKKwkJCQkgICAgCWlmIChzdHJj
YXNlY21wKCJVcGdyYWRlIiwgdmFsdWVwYXJ0KSA9PSAwKQorCQkJCQkgICAgcHJpdi0+aHR0cF91
cGdyYWRlX3JlcSB8PQorCQkJCQkgICAgCUhUVFBfQ09OTkVDVElPTl9VUEdSQURFOworCQkJCSAg
ICBmcmVlKHZhbHVlY29weSk7CisJCQkJfQorCisKIAkJCQlpZiAoc3RyY2FzZWNtcCgiVXBncmFk
ZSIsIGtleSkgPT0gMCAmJgogCQkJCSAgICBzdHJjYXNlY21wKCJ3ZWJzb2NrZXQiLCB2YWx1ZSkg
PT0gMCkKIAkJCQkJcHJpdi0+aHR0cF91cGdyYWRlX3JlcSB8PQo=









Re: relayd websocket

2019-03-05 Thread Claudio Jeker
On Tue, Mar 05, 2019 at 04:21:30PM +, Rivo Nurges wrote:
> Hi!
> 
> RFC 6455 4.2.1 states:
> 4.   A |Connection| header field that *includes* the token "Upgrade",
>   treated as an ASCII case-insensitive value.
> 
> In my test case Firefox sends: Connection: keep-alive, Upgrade
> 
> Relayd currently expects Connection to equal Upgrade, not include Upgrade.
> 
> I haven't figured out how to configure Thunderbird to send proper diffs, 
> so I'm sending bas64 encoded version too.
> 
> Index: usr.sbin/relayd/relay_http.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
> retrieving revision 1.72
> diff -u -p -r1.72 relay_http.c
> --- usr.sbin/relayd/relay_http.c  4 Mar 2019 21:25:03 -   1.72
> +++ usr.sbin/relayd/relay_http.c  5 Mar 2019 16:03:56 -
> @@ -400,7 +400,7 @@ relay_read_http(struct bufferevent *bev,
>   if (cre->line != 1) {
>   if (cre->dir == RELAY_DIR_REQUEST) {
>   if (strcasecmp("Connection", key) == 0 &&
> - strcasecmp("Upgrade", value) == 0)
> + strcasecmp("Upgrade", value) >= 0)
>   priv->http_upgrade_req |=
>   HTTP_CONNECTION_UPGRADE;
>   if (strcasecmp("Upgrade", key) == 0 &&
> 

I guess that this would need strcasestr() instead of strcasecmp(), since you
are looking for the substring "Upgrade" in value. Maybe more is needed if
we want to be sure that 'Connection: Upgrade-maybe' does not match.

-- 
:wq Claudio