relayd silently ignores ill-defined protocol mixtures

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

relayd silently ignores ill-defined protocol mixtures

Nick Guenther-2
>Synopsis: relayd silently ignores ill-defined protocol mixtures
>Category: system
>Environment:
        System      : OpenBSD 6.6
        Details     : OpenBSD 6.6 (GENERIC.MP) #372: Sat Oct 12 10:56:27 MDT 2019
                         [hidden email]:/usr/src/sys/arch/amd64/compile/GENERIC.MP

        Architecture: OpenBSD.amd64
        Machine     : amd64

>Description:

        a. Multiple `protocol`s lines are allowed in a relay but only the *last* defines
        what happens. It would be so much more helpful if this was a straight error.
       
        b. tcp/tls/http options can be set in `dns protocol`s -- though they are ignored.
        This should also be an error.

>How-To-Repeat:
       
        a.
       
        This relayd speaks DNS on UDP port 80:
       
        ```
        $ cat > relayd.conf <<EOF
        table <web> { "127.0.0.1" }
        table <app> { "127.0.0.1" }
       
        http protocol A {
                return error
       
                match request path "/app/*" forward to <app>
        }
       
        dns protocol B {
                return error
        }
       
        relay P {
                listen on 0.0.0.0 port 80
       
                protocol A
                forward to <web> port 8080
       
                protocol B
                forward to <app> port 8082
        }
        EOF
        $ doas relayd -f relayd.conf
        ```
       
        ```
        $ fstat | grep relayd | grep internet
        _relayd  relayd     29047   10* internet dgram udp *:80
        _relayd  relayd     40704   10* internet dgram udp *:80
        _relayd  relayd     21221   10* internet dgram udp *:80
        _relayd  relayd     88624    4* internet raw icmp 0x0
        _relayd  relayd     88624    5* internet raw icmp 0x0
        _relayd  relayd     88624    6* internet6 raw ipv6-icmp 0x0
        _relayd  relayd     88624    7* internet6 raw ipv6-icmp 0x0
       
        ```

        while this one speaks http:

        ```
        $ cat > relayd.conf <<EOF
        table <web> { "127.0.0.1" }
        table <app> { "127.0.0.1" }

        dns protocol A {
                return error

                match request path "/app/*" forward to <app>
        }

        http protocol B {
                return error
        }

        relay P {
                listen on 0.0.0.0 port 80

                protocol A
                forward to <web> port 8080

                protocol B
                forward to <app> port 8082
        }
        EOF
        $ doas relayd -f relayd.conf
        ```

        ```
        $ fstat | grep relayd | grep internet
        _relayd  relayd      2564   10* internet stream tcp 0x0 *:80
        _relayd  relayd     29428   10* internet stream tcp 0x0 *:80
        _relayd  relayd     61476   10* internet stream tcp 0x0 *:80
        _relayd  relayd     90200    4* internet raw icmp 0x0
        _relayd  relayd     90200    5* internet raw icmp 0x0
        _relayd  relayd     90200    6* internet6 raw ipv6-icmp 0x0
        _relayd  relayd     90200    7* internet6 raw ipv6-icmp 0x0
        $
        $ curl http://localhost # demonstrate that relayd is speaking http
        <!DOCTYPE html>
        <html>
        <head>
        <title>500 Internal Server Error</title>
        <style type="text/css"><!--
        body { background-color: #a00000; color: white; font-family: 'Comic Sans MS', 'Chalkboard SE', 'Comic Neue', sans-serif; }
        hr { border: 0; border-bottom: 1px dashed; }

        --></style>
        </head>
        <body>
        <h1>Internal Server Error</h1>
        <div id='m'></div>
        <div id='l'></div>
        <hr><address>OpenBSD relayd at 0.0.0.0 port 80</address>
        </body>
        </html>
        ```

        b.

        You can see in the latter example that `match request path` is legal in
        `dns protocol` blocks which makes no sense.

Reply | Threaded
Open this post in threaded view
|

Re: relayd silently ignores ill-defined protocol mixtures

Sebastian Benoit-3
Nick([hidden email]) on 2020.02.11 06:41:32 +0000:

> >Synopsis: relayd silently ignores ill-defined protocol mixtures
> >Category: system
> >Environment:
> System      : OpenBSD 6.6
> Details     : OpenBSD 6.6 (GENERIC.MP) #372: Sat Oct 12 10:56:27 MDT 2019
> [hidden email]:/usr/src/sys/arch/amd64/compile/GENERIC.MP
>
> Architecture: OpenBSD.amd64
> Machine     : amd64
>
> >Description:
>
> a. Multiple `protocol`s lines are allowed in a relay but only the *last* defines
> what happens. It would be so much more helpful if this was a straight error.

 
> b. tcp/tls/http options can be set in `dns protocol`s -- though they are ignored.
> This should also be an error.

Here is a diff for both a. and b.

ok?

(benno_relayd_protocol_only_once.diff)

diff --git usr.sbin/relayd/parse.y usr.sbin/relayd/parse.y
index 2fad6bee5bf..cac18491d87 100644
--- usr.sbin/relayd/parse.y
+++ usr.sbin/relayd/parse.y
@@ -1088,7 +1088,6 @@ proto : relay_proto PROTO STRING {
  yyerror("invalid TLS protocol");
  YYERROR;
  }
-
  TAILQ_INSERT_TAIL(conf->sc_protos, proto, entry);
  }
  ;
@@ -1102,10 +1101,38 @@ protopts_l : protopts_l protoptsl nl
  | protoptsl optnl
  ;
 
-protoptsl : ssltls tlsflags
- | ssltls '{' tlsflags_l '}'
- | TCP tcpflags
- | TCP '{' tcpflags_l '}'
+protoptsl : ssltls {
+ if (!(proto->type == RELAY_PROTO_TCP ||
+    proto->type == RELAY_PROTO_HTTP)) {
+ yyerror("can set tls options only for "
+    "tcp or http protocols");
+ YYERROR;
+ }
+ } tlsflags
+ | ssltls {
+ if (!(proto->type == RELAY_PROTO_TCP ||
+    proto->type == RELAY_PROTO_HTTP)) {
+ yyerror("can set tls options only for "
+    "tcp or http protocols");
+ YYERROR;
+ }
+ } '{' tlsflags_l '}'
+ | TCP {
+ if (!(proto->type == RELAY_PROTO_TCP ||
+    proto->type == RELAY_PROTO_HTTP)) {
+ yyerror("can set tcp options only for "
+    "tcp or http protocols");
+ YYERROR;
+ }
+ } tcpflags
+ | TCP {
+ if (!(proto->type == RELAY_PROTO_TCP ||
+    proto->type == RELAY_PROTO_HTTP)) {
+ yyerror("can set tcp options only for "
+    "tcp or http protocols");
+ YYERROR;
+ }
+ } '{' tcpflags_l '}'
  | HTTP {
  if (proto->type != RELAY_PROTO_HTTP) {
  yyerror("can set http options only for "
@@ -1905,6 +1932,11 @@ relayoptsl : LISTEN ON STRING port opttls {
  | PROTO STRING {
  struct protocol *p;
 
+ if (rlay->rl_conf.proto != EMPTY_ID) {
+ yyerror("more than one protocol specified");
+ YYERROR;
+ }
+
  TAILQ_FOREACH(p, conf->sc_protos, entry)
  if (!strcmp(p->name, $2))
  break;



> >How-To-Repeat:
>
> a.
>
> This relayd speaks DNS on UDP port 80:
>
> ```
> $ cat > relayd.conf <<EOF
> table <web> { "127.0.0.1" }
> table <app> { "127.0.0.1" }
>
> http protocol A {
>        return error
>
>        match request path "/app/*" forward to <app>
> }
>
> dns protocol B {
>        return error
> }
>
> relay P {
>        listen on 0.0.0.0 port 80
>
>        protocol A
>        forward to <web> port 8080
>
>        protocol B
>        forward to <app> port 8082
> }
> EOF
> $ doas relayd -f relayd.conf
> ```
>
> ```
> $ fstat | grep relayd | grep internet
> _relayd  relayd     29047   10* internet dgram udp *:80
> _relayd  relayd     40704   10* internet dgram udp *:80
> _relayd  relayd     21221   10* internet dgram udp *:80
> _relayd  relayd     88624    4* internet raw icmp 0x0
> _relayd  relayd     88624    5* internet raw icmp 0x0
> _relayd  relayd     88624    6* internet6 raw ipv6-icmp 0x0
> _relayd  relayd     88624    7* internet6 raw ipv6-icmp 0x0
>
> ```
>
> while this one speaks http:
>
> ```
> $ cat > relayd.conf <<EOF
> table <web> { "127.0.0.1" }
> table <app> { "127.0.0.1" }
>
> dns protocol A {
> return error
>
> match request path "/app/*" forward to <app>
> }
>
> http protocol B {
> return error
> }
>
> relay P {
> listen on 0.0.0.0 port 80
>
> protocol A
> forward to <web> port 8080
>
> protocol B
> forward to <app> port 8082
> }
> EOF
> $ doas relayd -f relayd.conf
> ```
>
> ```
> $ fstat | grep relayd | grep internet
> _relayd  relayd      2564   10* internet stream tcp 0x0 *:80
> _relayd  relayd     29428   10* internet stream tcp 0x0 *:80
> _relayd  relayd     61476   10* internet stream tcp 0x0 *:80
> _relayd  relayd     90200    4* internet raw icmp 0x0
> _relayd  relayd     90200    5* internet raw icmp 0x0
> _relayd  relayd     90200    6* internet6 raw ipv6-icmp 0x0
> _relayd  relayd     90200    7* internet6 raw ipv6-icmp 0x0
> $
> $ curl http://localhost # demonstrate that relayd is speaking http
> <!DOCTYPE html>
> <html>
> <head>
> <title>500 Internal Server Error</title>
> <style type="text/css"><!--
> body { background-color: #a00000; color: white; font-family: 'Comic Sans MS', 'Chalkboard SE', 'Comic Neue', sans-serif; }
> hr { border: 0; border-bottom: 1px dashed; }
>
> --></style>
> </head>
> <body>
> <h1>Internal Server Error</h1>
> <div id='m'></div>
> <div id='l'></div>
> <hr><address>OpenBSD relayd at 0.0.0.0 port 80</address>
> </body>
> </html>
> ```
>
> b.
>
> You can see in the latter example that `match request path` is legal in
> `dns protocol` blocks which makes no sense.
>