Attention is currently required from: flichtenheld, plaisthos. Hello plaisthos, flichtenheld,
I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/439?usp=email to review the following change. Change subject: mroute: properly print protocol at the end of the string ...................................................................... mroute: properly print protocol at the end of the string mroute: substitute unused field with proto Rather than adding a new field 'proto', take advantage of the 'unused' field and rename it. Hashing will now start at the 'proto' field rather than 'type'. MULTI: ensure we've got the correct protocol with virtual addresses MULTI: ensure we've got the same value as protocol for vaddressed Change-Id: I6688362d8461c112bf425ddfe488d511a64cc37e Signed-off-by: Gianmarco De Gregori <gianma...@mandelbit.com> --- M src/openvpn/forward.c M src/openvpn/mroute.c M src/openvpn/mroute.h M src/openvpn/mtcp.c M src/openvpn/mudp.c M src/openvpn/multi.c M src/openvpn/ssl.c 7 files changed, 45 insertions(+), 29 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/39/439/1 diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 27415ee..63a684b 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -1114,13 +1114,16 @@ decrypt_status = openvpn_decrypt(&c->c2.buf, c->c2.buffers->decrypt_buf, co, &c->c2.frame, ad_start); - if (!decrypt_status - /* all sockets are of the same type, so just check the first one */ - && link_socket_connection_oriented(c->c2.link_sockets[0])) + for (int i = 0; i < c->c1.link_sockets_num; i++) { - /* decryption errors are fatal in TCP mode */ - register_signal(c->sig, SIGUSR1, "decryption-error"); /* SOFT-SIGUSR1 -- decryption error in TCP mode */ - msg(D_STREAM_ERRORS, "Fatal decryption error (process_incoming_link), restarting"); + if (!decrypt_status + /* all sockets are of the same type, so just check the first one (not anymore!) */ + && link_socket_connection_oriented(c->c2.link_sockets[i])) + { + /* decryption errors are fatal in TCP mode */ + register_signal(c->sig, SIGUSR1, "decryption-error"); /* SOFT-SIGUSR1 -- decryption error in TCP mode */ + msg(D_STREAM_ERRORS, "Fatal decryption error (process_incoming_link), restarting"); + } } } else @@ -2239,6 +2242,7 @@ if (status > 0) { + /*printf("\nstatus: %d\n", status); */ int i; mtcp->event_set_status = 0; for (i = 0; i < status; ++i) @@ -2275,10 +2279,6 @@ mtcp->event_set_status = ES_TIMEOUT; } } - else - { - mtcp->event_set_status = SOCKET_READ; - } } /* 'now' should always be a reasonably up-to-date timestamp */ diff --git a/src/openvpn/mroute.c b/src/openvpn/mroute.c index 0017a48..c72fe10 100644 --- a/src/openvpn/mroute.c +++ b/src/openvpn/mroute.c @@ -421,7 +421,6 @@ { buf_printf(&out, ":%d", ntohs(maddr.v4.port)); } - buf_printf(&out, ":%d", maddr.proto); } break; @@ -454,6 +453,7 @@ buf_printf(&out, "UNKNOWN"); break; } + buf_printf(&out, "|%d", maddr.proto); return BSTR(&out); } else diff --git a/src/openvpn/mroute.h b/src/openvpn/mroute.h index 7c8972f..4e6d32c 100644 --- a/src/openvpn/mroute.h +++ b/src/openvpn/mroute.h @@ -74,9 +74,8 @@ struct mroute_addr { uint8_t len; /* length of address */ - uint8_t unused; - uint8_t type; /* MR_ADDR/MR_WITH flags */ uint8_t proto; + uint8_t type; /* MR_ADDR/MR_WITH flags */ uint8_t netbits; /* number of bits in network part of address, * valid if MR_WITH_NETBITS is set */ union { @@ -231,7 +230,7 @@ mroute_addr_hash_ptr(const struct mroute_addr *a) { /* NOTE: depends on ordering of struct mroute_addr */ - return (uint8_t *) &a->type; + return (uint8_t *) &a->proto; } static inline uint32_t diff --git a/src/openvpn/mtcp.c b/src/openvpn/mtcp.c index d4ce642..ba0905e 100644 --- a/src/openvpn/mtcp.c +++ b/src/openvpn/mtcp.c @@ -109,7 +109,7 @@ mi = multi_create_instance(m, NULL, ls); if (mi && !proto_is_dgram(ls->info.proto)) { - printf("\nTCP add\n"); + mi->real.proto = ls->info.proto; struct hash_element *he; const uint32_t hv = hash_value(hash, &mi->real); struct hash_bucket *bucket = hash_bucket(hash, hv); @@ -746,22 +746,26 @@ ev_arg->u.ls); } } - multi_get_timeout(m, &m->top.c2.timeval); - io_wait_udp(&m->top, m->mtcp, p2mp_iow_flags(m)); - MULTI_CHECK_SIG(m); - multi_process_per_second_timers(m); - - if (m->mtcp->event_set_status == ES_TIMEOUT) + while (true) { - multi_process_timeout(m, MPP_PRE_SELECT | MPP_CLOSE_ON_SIGNAL); - } - else - { - multi_process_io_udp(m); + multi_get_timeout(m, &m->top.c2.timeval); + io_wait_udp(&m->top, m->mtcp, p2mp_iow_flags(m)); MULTI_CHECK_SIG(m); - } + multi_process_per_second_timers(m); + + if (m->mtcp->event_set_status == ES_TIMEOUT) + { + multi_process_timeout(m, MPP_PRE_SELECT | MPP_CLOSE_ON_SIGNAL); + } + else + { + multi_process_io_udp(m); + MULTI_CHECK_SIG(m); + break; + } + } break; } } diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c index e9182c8..4979751 100644 --- a/src/openvpn/mudp.c +++ b/src/openvpn/mudp.c @@ -193,6 +193,7 @@ struct multi_instance *mi = NULL; struct hash *hash = m->hash; real.proto = ls->info.proto; + m->local.proto = real.proto; if (mroute_extract_openvpn_sockaddr(&real, &m->top.c2.from.dest, true) && m->top.c2.buf.len > 0) diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 3522206..5098581 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -1157,11 +1157,12 @@ */ static struct multi_instance * multi_get_instance_by_virtual_addr(struct multi_context *m, - const struct mroute_addr *addr, + struct mroute_addr *addr, bool cidr_routing) { struct multi_route *route; struct multi_instance *ret = NULL; + addr->proto = 0; /* check for local address */ if (mroute_addr_equal(addr, &m->local)) @@ -1247,6 +1248,7 @@ CLEAR(remote_si); remote_si.addr.in4.sin_family = AF_INET; remote_si.addr.in4.sin_addr.s_addr = htonl(a); + addr.proto = 0; ASSERT(mroute_extract_openvpn_sockaddr(&addr, &remote_si, false)); if (netbits >= 0) @@ -3351,6 +3353,14 @@ bool ret = true; bool floated = false; + /* + * Since we don't really need the protocol on vaddresses for internal VPN + * payload packets, make sure we have the same value to void hashing insert + * and search issues. + */ + src.proto = 0; + dest.proto = src.proto; + if (m->pending) { return true; @@ -3417,7 +3427,6 @@ 0, &c->c2.to_tun, DEV_TYPE_TUN); - /* drop packet if extract failed */ if (!(mroute_flags & MROUTE_EXTRACT_SUCCEEDED)) { @@ -3555,6 +3564,8 @@ const int dev_type = TUNNEL_TYPE(m->top.c1.tuntap); int16_t vid = 0; + src.proto = 0; + dest.proto = src.proto; #ifdef MULTI_DEBUG_EVENT_LOOP printf("TUN -> TCP/UDP [%d]\n", BLEN(&m->top.c2.buf)); diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index cee4afe..73d6db0 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -3926,6 +3926,7 @@ { msg(D_MULTI_DROPPED, "Incoming control channel packet too big, dropping."); + printf("\nif (!buf_copy(in, buf))\n"); goto error; } reliable_mark_active_incoming(ks->rec_reliable, in, id, op); -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/439?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I6688362d8461c112bf425ddfe488d511a64cc37e Gerrit-Change-Number: 439 Gerrit-PatchSet: 1 Gerrit-Owner: ordex <a...@unstable.cc> Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com> Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org> Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net> Gerrit-Attention: plaisthos <arne-open...@rfc2549.org> Gerrit-Attention: flichtenheld <fr...@lichtenheld.com> Gerrit-MessageType: newchange
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel