Add ability to return true

[pixelshot91]

Currently, the generated function returns either false, or check that net is in a list of user prefix
I would like to have a simple true/false response and handle the verification in my bird.conf

If no prefixes is given in the service in birdwatcher.conf, then no further check is required, and the function return true is the command is successful.
Can I do that ?

[pixelshot91]

I seems difficult because from the functions.tpl perspective, a empty slice or a nil slice is undistinguishable
So if PrefixSet.prefixes = [], it will be treated as if PrefixSet.prefixes = nil, and render return false

[pixelshot91]

There is a field disablePrefixCheck bool added in the initial commit, but wasn't ever use

birdwatcher/birdwatcher/servicecheck.go

Line 29 in 747ce0b

disablePrefixCheck bool

Maybe it was intended for the feature I want ?

[skoef]

@pixelshot91 interesting idea, could you share an example usecase for this?

[pixelshot91]

I want to advertise my service prefix to my peer, here a /24.

In particular cases, like a DDoS targetting one or several ips of that /24, my
peer supports the RTBH feature and I can send them a /32 advertisement with the
RTBH community (ASN:666).

Now, in other cases, managed by birdwatcher, we want to stop the advertisement
of the service prefix /24. Removing the advertisement of the /24 should also
stop the RTBH advertisement.

I didn't find a proper way to do so in bird using the config generated by birdwatcher.

Here is a configuration snippet we use:

function match_route()
{
  return net ~ [
    a.b.c.0/24
  ];
}


# Originate my service prefixes
protocol static MY_SERVICE {
  a.b.c.0/24 reject;
}

# Originate my RTBH prefix
protocol static BLACKHOLED {
  a.b.c.13/32 reject;
}

filter BGP_OUT {
  # We discard/drop the prefix if birdwatcher does not allow it.
  if ! match_route() then reject;

  if proto = "BLACKHOLED" then {
    bgp_community.add((x,y));
    accept;
  }
  if proto = "MY_SERVICE" {
    bgp_community.add((a,b))
    accept;
  }
}

With this config, we are able to advertise the /24 service prefix based on the
birdwatcher configuration.
In the case of the blackhole prefixes, it won't work because the the match_route()
will return false as the /32 is not an exact match of the /24.

What we suggest would be to make birdwatcher generates a simple True/False
configuration, acting more like a killswitch instead of a prefix filtering
function.

The prefix filtering is up to me in my filter function logic.

Assuming the config generated by birdwatcher is :

function enable_advertisement()
{
  return True; # or False depending on the birdwatcher healthcheck
}

My bird configuration would be

# Originate my service prefixes
protocol static MY_SERVICE {
  a.b.c.0/24 reject;
}

# Originate my RTBH prefix
protocol static BLACKHOLED {
  a.b.c.13/32 reject;
}

filter BGP_OUT {
  # We stop advertising if birdwatcher does not allow it.
  if ! enable_advertisement() then reject;

  # RTBH prefixes must be a more specific prefix of our service prefix
  if proto = "BLACKHOLED" and net ~ [a.b.c.0/24+] then {
    bgp_community.add((x,y));
    accept;
  }
  # Make sure the prefix is an exact match.
  if proto = "MY_SERVICE" and net ~ [a.b.c.0/24] then {
    bgp_community.add((a,b))
    accept;
  }
}

Sure thing the configuration generated could provide the + but it's quite
specific, some people might need something like a.b.c.d/19 le 24 ge 31 or
other weird stuff.

Making the generated config a simple killswitch would allow to do custom things
in the bird configuration directly.

[skoef]

I think I get what you're trying to accomplish and that is indeed currently not implemented in birdwatcher, however: wouldn't adding the /24 and all the /32's as prefixes to the service essentially do the same now? I get that that is not a super elegant way of configuring birdwatcher, but it might be a workaround.
I'll think of a clean way to implement this in birdwatcher. I did see your PR but I didn't really have time to dig into it. I will get back to you on this!

[skoef]

@pixelshot91 could you try out the branch from my pull request, to see if that has the expected results?
I based it loosely on your PR, thanks a lot, but I chose I slightly different approach and spotted some other stuff that was up for improvement while at it.

[pixelshot91]

wouldn't adding the /24 and all the /32's as prefixes to the service essentially do the same now?

Yes it would, but it is very inconvenient and not elegant.
I want to add three /24 and three /48, so it would result in 6 * 256 IPs to put in the birdwatcher.conf

could you try out the branch from my pull request, to see if that has the expected results?

Ok thanks, I will try it out

[pixelshot91]

I am sorry, but I cannot make it work
I tried with prefixes = [] or with no prefixes entry at all, but the config file stays empty

Also, sometimes the file is not fully generated before the first birdc configure leading to hard-to-read error

Apr 02 14:06:27 node birdwatcher[14576]: level=info msg="service transitioning to up" command=/usr/local/bin/check.sh service=myservice successes=9
Apr 02 14:06:27 node birdwatcher[14576]: level=info msg="prefixes updated, reloading" command="/usr/sbin/birdc configure"
Apr 02 14:06:27 node birdwatcher[14576]: level=warning msg="reloading failed" command="/usr/sbin/birdc configure" error="exit status 1" output="[66 73 82 68 32 50 46 48 46 49 50 32 114 101 97 100 121 46 10 82 101 97 100 105 110 103 32 99 111 110 102 105 103 117 114 97 116 105 111 110 32 102 114 111 109 32 47 101 116 99 47 98 105 114 100 47 98 105 114 100 46 99 111 110 102 10 47 101 116 99 47 98 105 114 100 47 98 105 114 100 46 99 111 110 102 58 52 50 58 55 32 115 121 110 116 97 120 32 101 114 114 111 114 44 32 117 110 101 120 112 101 99 116 101 100 32 67 70 95 83 89 77 95 85 78 68 69 70 73 78 69 68 10]"
Apr 02 14:06:27 node birdwatcher[14576]: level=error msg="could not apply BIRD config" error="exit status 1"

which can be converted to

BIRD 2.0.12 ready.
Reading configuration from /etc/bird/bird.conf
/etc/bird/bird.conf:42:7 syntax error, unexpected CF_SYM_UNDEFINED

Because the first function is generated but not the second one

I find the code a bit difficult to read, so I tried to re-implement it from scratch in Rust here: https://github.com/pixelshot91/birdwatcher-rs
It seems to work on a simple example, but it is not finished yet (need more tests, and more doc)

[skoef]

That output is not very readable, I'll fix this.

Could you share your entire (relevant) configuration so I can try to reproduce it?