MINOR: tcp: make set-src/set-src-port and set-dst/set-dst-port commutative

When the tcp/http actions above were introduced in 1.7-dev4, we used to
proceed like this :

  - set-src/set-dst would force the port to zero
  - set-src-port/set-dst-port would not do anything if the address family is
    neither AF_INET nor AF_INET6.

It was a stupid idea of mine to request this behaviour because it ensures
that these functions cannot be used in a wide number of situations. Because
of the first rule, it is necessary to save the source port one way or
another if only the address has to be changed (so you have to use an
variable). Due to the second rule, there's no way to set the source port
on a unix socket without first overwriting the address. And sometimes it's
really not convenient, especially when there's no way to guarantee that all
fields will properly be set.

In order to fix all this, this small change does the following :
  - set-src/set-dst always preserve the original port even if the address
    family changes. If the previous address family didn't have a port (eg:
    AF_UNIX), then the port is set to zero ;

  - set-src-port/set-dst-port always preserve the original address. If the
    address doesn't have a port, then the family is forced to IPv4 and the
    address to "0.0.0.0".

Thanks to this it now becomes possible to perform one action, the other or
both in any order.

diff --git a/doc/configuration.txt b/doc/configuration.txt
index b5e67fe3540041502c2c3cdc9979167f05d39bfd..61d4e460755d7502c8227193751362c812c87fee 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -3851,7 +3851,8 @@ http-request { allow | tarpit | auth [realm <realm>] | redirect <rule> |
          http-request set-src hdr(x-forwarded-for)
          http-request set-src src,ipmask(24)
 
-      When set-src is successful, the source port is set to 0.
+      When possible, set-src preserves the original source port as long as the
+      address family allows it, otherwise the source port is set to 0.
 
     - set-src-port <expr> :
       Is used to set the source port address to the value of specified
@@ -3865,8 +3866,9 @@ http-request { allow | tarpit | auth [realm <realm>] | redirect <rule> |
          http-request set-src-port hdr(x-port)
          http-request set-src-port int(4000)
 
-      Be careful to use "set-src-port" after "set-src", because "set-src" sets
-      the source port to 0.
+      When possible, set-src-port preserves the original source address as long
+      as the address family supports a port, otherwise it forces the source
+      address to IPv4 "0.0.0.0" before rewriting the port.
 
     - set-dst <expr> :
       Is used to set the destination IP address to the value of specified
@@ -3883,6 +3885,9 @@ http-request { allow | tarpit | auth [realm <realm>] | redirect <rule> |
          http-request set-dst hdr(x-dst)
          http-request set-dst dst,ipmask(24)
 
+      When possible, set-dst preserves the original destination port as long as
+      the address family allows it, otherwise the destination port is set to 0.
+
     - set-dst-port <expr> :
       Is used to set the destination port address to the value of specified
       expression. If you want to connect to the new address/port, use
@@ -3896,6 +3901,10 @@ http-request { allow | tarpit | auth [realm <realm>] | redirect <rule> |
          http-request set-dst-port hdr(x-port)
          http-request set-dst-port int(4000)
 
+      When possible, set-dst-port preserves the original destination address as
+      long as the address family supports a port, otherwise it forces the
+      destination address to IPv4 "0.0.0.0" before rewriting the port.
+
     - "silent-drop" : this stops the evaluation of the rules and makes the
       client-facing connection suddenly disappear using a system-dependant way
       that tries to prevent the client from being notified. The effect it then

diff --git a/src/proto_tcp.c b/src/proto_tcp.c
index 424731a6a21283cdc320aa21aa41a082c9725246..c3c998e8125e295ce09f795582300fe35a8f40c1 100644 (file)
--- a/src/proto_tcp.c
+++ b/src/proto_tcp.c
@@ -1438,7 +1438,9 @@ int tcp_exec_req_rules(struct session *sess)
 }
 
 /*
- * Execute the "set-src" action. May be called from {tcp,http}request
+ * Execute the "set-src" action. May be called from {tcp,http}request.
+ * It only changes the address and tries to preserve the original port. If the
+ * previous family was neither AF_INET nor AF_INET6, the port is set to zero.
  */
 enum act_return tcp_action_req_set_src(struct act_rule *rule, struct proxy *px,
                                               struct session *sess, struct stream *s, int flags)
@@ -1450,14 +1452,16 @@ enum act_return tcp_action_req_set_src(struct act_rule *rule, struct proxy *px,
 
                smp = sample_fetch_as_type(px, sess, s, SMP_OPT_DIR_REQ|SMP_OPT_FINAL, rule->arg.expr, SMP_T_ADDR);
                if (smp) {
+                       int port = get_net_port(&cli_conn->addr.from);
+
                        if (smp->data.type == SMP_T_IPV4) {
                                ((struct sockaddr_in *)&cli_conn->addr.from)->sin_family = AF_INET;
                                ((struct sockaddr_in *)&cli_conn->addr.from)->sin_addr.s_addr = smp->data.u.ipv4.s_addr;
-                               ((struct sockaddr_in *)&cli_conn->addr.from)->sin_port = 0;
+                               ((struct sockaddr_in *)&cli_conn->addr.from)->sin_port = port;
                        } else if (smp->data.type == SMP_T_IPV6) {
                                ((struct sockaddr_in6 *)&cli_conn->addr.from)->sin6_family = AF_INET6;
                                memcpy(&((struct sockaddr_in6 *)&cli_conn->addr.from)->sin6_addr, &smp->data.u.ipv6, sizeof(struct in6_addr));
-                               ((struct sockaddr_in6 *)&cli_conn->addr.from)->sin6_port = 0;
+                               ((struct sockaddr_in6 *)&cli_conn->addr.from)->sin6_port = port;
                        }
                }
                cli_conn->flags |= CO_FL_ADDR_FROM_SET;
@@ -1466,7 +1470,9 @@ enum act_return tcp_action_req_set_src(struct act_rule *rule, struct proxy *px,
 }
 
 /*
- * Execute the "set-dst" action. May be called from {tcp,http}request
+ * Execute the "set-dst" action. May be called from {tcp,http}request.
+ * It only changes the address and tries to preserve the original port. If the
+ * previous family was neither AF_INET nor AF_INET6, the port is set to zero.
  */
 enum act_return tcp_action_req_set_dst(struct act_rule *rule, struct proxy *px,
                                               struct session *sess, struct stream *s, int flags)
@@ -1478,13 +1484,15 @@ enum act_return tcp_action_req_set_dst(struct act_rule *rule, struct proxy *px,
 
                smp = sample_fetch_as_type(px, sess, s, SMP_OPT_DIR_REQ|SMP_OPT_FINAL, rule->arg.expr, SMP_T_ADDR);
                if (smp) {
+                       int port = get_net_port(&cli_conn->addr.to);
+
                        if (smp->data.type == SMP_T_IPV4) {
                                ((struct sockaddr_in *)&cli_conn->addr.to)->sin_family = AF_INET;
                                ((struct sockaddr_in *)&cli_conn->addr.to)->sin_addr.s_addr = smp->data.u.ipv4.s_addr;
                        } else if (smp->data.type == SMP_T_IPV6) {
                                ((struct sockaddr_in6 *)&cli_conn->addr.to)->sin6_family = AF_INET6;
                                memcpy(&((struct sockaddr_in6 *)&cli_conn->addr.to)->sin6_addr, &smp->data.u.ipv6, sizeof(struct in6_addr));
-                               ((struct sockaddr_in6 *)&cli_conn->addr.to)->sin6_port = 0;
+                               ((struct sockaddr_in6 *)&cli_conn->addr.to)->sin6_port = port;
                        }
                        cli_conn->flags |= CO_FL_ADDR_TO_SET;
                }
@@ -1493,8 +1501,10 @@ enum act_return tcp_action_req_set_dst(struct act_rule *rule, struct proxy *px,
 }
 
 /*
- * Execute the "set-src-port" action. May be called from {tcp,http}request
- * We must test the sin_family before setting the port
+ * Execute the "set-src-port" action. May be called from {tcp,http}request.
+ * We must test the sin_family before setting the port. If the address family
+ * is neither AF_INET nor AF_INET6, the address is forced to AF_INET "0.0.0.0"
+ * and the port is assigned.
  */
 enum act_return tcp_action_req_set_src_port(struct act_rule *rule, struct proxy *px,
                                               struct session *sess, struct stream *s, int flags)
@@ -1508,10 +1518,14 @@ enum act_return tcp_action_req_set_src_port(struct act_rule *rule, struct proxy
 
                smp = sample_fetch_as_type(px, sess, s, SMP_OPT_DIR_REQ|SMP_OPT_FINAL, rule->arg.expr, SMP_T_SINT);
                if (smp) {
-                       if (((struct sockaddr_storage *)&cli_conn->addr.from)->ss_family == AF_INET) {
-                               ((struct sockaddr_in *)&cli_conn->addr.from)->sin_port = htons(smp->data.u.sint);
-                       } else if (((struct sockaddr_storage *)&cli_conn->addr.from)->ss_family == AF_INET6) {
+                       if (cli_conn->addr.from.ss_family == AF_INET6) {
                                ((struct sockaddr_in6 *)&cli_conn->addr.from)->sin6_port = htons(smp->data.u.sint);
+                       } else {
+                               if (cli_conn->addr.from.ss_family != AF_INET) {
+                                       cli_conn->addr.from.ss_family = AF_INET;
+                                       ((struct sockaddr_in *)&cli_conn->addr.from)->sin_addr.s_addr = 0;
+                               }
+                               ((struct sockaddr_in *)&cli_conn->addr.from)->sin_port = htons(smp->data.u.sint);
                        }
                }
        }
@@ -1519,8 +1533,10 @@ enum act_return tcp_action_req_set_src_port(struct act_rule *rule, struct proxy
 }
 
 /*
- * Execute the "set-dst-port" action. May be called from {tcp,http}request
- * We must test the sin_family before setting the port
+ * Execute the "set-dst-port" action. May be called from {tcp,http}request.
+ * We must test the sin_family before setting the port. If the address family
+ * is neither AF_INET nor AF_INET6, the address is forced to AF_INET "0.0.0.0"
+ * and the port is assigned.
  */
 enum act_return tcp_action_req_set_dst_port(struct act_rule *rule, struct proxy *px,
                                               struct session *sess, struct stream *s, int flags)
@@ -1534,10 +1550,14 @@ enum act_return tcp_action_req_set_dst_port(struct act_rule *rule, struct proxy
 
                smp = sample_fetch_as_type(px, sess, s, SMP_OPT_DIR_REQ|SMP_OPT_FINAL, rule->arg.expr, SMP_T_SINT);
                if (smp) {
-                       if (((struct sockaddr_storage *)&cli_conn->addr.to)->ss_family == AF_INET) {
-                               ((struct sockaddr_in *)&cli_conn->addr.to)->sin_port = htons(smp->data.u.sint);
-                       } else if (((struct sockaddr_storage *)&cli_conn->addr.to)->ss_family == AF_INET6) {
-                               ((struct sockaddr_in6 *)&cli_conn->addr.to)->sin6_port = htons(smp->data.u.sint);
+                       if (cli_conn->addr.from.ss_family == AF_INET6) {
+                               ((struct sockaddr_in6 *)&cli_conn->addr.from)->sin6_port = htons(smp->data.u.sint);
+                       } else {
+                               if (cli_conn->addr.from.ss_family != AF_INET) {
+                                       cli_conn->addr.from.ss_family = AF_INET;
+                                       ((struct sockaddr_in *)&cli_conn->addr.from)->sin_addr.s_addr = 0;
+                               }
+                               ((struct sockaddr_in *)&cli_conn->addr.from)->sin_port = htons(smp->data.u.sint);
                        }
                }
        }