relayd websocket

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

relayd websocket

Rivo Nurges-2
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 -0000 1.72
+++ usr.sbin/relayd/relay_http.c 5 Mar 2019 16:03:56 -0000
@@ -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 &&


begin-base64 644 websocket.diff
SW5kZXg6IHVzci5zYmluL3JlbGF5ZC9yZWxheV9odHRwLmMKPT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQpSQ1MgZmlsZTog
L2N2cy9zcmMvdXNyLnNiaW4vcmVsYXlkL3JlbGF5X2h0dHAuYyx2CnJldHJpZXZpbmcgcmV2aXNp
b24gMS43MgpkaWZmIC11IC1wIC1yMS43MiByZWxheV9odHRwLmMKLS0tIHVzci5zYmluL3JlbGF5
ZC9yZWxheV9odHRwLmMJNCBNYXIgMjAxOSAyMToyNTowMyAtMDAwMAkxLjcyCisrKyB1c3Iuc2Jp
bi9yZWxheWQvcmVsYXlfaHR0cC5jCTUgTWFyIDIwMTkgMTY6MDM6NTYgLTAwMDAKQEAgLTQwMCw3
ICs0MDAsNyBAQCByZWxheV9yZWFkX2h0dHAoc3RydWN0IGJ1ZmZlcmV2ZW50ICpiZXYsCiAJCWlm
IChjcmUtPmxpbmUgIT0gMSkgewogCQkJaWYgKGNyZS0+ZGlyID09IFJFTEFZX0RJUl9SRVFVRVNU
KSB7CiAJCQkJaWYgKHN0cmNhc2VjbXAoIkNvbm5lY3Rpb24iLCBrZXkpID09IDAgJiYKLQkJCQkg
ICAgc3RyY2FzZWNtcCgiVXBncmFkZSIsIHZhbHVlKSA9PSAwKQorCQkJCSAgICBzdHJjYXNlY21w
KCJVcGdyYWRlIiwgdmFsdWUpID49IDApCiAJCQkJCXByaXYtPmh0dHBfdXBncmFkZV9yZXEgfD0K
IAkJCQkJICAgIEhUVFBfQ09OTkVDVElPTl9VUEdSQURFOwogCQkJCWlmIChzdHJjYXNlY21wKCJV
cGdyYWRlIiwga2V5KSA9PSAwICYmCg==
====

Reply | Threaded
Open this post in threaded view
|

Re: relayd websocket

Claudio Jeker
On Tue, Mar 05, 2019 at 04:21:30PM +0000, 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 -0000 1.72
> +++ usr.sbin/relayd/relay_http.c 5 Mar 2019 16:03:56 -0000
> @@ -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

Reply | Threaded
Open this post in threaded view
|

Re: relayd websocket

Rivo Nurges-2
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.c 4 Mar 2019 21:25:03 -0000 1.72
+++ usr.sbin/relayd/relay_http.c 5 Mar 2019 22:33:47 -0000
@@ -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(&valuecopy, ", ")) != 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=
====






Reply | Threaded
Open this post in threaded view
|

Re: relayd websocket

Sebastian Benoit-3
Rivo Nurges([hidden email]) on 2019.03.05 22:42:13 +0000:

> 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 -0000 1.72
> +++ usr.sbin/relayd/relay_http.c 5 Mar 2019 22:33:47 -0000
> @@ -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(&valuecopy, ", ")) != 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=
> ====
>
>
>
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: relayd websocket

Rivo Nurges-2
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

Reply | Threaded
Open this post in threaded view
|

Re: relayd websocket

Rivo Nurges-2
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 -0000 1.72
+++ usr.sbin/relayd/relay_http.c 6 Mar 2019 20:53:59 -0000
@@ -36,6 +36,7 @@
  #include <siphash.h>
  #include <imsg.h>
  #include <unistd.h>
+#include <ctype.h>

  #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(&valuecopy,
+ ",")) != NULL) {
+ while (isblank(*valuepart))
+    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
bHVlKSA9PSAwKQogCQkJCQlwcml2LT5odHRwX3VwZ3JhZGVfcmVxIHw9Cg==
====

Reply | Threaded
Open this post in threaded view
|

Re: relayd websocket

Rivo Nurges-2
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 -0000 1.72
> +++ usr.sbin/relayd/relay_http.c 6 Mar 2019 20:53:59 -0000
> @@ -36,6 +36,7 @@
>    #include <siphash.h>
>    #include <imsg.h>
>    #include <unistd.h>
> +#include <ctype.h>
>
>    #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(&valuecopy,
> + ",")) != NULL) {
> + while (isblank(*valuepart))
> +    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
> bHVlKSA9PSAwKQogCQkJCQlwcml2LT5odHRwX3VwZ3JhZGVfcmVxIHw9Cg==
> ====
>


Reply | Threaded
Open this post in threaded view
|

Re: relayd websocket

Rivo Nurges-2
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 -0000 1.72
>> +++ usr.sbin/relayd/relay_http.c 6 Mar 2019 20:53:59 -0000
>> @@ -36,6 +36,7 @@
>>     #include <siphash.h>
>>     #include <imsg.h>
>>     #include <unistd.h>
>> +#include <ctype.h>
>>
>>     #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(&valuecopy,
>> + ",")) != NULL) {
>> + while (isblank(*valuepart))
>> +    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
>> bHVlKSA9PSAwKQogCQkJCQlwcml2LT5odHRwX3VwZ3JhZGVfcmVxIHw9Cg==
>> ====
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: relayd websocket

Reyk Floeter-2
In reply to this post by Sebastian Benoit-3
On Wed, Mar 06, 2019 at 05:36:32PM +0100, Sebastian Benoit wrote:

> Rivo Nurges([hidden email]) on 2019.03.05 22:42:13 +0000:
> > 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 -0000 1.10
+++ usr.sbin/relayd/http.h 8 May 2019 16:17:38 -0000
@@ -251,10 +251,4 @@ struct http_descriptor {
  struct kvtree http_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 -0000 1.242
+++ usr.sbin/relayd/relay.c 8 May 2019 16:17:39 -0000
@@ -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 -0000 1.72
+++ usr.sbin/relayd/relay_http.c 8 May 2019 16:17:40 -0000
@@ -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(&con->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)
- priv->http_upgrade_req |=
-    HTTP_CONNECTION_UPGRADE;
- if (strcasecmp("Upgrade", key) == 0 &&
-    strcasecmp("websocket", value) == 0)
- priv->http_upgrade_req |=
-    HTTP_UPGRADE_WEBSOCKET;
- }
-
  if ((hdr = kv_add(&desc->http_headers, key,
     value, unique)) == NULL) {
  relay_abort_http(con, 400,
@@ -445,37 +423,34 @@ relay_read_http(struct bufferevent *bev,
  return;
  }
 
- /* HTTP 101 Switching Protocols */
- if (cre->dir == RELAY_DIR_REQUEST) {
- if ((priv->http_upgrade_req & HTTP_UPGRADE_WEBSOCKET) &&
-    !(proto->httpflags & HTTPFLAG_WEBSOCKETS)) {
+ /*
+ * HTTP 101 Switching Protocols
+ */
+ upgrade = kv_find_value(&desc->http_headers,
+    "Connection", "upgrade", ",");
+ upgrade_ws = kv_find_value(&desc->http_headers,
+    "Upgrade", "websocket", ",");
+ if (cre->dir == RELAY_DIR_REQUEST && upgrade_ws != NULL) {
+ if ((proto->httpflags & HTTPFLAG_WEBSOCKETS) == 0) {
  relay_abort_http(con, 403,
     "Websocket Forbidden", 0);
  return;
- }
- if ((priv->http_upgrade_req & HTTP_UPGRADE_WEBSOCKET) &&
-    !(priv->http_upgrade_req & HTTP_CONNECTION_UPGRADE))
- {
+ } else if (upgrade == NULL) {
  relay_abort_http(con, 400,
     "Bad Websocket Request", 0);
  return;
- }
- if ((priv->http_upgrade_req & HTTP_UPGRADE_WEBSOCKET) &&
-    (desc->http_method != HTTP_METHOD_GET)) {
+ } else if (desc->http_method != HTTP_METHOD_GET) {
  relay_abort_http(con, 405,
     "Websocket Method Not Allowed", 0);
  return;
  }
  } else if (cre->dir == RELAY_DIR_RESPONSE &&
     desc->http_status == 101) {
- if (((priv->http_upgrade_req &
-    (HTTP_CONNECTION_UPGRADE | HTTP_UPGRADE_WEBSOCKET))
-    ==
-    (HTTP_CONNECTION_UPGRADE | HTTP_UPGRADE_WEBSOCKET))
-    && proto->httpflags & HTTPFLAG_WEBSOCKETS) {
+ if (upgrade_ws != NULL && upgrade != NULL &&
+    (proto->httpflags & HTTPFLAG_WEBSOCKETS)) {
  cre->dst->toread = TOREAD_UNLIMITED;
  cre->dst->bev->readcb = relay_read;
- }  else  {
+ } else {
  relay_abort_http(con, 502,
     "Bad Websocket Gateway", 0);
  return;
Index: usr.sbin/relayd/relayd.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
retrieving revision 1.175
diff -u -p -u -p -r1.175 relayd.c
--- usr.sbin/relayd/relayd.c 24 Apr 2019 19:13:49 -0000 1.175
+++ usr.sbin/relayd/relayd.c 8 May 2019 16:17:40 -0000
@@ -812,6 +812,45 @@ kv_find(struct kvtree *keys, struct kv *
  return (match);
 }
 
+struct kv *
+kv_find_value(struct kvtree *keys, char *key, const char *value,
+    const char *delim)
+{
+ struct kv *match, kv;
+ char *val = NULL, *next, *ptr;
+
+ kv.kv_key = key;
+ if ((match = RB_FIND(kvtree, keys, &kv)) == NULL)
+ return (NULL);
+
+ if (match->kv_value == NULL)
+ return (NULL);
+
+ if (delim == NULL) {
+ if (strcasestr(match->kv_value, value) == NULL)
+ return (NULL);
+ } else {
+ if ((val = strdup(match->kv_value)) == NULL)
+ return (NULL);
+ for (next = ptr = val; ptr != NULL;
+    ptr = strsep(&next, delim)) {
+ /* strip leading whitespace or newlines */
+ ptr += strspn(ptr, " \t\r\n");
+ if (strcasecmp(ptr, value) == 0)
+ goto done;
+ }
+ match = NULL;
+ }
+
+ done:
+#ifdef DEBUG
+ if (match != NULL)
+ DPRINTF("%s: matched %s: %s", __func__, key, value);
+#endif
+ free(val);
+ return (match);
+}
+
 int
 kv_cmp(struct kv *a, struct kv *b)
 {
Index: usr.sbin/relayd/relayd.h
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relayd.h,v
retrieving revision 1.252
diff -u -p -u -p -r1.252 relayd.h
--- usr.sbin/relayd/relayd.h 4 Mar 2019 21:25:03 -0000 1.252
+++ usr.sbin/relayd/relayd.h 8 May 2019 16:17:41 -0000
@@ -1324,6 +1324,8 @@ void relay_log(struct rsession *, char
 int kv_log(struct rsession *, struct kv *, u_int16_t,
      enum direction);
 struct kv *kv_find(struct kvtree *, struct kv *);
+struct kv *kv_find_value(struct kvtree *, char *, const char *,
+     const char *);
 int kv_cmp(struct kv *, struct kv *);
 int rule_add(struct protocol *, struct relay_rule *, const char
      *);

Reply | Threaded
Open this post in threaded view
|

Re: relayd websocket

Reyk Floeter-2
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([hidden email]) on 2019.03.05 22:42:13 +0000:
> > > 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 -0000 1.10
+++ usr.sbin/relayd/http.h 8 May 2019 16:55:59 -0000
@@ -251,10 +251,4 @@ struct http_descriptor {
  struct kvtree http_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 -0000 1.242
+++ usr.sbin/relayd/relay.c 8 May 2019 16:56:00 -0000
@@ -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 -0000 1.72
+++ usr.sbin/relayd/relay_http.c 8 May 2019 16:56:01 -0000
@@ -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(&con->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)
- priv->http_upgrade_req |=
-    HTTP_CONNECTION_UPGRADE;
- if (strcasecmp("Upgrade", key) == 0 &&
-    strcasecmp("websocket", value) == 0)
- priv->http_upgrade_req |=
-    HTTP_UPGRADE_WEBSOCKET;
- }
-
  if ((hdr = kv_add(&desc->http_headers, key,
     value, unique)) == NULL) {
  relay_abort_http(con, 400,
@@ -445,37 +423,34 @@ relay_read_http(struct bufferevent *bev,
  return;
  }
 
- /* HTTP 101 Switching Protocols */
- if (cre->dir == RELAY_DIR_REQUEST) {
- if ((priv->http_upgrade_req & HTTP_UPGRADE_WEBSOCKET) &&
-    !(proto->httpflags & HTTPFLAG_WEBSOCKETS)) {
+ /*
+ * HTTP 101 Switching Protocols
+ */
+ upgrade = kv_find_value(&desc->http_headers,
+    "Connection", "upgrade", ",");
+ upgrade_ws = kv_find_value(&desc->http_headers,
+    "Upgrade", "websocket", ",");
+ if (cre->dir == RELAY_DIR_REQUEST && upgrade_ws != NULL) {
+ if ((proto->httpflags & HTTPFLAG_WEBSOCKETS) == 0) {
  relay_abort_http(con, 403,
     "Websocket Forbidden", 0);
  return;
- }
- if ((priv->http_upgrade_req & HTTP_UPGRADE_WEBSOCKET) &&
-    !(priv->http_upgrade_req & HTTP_CONNECTION_UPGRADE))
- {
+ } else if (upgrade == NULL) {
  relay_abort_http(con, 400,
     "Bad Websocket Request", 0);
  return;
- }
- if ((priv->http_upgrade_req & HTTP_UPGRADE_WEBSOCKET) &&
-    (desc->http_method != HTTP_METHOD_GET)) {
+ } else if (desc->http_method != HTTP_METHOD_GET) {
  relay_abort_http(con, 405,
     "Websocket Method Not Allowed", 0);
  return;
  }
  } else if (cre->dir == RELAY_DIR_RESPONSE &&
     desc->http_status == 101) {
- if (((priv->http_upgrade_req &
-    (HTTP_CONNECTION_UPGRADE | HTTP_UPGRADE_WEBSOCKET))
-    ==
-    (HTTP_CONNECTION_UPGRADE | HTTP_UPGRADE_WEBSOCKET))
-    && proto->httpflags & HTTPFLAG_WEBSOCKETS) {
+ if (upgrade_ws != NULL && upgrade != NULL &&
+    (proto->httpflags & HTTPFLAG_WEBSOCKETS)) {
  cre->dst->toread = TOREAD_UNLIMITED;
  cre->dst->bev->readcb = relay_read;
- }  else  {
+ } else {
  relay_abort_http(con, 502,
     "Bad Websocket Gateway", 0);
  return;
Index: usr.sbin/relayd/relayd.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
retrieving revision 1.175
diff -u -p -u -p -r1.175 relayd.c
--- usr.sbin/relayd/relayd.c 24 Apr 2019 19:13:49 -0000 1.175
+++ usr.sbin/relayd/relayd.c 8 May 2019 16:56:02 -0000
@@ -812,6 +812,46 @@ kv_find(struct kvtree *keys, struct kv *
  return (match);
 }
 
+struct kv *
+kv_find_value(struct kvtree *keys, char *key, const char *value,
+    const char *delim)
+{
+ struct kv *match, kv;
+ char *val = NULL, *next, *ptr;
+
+ kv.kv_key = key;
+ if ((match = RB_FIND(kvtree, keys, &kv)) == NULL)
+ return (NULL);
+
+ if (match->kv_value == NULL)
+ return (NULL);
+
+ if (delim == NULL) {
+ if (strcasecmp(match->kv_value, value) == 0)
+ goto done;
+ } else {
+ if ((val = strdup(match->kv_value)) == NULL)
+ return (NULL);
+ for (next = ptr = val; ptr != NULL;
+    ptr = strsep(&next, delim)) {
+ /* strip leading whitespace or newlines */
+ ptr += strspn(ptr, " \t\r\n");
+ if (strcasecmp(ptr, value) == 0)
+ goto done;
+ }
+ }
+
+ /* not matched */
+ match = NULL;
+ done:
+#ifdef DEBUG
+ if (match != NULL)
+ DPRINTF("%s: matched %s: %s", __func__, key, value);
+#endif
+ free(val);
+ return (match);
+}
+
 int
 kv_cmp(struct kv *a, struct kv *b)
 {
Index: usr.sbin/relayd/relayd.h
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relayd.h,v
retrieving revision 1.252
diff -u -p -u -p -r1.252 relayd.h
--- usr.sbin/relayd/relayd.h 4 Mar 2019 21:25:03 -0000 1.252
+++ usr.sbin/relayd/relayd.h 8 May 2019 16:56:02 -0000
@@ -1324,6 +1324,8 @@ void relay_log(struct rsession *, char
 int kv_log(struct rsession *, struct kv *, u_int16_t,
      enum direction);
 struct kv *kv_find(struct kvtree *, struct kv *);
+struct kv *kv_find_value(struct kvtree *, char *, const char *,
+     const char *);
 int kv_cmp(struct kv *, struct kv *);
 int rule_add(struct protocol *, struct relay_rule *, const char
      *);

Reply | Threaded
Open this post in threaded view
|

Re: relayd websocket

Reyk Floeter-2
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([hidden email]) on 2019.03.05 22:42:13 +0000:
> > > > 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 -0000 1.10
+++ usr.sbin/relayd/http.h 8 May 2019 18:34:09 -0000
@@ -251,10 +251,4 @@ struct http_descriptor {
  struct kvtree http_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 -0000 1.242
+++ usr.sbin/relayd/relay.c 8 May 2019 18:34:09 -0000
@@ -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 -0000 1.72
+++ usr.sbin/relayd/relay_http.c 8 May 2019 18:34:09 -0000
@@ -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(&con->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)
- priv->http_upgrade_req |=
-    HTTP_CONNECTION_UPGRADE;
- if (strcasecmp("Upgrade", key) == 0 &&
-    strcasecmp("websocket", value) == 0)
- priv->http_upgrade_req |=
-    HTTP_UPGRADE_WEBSOCKET;
- }
-
  if ((hdr = kv_add(&desc->http_headers, key,
     value, unique)) == NULL) {
  relay_abort_http(con, 400,
@@ -445,37 +423,34 @@ relay_read_http(struct bufferevent *bev,
  return;
  }
 
- /* HTTP 101 Switching Protocols */
- if (cre->dir == RELAY_DIR_REQUEST) {
- if ((priv->http_upgrade_req & HTTP_UPGRADE_WEBSOCKET) &&
-    !(proto->httpflags & HTTPFLAG_WEBSOCKETS)) {
+ /*
+ * HTTP 101 Switching Protocols
+ */
+ upgrade = kv_find_value(&desc->http_headers,
+    "Connection", "upgrade", ",");
+ upgrade_ws = kv_find_value(&desc->http_headers,
+    "Upgrade", "websocket", ",");
+ if (cre->dir == RELAY_DIR_REQUEST && upgrade_ws != NULL) {
+ if ((proto->httpflags & HTTPFLAG_WEBSOCKETS) == 0) {
  relay_abort_http(con, 403,
     "Websocket Forbidden", 0);
  return;
- }
- if ((priv->http_upgrade_req & HTTP_UPGRADE_WEBSOCKET) &&
-    !(priv->http_upgrade_req & HTTP_CONNECTION_UPGRADE))
- {
+ } else if (upgrade == NULL) {
  relay_abort_http(con, 400,
     "Bad Websocket Request", 0);
  return;
- }
- if ((priv->http_upgrade_req & HTTP_UPGRADE_WEBSOCKET) &&
-    (desc->http_method != HTTP_METHOD_GET)) {
+ } else if (desc->http_method != HTTP_METHOD_GET) {
  relay_abort_http(con, 405,
     "Websocket Method Not Allowed", 0);
  return;
  }
  } else if (cre->dir == RELAY_DIR_RESPONSE &&
     desc->http_status == 101) {
- if (((priv->http_upgrade_req &
-    (HTTP_CONNECTION_UPGRADE | HTTP_UPGRADE_WEBSOCKET))
-    ==
-    (HTTP_CONNECTION_UPGRADE | HTTP_UPGRADE_WEBSOCKET))
-    && proto->httpflags & HTTPFLAG_WEBSOCKETS) {
+ if (upgrade_ws != NULL && upgrade != NULL &&
+    (proto->httpflags & HTTPFLAG_WEBSOCKETS)) {
  cre->dst->toread = TOREAD_UNLIMITED;
  cre->dst->bev->readcb = relay_read;
- }  else  {
+ } else {
  relay_abort_http(con, 502,
     "Bad Websocket Gateway", 0);
  return;
Index: usr.sbin/relayd/relayd.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
retrieving revision 1.175
diff -u -p -u -p -r1.175 relayd.c
--- usr.sbin/relayd/relayd.c 24 Apr 2019 19:13:49 -0000 1.175
+++ usr.sbin/relayd/relayd.c 8 May 2019 18:34:09 -0000
@@ -812,6 +812,48 @@ kv_find(struct kvtree *keys, struct kv *
  return (match);
 }
 
+struct kv *
+kv_find_value(struct kvtree *keys, char *key, const char *value,
+    const char *delim)
+{
+ struct kv *match, kv;
+ char *val = NULL, *next, *ptr;
+ size_t len;
+
+ kv.kv_key = key;
+ if ((match = RB_FIND(kvtree, keys, &kv)) == NULL)
+ return (NULL);
+
+ if (match->kv_value == NULL)
+ return (NULL);
+
+ if (delim == NULL) {
+ if (strcasecmp(match->kv_value, value) == 0)
+ goto done;
+ } else {
+ if ((val = strdup(match->kv_value)) == NULL)
+ return (NULL);
+ for (next = ptr = val; ptr != NULL;
+    ptr = strsep(&next, delim)) {
+ /* strip whitespace */
+ ptr += strspn(ptr, " \t");
+ len = strcspn(ptr, " \t");
+ if (strncasecmp(ptr, value, len) == 0)
+ goto done;
+ }
+ }
+
+ /* not matched */
+ match = NULL;
+ done:
+#ifdef DEBUG
+ if (match != NULL)
+ DPRINTF("%s: matched %s: %s", __func__, key, value);
+#endif
+ free(val);
+ return (match);
+}
+
 int
 kv_cmp(struct kv *a, struct kv *b)
 {
Index: usr.sbin/relayd/relayd.h
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relayd.h,v
retrieving revision 1.252
diff -u -p -u -p -r1.252 relayd.h
--- usr.sbin/relayd/relayd.h 4 Mar 2019 21:25:03 -0000 1.252
+++ usr.sbin/relayd/relayd.h 8 May 2019 18:34:10 -0000
@@ -1324,6 +1324,8 @@ void relay_log(struct rsession *, char
 int kv_log(struct rsession *, struct kv *, u_int16_t,
      enum direction);
 struct kv *kv_find(struct kvtree *, struct kv *);
+struct kv *kv_find_value(struct kvtree *, char *, const char *,
+     const char *);
 int kv_cmp(struct kv *, struct kv *);
 int rule_add(struct protocol *, struct relay_rule *, const char
      *);

Reply | Threaded
Open this post in threaded view
|

Re: relayd websocket

Rivo Nurges-2
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([hidden email]) on 2019.03.05 22:42:13 +0000:
>>>>> 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 -0000 1.10
> +++ usr.sbin/relayd/http.h 8 May 2019 18:34:09 -0000
> @@ -251,10 +251,4 @@ struct http_descriptor {
>   struct kvtree http_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 -0000 1.242
> +++ usr.sbin/relayd/relay.c 8 May 2019 18:34:09 -0000
> @@ -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 -0000 1.72
> +++ usr.sbin/relayd/relay_http.c 8 May 2019 18:34:09 -0000
> @@ -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(&con->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)
> - priv->http_upgrade_req |=
> -    HTTP_CONNECTION_UPGRADE;
> - if (strcasecmp("Upgrade", key) == 0 &&
> -    strcasecmp("websocket", value) == 0)
> - priv->http_upgrade_req |=
> -    HTTP_UPGRADE_WEBSOCKET;
> - }
> -
>   if ((hdr = kv_add(&desc->http_headers, key,
>      value, unique)) == NULL) {
>   relay_abort_http(con, 400,
> @@ -445,37 +423,34 @@ relay_read_http(struct bufferevent *bev,
>   return;
>   }
>  
> - /* HTTP 101 Switching Protocols */
> - if (cre->dir == RELAY_DIR_REQUEST) {
> - if ((priv->http_upgrade_req & HTTP_UPGRADE_WEBSOCKET) &&
> -    !(proto->httpflags & HTTPFLAG_WEBSOCKETS)) {
> + /*
> + * HTTP 101 Switching Protocols
> + */
> + upgrade = kv_find_value(&desc->http_headers,
> +    "Connection", "upgrade", ",");
> + upgrade_ws = kv_find_value(&desc->http_headers,
> +    "Upgrade", "websocket", ",");
> + if (cre->dir == RELAY_DIR_REQUEST && upgrade_ws != NULL) {
> + if ((proto->httpflags & HTTPFLAG_WEBSOCKETS) == 0) {
>   relay_abort_http(con, 403,
>      "Websocket Forbidden", 0);
>   return;
> - }
> - if ((priv->http_upgrade_req & HTTP_UPGRADE_WEBSOCKET) &&
> -    !(priv->http_upgrade_req & HTTP_CONNECTION_UPGRADE))
> - {
> + } else if (upgrade == NULL) {
>   relay_abort_http(con, 400,
>      "Bad Websocket Request", 0);
>   return;
> - }
> - if ((priv->http_upgrade_req & HTTP_UPGRADE_WEBSOCKET) &&
> -    (desc->http_method != HTTP_METHOD_GET)) {
> + } else if (desc->http_method != HTTP_METHOD_GET) {
>   relay_abort_http(con, 405,
>      "Websocket Method Not Allowed", 0);
>   return;
>   }
>   } else if (cre->dir == RELAY_DIR_RESPONSE &&
>      desc->http_status == 101) {
> - if (((priv->http_upgrade_req &
> -    (HTTP_CONNECTION_UPGRADE | HTTP_UPGRADE_WEBSOCKET))
> -    ==
> -    (HTTP_CONNECTION_UPGRADE | HTTP_UPGRADE_WEBSOCKET))
> -    && proto->httpflags & HTTPFLAG_WEBSOCKETS) {
> + if (upgrade_ws != NULL && upgrade != NULL &&
> +    (proto->httpflags & HTTPFLAG_WEBSOCKETS)) {
>   cre->dst->toread = TOREAD_UNLIMITED;
>   cre->dst->bev->readcb = relay_read;
> - }  else  {
> + } else {
>   relay_abort_http(con, 502,
>      "Bad Websocket Gateway", 0);
>   return;
> Index: usr.sbin/relayd/relayd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
> retrieving revision 1.175
> diff -u -p -u -p -r1.175 relayd.c
> --- usr.sbin/relayd/relayd.c 24 Apr 2019 19:13:49 -0000 1.175
> +++ usr.sbin/relayd/relayd.c 8 May 2019 18:34:09 -0000
> @@ -812,6 +812,48 @@ kv_find(struct kvtree *keys, struct kv *
>   return (match);
>   }
>  
> +struct kv *
> +kv_find_value(struct kvtree *keys, char *key, const char *value,
> +    const char *delim)
> +{
> + struct kv *match, kv;
> + char *val = NULL, *next, *ptr;
> + size_t len;
> +
> + kv.kv_key = key;
> + if ((match = RB_FIND(kvtree, keys, &kv)) == NULL)
> + return (NULL);
> +
> + if (match->kv_value == NULL)
> + return (NULL);
> +
> + if (delim == NULL) {
> + if (strcasecmp(match->kv_value, value) == 0)
> + goto done;
> + } else {
> + if ((val = strdup(match->kv_value)) == NULL)
> + return (NULL);
> + for (next = ptr = val; ptr != NULL;
> +    ptr = strsep(&next, delim)) {
> + /* strip whitespace */
> + ptr += strspn(ptr, " \t");
> + len = strcspn(ptr, " \t");
> + if (strncasecmp(ptr, value, len) == 0)
> + goto done;
> + }
> + }
> +
> + /* not matched */
> + match = NULL;
> + done:
> +#ifdef DEBUG
> + if (match != NULL)
> + DPRINTF("%s: matched %s: %s", __func__, key, value);
> +#endif
> + free(val);
> + return (match);
> +}
> +
>   int
>   kv_cmp(struct kv *a, struct kv *b)
>   {
> Index: usr.sbin/relayd/relayd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/relayd.h,v
> retrieving revision 1.252
> diff -u -p -u -p -r1.252 relayd.h
> --- usr.sbin/relayd/relayd.h 4 Mar 2019 21:25:03 -0000 1.252
> +++ usr.sbin/relayd/relayd.h 8 May 2019 18:34:10 -0000
> @@ -1324,6 +1324,8 @@ void relay_log(struct rsession *, char
>   int kv_log(struct rsession *, struct kv *, u_int16_t,
>       enum direction);
>   struct kv *kv_find(struct kvtree *, struct kv *);
> +struct kv *kv_find_value(struct kvtree *, char *, const char *,
> +     const char *);
>   int kv_cmp(struct kv *, struct kv *);
>   int rule_add(struct protocol *, struct relay_rule *, const char
>       *);
>
>

Reply | Threaded
Open this post in threaded view
|

Re: relayd websocket

Sebastian Benoit-3
In reply to this post by Reyk Floeter-2

ok benno@

Reyk Floeter([hidden email]) 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([hidden email]) on 2019.03.05 22:42:13 +0000:
> > > > > 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 -0000 1.10
> +++ usr.sbin/relayd/http.h 8 May 2019 18:34:09 -0000
> @@ -251,10 +251,4 @@ struct http_descriptor {
>   struct kvtree http_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 -0000 1.242
> +++ usr.sbin/relayd/relay.c 8 May 2019 18:34:09 -0000
> @@ -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 -0000 1.72
> +++ usr.sbin/relayd/relay_http.c 8 May 2019 18:34:09 -0000
> @@ -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(&con->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)
> - priv->http_upgrade_req |=
> -    HTTP_CONNECTION_UPGRADE;
> - if (strcasecmp("Upgrade", key) == 0 &&
> -    strcasecmp("websocket", value) == 0)
> - priv->http_upgrade_req |=
> -    HTTP_UPGRADE_WEBSOCKET;
> - }
> -
>   if ((hdr = kv_add(&desc->http_headers, key,
>      value, unique)) == NULL) {
>   relay_abort_http(con, 400,
> @@ -445,37 +423,34 @@ relay_read_http(struct bufferevent *bev,
>   return;
>   }
>  
> - /* HTTP 101 Switching Protocols */
> - if (cre->dir == RELAY_DIR_REQUEST) {
> - if ((priv->http_upgrade_req & HTTP_UPGRADE_WEBSOCKET) &&
> -    !(proto->httpflags & HTTPFLAG_WEBSOCKETS)) {
> + /*
> + * HTTP 101 Switching Protocols
> + */
> + upgrade = kv_find_value(&desc->http_headers,
> +    "Connection", "upgrade", ",");
> + upgrade_ws = kv_find_value(&desc->http_headers,
> +    "Upgrade", "websocket", ",");
> + if (cre->dir == RELAY_DIR_REQUEST && upgrade_ws != NULL) {
> + if ((proto->httpflags & HTTPFLAG_WEBSOCKETS) == 0) {
>   relay_abort_http(con, 403,
>      "Websocket Forbidden", 0);
>   return;
> - }
> - if ((priv->http_upgrade_req & HTTP_UPGRADE_WEBSOCKET) &&
> -    !(priv->http_upgrade_req & HTTP_CONNECTION_UPGRADE))
> - {
> + } else if (upgrade == NULL) {
>   relay_abort_http(con, 400,
>      "Bad Websocket Request", 0);
>   return;
> - }
> - if ((priv->http_upgrade_req & HTTP_UPGRADE_WEBSOCKET) &&
> -    (desc->http_method != HTTP_METHOD_GET)) {
> + } else if (desc->http_method != HTTP_METHOD_GET) {
>   relay_abort_http(con, 405,
>      "Websocket Method Not Allowed", 0);
>   return;
>   }
>   } else if (cre->dir == RELAY_DIR_RESPONSE &&
>      desc->http_status == 101) {
> - if (((priv->http_upgrade_req &
> -    (HTTP_CONNECTION_UPGRADE | HTTP_UPGRADE_WEBSOCKET))
> -    ==
> -    (HTTP_CONNECTION_UPGRADE | HTTP_UPGRADE_WEBSOCKET))
> -    && proto->httpflags & HTTPFLAG_WEBSOCKETS) {
> + if (upgrade_ws != NULL && upgrade != NULL &&
> +    (proto->httpflags & HTTPFLAG_WEBSOCKETS)) {
>   cre->dst->toread = TOREAD_UNLIMITED;
>   cre->dst->bev->readcb = relay_read;
> - }  else  {
> + } else {
>   relay_abort_http(con, 502,
>      "Bad Websocket Gateway", 0);
>   return;
> Index: usr.sbin/relayd/relayd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
> retrieving revision 1.175
> diff -u -p -u -p -r1.175 relayd.c
> --- usr.sbin/relayd/relayd.c 24 Apr 2019 19:13:49 -0000 1.175
> +++ usr.sbin/relayd/relayd.c 8 May 2019 18:34:09 -0000
> @@ -812,6 +812,48 @@ kv_find(struct kvtree *keys, struct kv *
>   return (match);
>  }
>  
> +struct kv *
> +kv_find_value(struct kvtree *keys, char *key, const char *value,
> +    const char *delim)
> +{
> + struct kv *match, kv;
> + char *val = NULL, *next, *ptr;
> + size_t len;
> +
> + kv.kv_key = key;
> + if ((match = RB_FIND(kvtree, keys, &kv)) == NULL)
> + return (NULL);
> +
> + if (match->kv_value == NULL)
> + return (NULL);
> +
> + if (delim == NULL) {
> + if (strcasecmp(match->kv_value, value) == 0)
> + goto done;
> + } else {
> + if ((val = strdup(match->kv_value)) == NULL)
> + return (NULL);
> + for (next = ptr = val; ptr != NULL;
> +    ptr = strsep(&next, delim)) {
> + /* strip whitespace */
> + ptr += strspn(ptr, " \t");
> + len = strcspn(ptr, " \t");
> + if (strncasecmp(ptr, value, len) == 0)
> + goto done;
> + }
> + }
> +
> + /* not matched */
> + match = NULL;
> + done:
> +#ifdef DEBUG
> + if (match != NULL)
> + DPRINTF("%s: matched %s: %s", __func__, key, value);
> +#endif
> + free(val);
> + return (match);
> +}
> +
>  int
>  kv_cmp(struct kv *a, struct kv *b)
>  {
> Index: usr.sbin/relayd/relayd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/relayd.h,v
> retrieving revision 1.252
> diff -u -p -u -p -r1.252 relayd.h
> --- usr.sbin/relayd/relayd.h 4 Mar 2019 21:25:03 -0000 1.252
> +++ usr.sbin/relayd/relayd.h 8 May 2019 18:34:10 -0000
> @@ -1324,6 +1324,8 @@ void relay_log(struct rsession *, char
>  int kv_log(struct rsession *, struct kv *, u_int16_t,
>       enum direction);
>  struct kv *kv_find(struct kvtree *, struct kv *);
> +struct kv *kv_find_value(struct kvtree *, char *, const char *,
> +     const char *);
>  int kv_cmp(struct kv *, struct kv *);
>  int rule_add(struct protocol *, struct relay_rule *, const char
>       *);
>