Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for Ethernet padding when checking receive packet size #2442

Merged
merged 2 commits into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 40 additions & 11 deletions keepalived/vrrp/vrrp.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include <netinet/ip6.h>
#include <stdint.h>
#include <net/if_arp.h>
#include <linux/if_ether.h>
#include <net/ethernet.h>
#ifdef _NETWORK_TIMESTAMP_
#include <linux/net_tstamp.h>
Expand Down Expand Up @@ -90,6 +91,11 @@
#include "ipvswrapper.h"
#endif

/* Ideally we would use a struct from a system header to determine the
* size of a vlan tag, but there doesn't seem to be one exposed to
* user space. */
#define VLAN_TAG_SIZE 4

/* If we don't have certain configuration, then we can optimise the
* resources that keepalived uses. These are cleared by start_vrrp()
* in clear_summary_flags() and set in vrrp_complete_instance()
Expand Down Expand Up @@ -841,12 +847,27 @@ vrrp_check_packet(vrrp_t *vrrp, const vrrphdr_t *hd, const char *buffer, ssize_t
* packet (including fixed fields, and IPvX address(es)).
*/
if (buflen != expected_len) {
log_message(LOG_INFO, "(%s) vrrp packet too %s, length %zu and expect %zu",
vrrp->iname,
buflen > expected_len ? "long" : "short",
buflen, expected_len);
++vrrp->stats->packet_len_err;
return VRRP_PACKET_KO;
/* Allow for Ethernet frame padding. If there is padding, the
* frame length (excluding FCS) is 60 octets (ETH_ZLEN).
* The Ethernet header (14 bytes - ETH_HLEN) and any Vlan
* headers (4 bytes each) are removed before we receive the
* packet.
* Padding added is ETH_ZLEN - ETH_HLEN - expected_len, or
* multiples of 4 (Vlan header) less than that. Checking the
* amount of padding added can therefore only be done modulo 4.
*/
if (expected_len < ETH_ZLEN - ETH_HLEN &&
expected_len < buflen &&
(buflen - expected_len) % VLAN_TAG_SIZE == (VLAN_TAG_SIZE - (ETH_ZLEN - ETH_HLEN) % VLAN_TAG_SIZE) % VLAN_TAG_SIZE) {
/* This is OK, there is some padding */
} else {
log_message(LOG_INFO, "(%s) vrrp packet too %s, length %zu and expect %zu",
vrrp->iname,
buflen > expected_len ? "long" : "short",
buflen, expected_len);
++vrrp->stats->packet_len_err;
return VRRP_PACKET_KO;
}
}

/* MUST verify that the IPv4 TTL/IPv6 HL is 255 (but not if unicast) */
Expand Down Expand Up @@ -974,11 +995,19 @@ vrrp_check_packet(vrrp_t *vrrp, const vrrphdr_t *hd, const char *buffer, ssize_t

/* Check the IP header total packet length matches what we received */
if (vrrp->family == AF_INET && ntohs(ip->tot_len) != buflen) {
log_message(LOG_INFO,
"(%s) ip_tot_len mismatch against received length. %d and received %zu",
vrrp->iname, ntohs(ip->tot_len), buflen);
++vrrp->stats->packet_len_err;
return VRRP_PACKET_KO;
/* Allow for Ethernet frame padding. See earlier comment
* for details. */
if (buflen <= ETH_ZLEN - ETH_HLEN &&
ntohs(ip->tot_len) < buflen &&
(buflen - ntohs(ip->tot_len)) % VLAN_TAG_SIZE == (VLAN_TAG_SIZE - (ETH_ZLEN - ETH_HLEN) % VLAN_TAG_SIZE) % VLAN_TAG_SIZE) {
/* This is OK, there is some padding */
} else {
log_message(LOG_INFO,
"(%s) ip_tot_len mismatch against received length. %d and received %zu",
vrrp->iname, ntohs(ip->tot_len), buflen);
++vrrp->stats->packet_len_err;
return VRRP_PACKET_KO;
}
}

/* MUST verify the VRRP checksum. Kernel takes care of checksum mismatch incase of IPv6. */
Expand Down
4 changes: 1 addition & 3 deletions keepalived/vrrp/vrrp_data.c
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ dump_vrrp(FILE *fp, const vrrp_t *vrrp)
conf_write(fp, " Master router = %s", inet_sockaddrtos(&vrrp->master_saddr));
conf_write(fp, " Master priority = %d", vrrp->master_priority);
if (vrrp->version == VRRP_VERSION_3)
conf_write(fp, " Master advert int = %.2f sec", vrrp->master_adver_int / TIMER_HZ_DOUBLE);
conf_write(fp, " Master advert interval = %u milli-sec", vrrp->master_adver_int / (TIMER_HZ / 1000));
}
}
if (vrrp->flags) {
Expand Down Expand Up @@ -747,8 +747,6 @@ dump_vrrp(FILE *fp, const vrrp_t *vrrp)
(vrrp->adver_int / (TIMER_HZ / 1000)),
(vrrp->version == VRRP_VERSION_2) ? "sec" : "milli-sec");
conf_write(fp, " Last advert sent = %ld.%6.6ld", vrrp->last_advert_sent.tv_sec, vrrp->last_advert_sent.tv_usec);
if (vrrp->state == VRRP_STATE_BACK && vrrp->version == VRRP_VERSION_3)
conf_write(fp, " Master advert interval = %u milli-sec", vrrp->master_adver_int / (TIMER_HZ / 1000));
#ifdef _WITH_FIREWALL_
conf_write(fp, " Accept = %s", vrrp->accept ? "enabled" : "disabled");
#endif
Expand Down
Loading