@jmlccdmd Ok, I'll re-test with in/out then.

My fault for not having the time to test this as one of the users who has a need for RFC compliant VRRP. The use of + for interface matching is less than ideal but if we do so we should take care to recommend that use of 802.1Q VLAN sub-interfaces not make use of the parent (untagged) interface else traffic matching will not be obvious.

Other than potential collision between eth0+ and eth0.100+ I don't think there would be anything else to watch out for.

@dmbaturin did you find time to check the pull requests? Checked with latest crux build and the issue still exists.
I can test this if needed.

This still needs some review and testing. @Merijn, thank you for the offer; testing will be much appreciated, once PRs are merged.

@jestabro i have encountered the first situation in my networks where i really need RFC-Complaint VRRP
(Some devices do not learn the MAC-address on the VRRP gateway, it works for some time and then stops).

Can we merge the PRs and test? I can try to find time to manually apply them to crux and test but i am not sure i can build the packages on my debian setup.

@jestabro i am not able to build the vyos-1x package because of dependencies on other packages.
Can we include them in rolling so i can test tomorrow?

@Merijn I will build and run a sanity check, and then we can merge into rolling for testing; I'll confirm when done. Thanks again for the offer to test.

@Merijn I am consulting with the author of the pull request; I still need to confirm behavior before we can consider merging.

The fix suggested in the pull request did not resolve the issue as a consequence of T1847. With that issue resolved, we can consider this merge.

@Merijn if you are still willing to test, I can provide build instructions, if you need. I noticed that you had been working with @hagbard recently on your build environment; if you are willing to use docker, I can give you step-by-step instructions to build with these patches.

This fix remains an open discussion with @zsdc and @Dmitry

This remains open; need to rejoin discussion.

RFC-compliant VRRP has been broken for all releases of 1.2 so not likely that it will be fixed in 1.2.7 but I think we should make an effort to fix it in 1.3 (?)

Side Note: Looking at this in a VM VRRP initially didn't work at all until I enabled promiscuous on the parent interface. I think this is a VM issue rather than a VyOS issue but will need to confirm when I get back to the office. Is this a known problem with VirtualBox (I don't run VirtualBox for VyOS in production so I wouldn't know).

As a proof of concept I made a transition script to handle dynamic add-remove of firewall rules for VRRP on transition which seems to work.

There are some things that need review though:

• This covers ethernet interface types only, would need to also look at supporting bond, etc (should be simple but need to validate input)
• Right now it only covers interface-based firewall rules, I haven't tested it with zone-policy
• Source and Destination NAT rules are covered, I think
• Any use of the "raw" or "mangle" table is not covered. I think there are some cases where we would need this (policy routing comes to mind).
• Right now this is implemented as a transition script, but ideally it would be rolled into vrrp-script-wrapper.py and keepalived.conf would be generated to always call the wrapper even if no user-defined script is in use.
• We would also want to call the script on firewall changes otherwise the VRRP interfaces will become out of sync
• The script has a band-aid for 1.2 using Python 3.4 that can be exclude on 1.3 as subprocess.run becomes available with newer Python.
• I didn't implement IPv6 in the script but it would be identical to IPv4 with the exception of not needing to consider NAT

Here is /config/scripts/vrrp-iptables.py :

#!/usr/bin/env python3

import sys
import os
import re
import subprocess

from datetime import datetime




log_file = "/tmp/vrrp-iptables.log"
tmp_file = "/tmp/vrrp-iptables." + str(os.getpid()) + ".iptables-save"




# fix for subprocess.run not supported in python < 3.5
try:
    from subprocess import CompletedProcess
except ImportError:
    class CompletedProcess:

        def __init__(self, args, returncode, stdout=None, stderr=None):
            self.args = args
            self.returncode = returncode
            self.stdout = stdout
            self.stderr = stderr

        def check_returncode(self):
            if self.returncode != 0:
                err = subprocess.CalledProcessError(self.returncode, self.args, output=self.stdout)
                raise err
            return self.returncode

    def sp_run(*popenargs, **kwargs):
        input = kwargs.pop("input", None)
        check = kwargs.pop("handle", False)
        if input is not None:
            if 'stdin' in kwargs:
                raise ValueError('stdin and input arguments may not both be used.')
            kwargs['stdin'] = subprocess.PIPE
        process = subprocess.Popen(*popenargs, **kwargs)
        try:
            outs, errs = process.communicate(input)
        except:
            process.kill()
            process.wait()
            raise
        returncode = process.poll()
        if check and returncode:
            raise subprocess.CalledProcessError(returncode, popenargs, output=outs)
        return CompletedProcess(popenargs, returncode, stdout=outs, stderr=errs)

    subprocess.run = sp_run

# end fix




def writelog(msg):
    timestamp = datetime.now().strftime("%Y-%m-%d %H:%M:%S")
    line = timestamp + " " + msg + "\n"
    # print(line)
    file = open(log_file, "a+") 
    file.write(line)
    file.close()
    




def get_iface_vmac(parent_iface):
    result = []
    cmd = "/sbin/ip -br link show | grep " + parent_iface + "v"
    output = subprocess.run(cmd, shell=True, stdout=subprocess.PIPE)
    output = output.stdout.decode("utf-8").splitlines()
    for line in output:
        iface = line.split()[0]
        if iface.index("@") != -1:
            iface = iface[:iface.index("@")]
            result.append(iface)
    return result




def get_iptables(parent_iface, mode="add"):
    result = []

    # interface firewall in rules
    cmd = "/sbin/iptables -S VYATTA_FW_IN_HOOK | grep ' \\-i " + parent_iface
    if mode == "delete" : cmd += "v'"
    else : cmd += " '"
    output = subprocess.run(cmd, shell=True, stdout=subprocess.PIPE)
    output = output.stdout.decode("utf-8").splitlines()
    for line in output:
        if mode == "delete" : line = "-D" + line[2:]
        result.append(line)

    # interface firewall local rules
    cmd = "/sbin/iptables -S VYATTA_FW_LOCAL_HOOK | grep ' \\-i " + parent_iface
    if mode == "delete" : cmd += "v'"
    else : cmd += " '"
    output = subprocess.run(cmd, shell=True, stdout=subprocess.PIPE)
    output = output.stdout.decode("utf-8").splitlines()
    for line in output:
        if mode == "delete" : line = "-D" + line[2:]
        result.append(line)

    # interface firewall out rules
    cmd = "/sbin/iptables -S VYATTA_FW_OUT_HOOK | grep ' \\-o " + parent_iface
    if mode == "delete" : cmd += "v'"
    else : cmd += " '"
    output = subprocess.run(cmd, shell=True, stdout=subprocess.PIPE)
    output = output.stdout.decode("utf-8").splitlines()
    for line in output:
        if mode == "delete" : line = "-D" + line[2:]
        result.append(line)

    # source nat rules
    cmd = "/sbin/iptables -t nat -S POSTROUTING | grep ' \\-o " + parent_iface
    if mode == "delete" : cmd += "v'"
    else : cmd += " '"
    output = subprocess.run(cmd, shell=True, stdout=subprocess.PIPE)
    output = output.stdout.decode("utf-8").splitlines()
    for line in output:
        if mode == "delete" : line = "-D" + line[2:]
        result.append("-t nat " + line)

    # destination nat rules
    cmd = "/sbin/iptables -t nat -S PREROUTING | grep ' \\-i " + parent_iface
    if mode == "delete" : cmd += "v'"
    else : cmd += " '"
    output = subprocess.run(cmd, shell=True, stdout=subprocess.PIPE)
    output = output.stdout.decode("utf-8").splitlines()
    for line in output:
        if mode == "delete" : line = "-D" + line[2:]
        result.append("-t nat " + line)

    if mode == "add":
        new_result = []
        iface_vmac = get_iface_vmac(parent_iface)
        for iface in iface_vmac:
            for line in result:
                line = line.replace(parent_iface, iface)
                new_result.append(line)
        result = new_result

    return result




def build(parent_iface, mode="add"):
    batch = []
    rules = get_iptables(parent_iface, "delete")
    if mode == "add":
        rules.extend(get_iptables(parent_iface, "add"))

    nat_rules = []
    filter_rules = []

    for line in rules:
        if line.find("-t nat") != -1 : nat_rules.append(line[7:])
        else : filter_rules.append(line)

    batch.append("# Generated by vrrp-iptable.py")
    batch.append("# Use iptables-restore -n <file> on this file to perform a non-destructive atomic update.")

    if len(nat_rules) > 0:
        batch.append("*nat")
        batch.extend(nat_rules)
        batch.append("COMMIT")

    if len(filter_rules) > 0:
        batch.append("*filter")
        batch.extend(filter_rules)
        batch.append("COMMIT")


    # write our batch file for iptables-restore -n
    if len(batch) > 2:
        file = open(tmp_file, "w+")
        file.write("\n".join(batch) + "\n")
        file.close()

        cmd = "/sbin/iptables-restore -n " + tmp_file
        output = subprocess.run(cmd, shell=True, stdout=subprocess.PIPE)
        output = output.stdout.decode("utf-8")
        if len(output) > 0 : writelog("The following errors were encountered:\n" + output)

        # clean up
        os.remove(tmp_file)




if len(sys.argv) == 4:
    action = sys.argv[1]
    iface  = sys.argv[2]
    group  = sys.argv[3]
    writelog("DEBUG action=" + action + ",iface=" + iface + ",group=" + group)
    if re.match(r"^eth[0-9.]*$", iface):
        if action == "stop":
            writelog("INFO removing iptables rules or virtual interfaces")
            build(iface, "delete")
        if action == "master":
            writelog("INFO adding iptables rules for virtual interfaces")
            build(iface, "add")
    else:
        writelog("ERROR invalid interface identifier")
else:
    writelog("ERROR invalid argument list, existing")

Here is the high-availability config that it was tested with:

set high-availability vrrp group LAN interface 'eth1'
set high-availability vrrp group LAN preempt-delay '300'
set high-availability vrrp group LAN priority '200'
set high-availability vrrp group LAN rfc3768-compatibility
set high-availability vrrp group LAN transition-script master '/config/scripts/vrrp-iptables.py'
set high-availability vrrp group LAN transition-script mode-force
set high-availability vrrp group LAN transition-script stop '/config/scripts/vrrp-iptables.py'
set high-availability vrrp group LAN virtual-address '10.99.0.1/32'
set high-availability vrrp group LAN vrid '100'
set high-availability vrrp sync-group CLUSTER member 'LAN'

Additional Notes:

• The script ignores the configuration system and instead pulls data from:
  • Active network interfaces ip -br link show actively looking for the format of <parent_iface>v<vrid>@<parent_interface>
  • Active iptables rules for assigning chains to interfaces for in , local, and out (though out doesn't seem to be needed in my test case there is probably a corner case where it gets used, such as the virtual IP being a different network from the parent interface IP)
• The script generates a temporary file in iptables-save syntax (named with its PID to avoid conflicts) and uses iptables-restore -n <tmp_file> to apply the changes as an atomic commit. Note the -n for non-destructive is critical and if omitted will wipe out current policy in favor of whatever is in the file. This has two advantages: The first is that any syntax error will prevent any changes from being made, avoiding a half-applied state; the second is that atomic commits using this method are very fast. It's unlikely that the speed is needed here, but I use this same method to apply 65000 rules on a production system and the execution time difference between a straight shell script loading iptables commands and using iptables-restore for is 15 minutes vs. less than 30 seconds. As an aside this is really how we should be handling firewall rule changes in VyOS to eliminate long commit times and half-broken configuration states that are hard to recover from.

Actually there is nothing that stops us from adding rules to netfilter referencing an interface that doesn't exist yet so this could be done at the time of interface and VRRP configuration rather than dynamically.

I will push to have a solution included in 1.3, reviewing both zsdc's and rps's suggested patches; we will not hold up epa1 for this, but rather include as a bug fix following.

Hello,

Same issue for me, i use the last RC 1.3 release (manualy builded yesterday).
Will you correct this bug until the LTS release ?

Best Regards,

Do we know if this made it into the 1.3.0 release or is this now a 1.4 issue?

I would say it's pretty critical as baseline functionality to have a functional RFC-compliant VRRP implementation that doesn't bypass firewall policy.

The FRRouting project now has a VRRP implementation, perhaps it would be worth investigating.

We've gone though all of 1.2 with this broken at this point. The use of the non-RFC-compliant implementation which relies on GARP instead of a virtual MAC has created operational incidents where the failover didn't get learned by some hosts several times in production over the past few years (some devices ignore GARP, or a spanning-tree topology change lines up with the failover and drops the burst of GARPs and systems hold on to stale ARP etc).

@rps this did not make it into 1.3.0, but was discussed recently and will be addressed; it is, as you point out, a regrettable omission in functionality

Has there been any movement on this? I've been following for a while since I've noticed the behavior myself and am hoping it can be resolved at some point.

@pyaskowski try zone-policy firewall

Just a suggestion, would it be a weird idea to move the firewall config from the interface section to the firewall section? A bit like the zone config. So something like:

set firewall local interface eth0 name <firewall-filter>
set firewall in interface eth0 name <firewall-filter>
set firewall out interface eth0 name <firewall-filter>
set firewall local interface bond0.10v22v6 ipv6-name <firewall-filter>

The problem is that using zone-policy firewall is a bit overkill for a pure router or even a router with async routing. In which scenario I guess only the local variant would be useful.

Or, come to think, some free from of set interfaces unknown <typeyourownname> firewall local name <ruleset> where you can only config stuff that doesn't really depend on an interface.

Or
` set interfaces vrrp [.....]

In T1185#133941, @roedie wrote:

Just a suggestion, would it be a weird idea to move the firewall config from the interface section to the firewall section? A bit like the zone config. So something like:

set firewall local interface eth0 name <firewall-filter>
set firewall in interface eth0 name <firewall-filter>
set firewall out interface eth0 name <firewall-filter>
set firewall local interface bond0.10v22v6 ipv6-name <firewall-filter>

The problem is that using zone-policy firewall is a bit overkill for a pure router or even a router with async routing. In which scenario I guess only the local variant would be useful.

A similar syntax change is in progress as part of a larger firewall refactor. It should reach the 1.4 branch in a week or so. It should allow for any valid existing interface name.

In T1185#133944, @sdev wrote:

A similar syntax change is in progress as part of a larger firewall refactor. It should reach the 1.4 branch in a week or so. It should allow for any valid existing interface name.

Nice, very interested in testing it.

It should be possible in https://github.com/vyos/vyos-1x/pull/1534 T2199

set firewall interface ethXvX

as stated before, in 1.4 now it's possible to specify corresponding interface.

Hi all, sorry for joining an old conversation, but I think it is a bit confused here and looks like you are trying to work-around the VRRP protocol specification with the argument rfc3768_compatibility - but I'm note sure to understand such behavior and if it is the best way.

I think you need to implement VRRP protocol specification in VyOS based on what RFCs say, not on what keepalived does or user/admin wants to do.

To complain with VRRP RFC, v2 and v3, the VRRP adv MUST be sent from the VMAC interface, this allow switches to correctly learn the MAC address of VIP:

7.2.  Transmitting VRRP Packets

   The following operations MUST be performed when transmitting a VRRP
   packet:

      - Fill in the VRRP packet fields with the appropriate virtual
      router configuration state

      - Compute the VRRP checksum

      - If the protected address is an IPv4 address, then:

         + Set the source MAC address to virtual router MAC Address

         + Set the source IPv4 address to interface primary IPv4 address

      - else // ipv6

         + Set the source MAC address to virtual router MAC Address

         + Set the source IPv6 address to interface link-local IPv6
         address

         -endif

         - Set the IPvX protocol to VRRP

         - Send the VRRP packet to the VRRP IPvX multicast group

   Note: VRRP packets are transmitted with the virtual router MAC
   address as the source MAC address to ensure that learning bridges
   correctly determine the LAN segment the virtual router is
   attached to.

Your keepalived configuration template is the following:

{%         if group_config.rfc3768_compatibility is vyos_defined and group_config.peer_address is vyos_defined %}
    use_vmac {{ group_config.interface }}v{{ group_config.vrid }}v{{ '4' if group_config['address'] | first | is_ipv4 else '6' }}
    vmac_xmit_base
{%         elif group_config.rfc3768_compatibility is vyos_defined %}
    use_vmac {{ group_config.interface }}v{{ group_config.vrid }}v{{ '4' if group_config['address'] | first | is_ipv4 else '6' }}
{%         endif %}

The argument rfc3768_compatibility and all related logic are really misleading, I'm not sure how to use it and it does not certainly complies to RFC 3768.

I think that what you need to do is simply block the possibility to the admin to bind VRRP in a different interface where VIP are configured with VMAC, which would bring some security concerns, but this is how VRRP works today; something like that:

vrrp_instance VI_1 {
    v3_checksum_as_v2    // if using VRRPv3, you still need to set this to override the default behavior and complies with  RFC 5798bis.
    state {{ state }}
    version {{ version }}
    {{ accept }}
    {{ preempt }}
    interface {{ interface}}    // same as use_vmac
    virtual_router_id {{ id }}
    priority {{ priority }}
    advert_int {{ int }}
    use_vmac {{ interface }}    // same as interface

    virtual_ipaddress {
        192.168.56.16/32 dev {{ interface }}    // same as interface and use_vmac
    }
}

This would complies with both, VRRPv2 and VRRPv3 standard specifications.

To avoid those security concerns, I think you should allow the administrator to accept VRRP (112) input traffic only from those routers in the cluster, by creating the correct netfliter rules in the base interface; and I think this is already possible with VyOS.

Finally, note that VRRP protocol has the following RFCs:

RFC 5798bis is in WG Consensus: Waiting for Write-Up and it introduces some important interop clarifications regarding RFC 5798.

VyOS is really interesting, thanks for your work and good job.

@n.fort what is the status of this?