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

accept source::as-set format as argument, use source as first for search #5

Closed
job opened this issue Dec 23, 2019 · 15 comments · Fixed by #70
Closed

accept source::as-set format as argument, use source as first for search #5

job opened this issue Dec 23, 2019 · 15 comments · Fixed by #70

Comments

@job
Copy link
Member

job commented Dec 23, 2019

17:00:01        @al-6453 | job-2914: https://www.peeringdb.com/net/1045 make bgpq4 maaningfully interpret string NTTCOM::AS2914:AS-GLOBAL
17:00:08        @al-6453 | whatever it means
@job job transferred this issue from another repository Jan 18, 2020
@dwcarder
Copy link

we got bit by this also, as we somewhat blindly shovel the irr object defined in peeringdb into bgpq.

Note that peval also can not parse this format.

@job
Copy link
Member Author

job commented Feb 16, 2022

@dwcarder do you feel comfortable making a pull request? I’m happy to review!

@lukastribus
Copy link

This also came up recently on ITNOG, and I will add some more information here:

Peering DB is validating the IRR Object as per peeringdb/peeringdb#151 :

Release Notes

To make the IRR as-set/route-set field of more operational value, strict rules apply

* the as-set/rs-set name has to conform to RFC 2622 (5.1 and 5.2)

* the source may be specified by `AS-SET@SOURCE` or `SOURCE::AS-SET` (preferred)

* valid sources are taken from the [list](http://www.irr.net/docs/list.html) of known IRRs

* multiple values must be separated by either `,`, ` `, or `, ` (i.e. comma, space or comma followed by space)

Applications currently have to go through a lot of trouble:

https://github.com/pierky/arouteserver/blob/eb6adaa8be512e987482b5fb8f663986c6fb9331/pierky/arouteserver/peering_db.py#L109-L176

@dwcarder
Copy link

I am increasingly of the mindset that the approach that @lukastribus points out is the only one workable in practice (strip off any known IRR prefix). In looking at the IRRd documentation https://irrd.readthedocs.io/en/stable/users/queries/whois/, there is no way to start a recursive query w/ !a or !i specifying a source for only the initial object.

@jwbensley
Copy link
Contributor

I think this needs more traction again; a fake AS-GOOGLE AS-SET has recently been added to RIPE which is empty and breaking automated prefix automation. I think this could be broken down into two subtly different feature requests;

  1. Add support to read the IRR source from peeringDB like "RADB::AS-GOOGLE"
  2. Add an option to specify a default RIR DB when on isn't specified e.g. the IRR NTT mirrors because they have sane merging of the different DBs an whois -h irr1-ber1-de -s NTTCOM AS-GOOGLE returns the correct AS-SET (but I'm not saying we should hard code a defualt, just add an option for a user specified default)

Thoughts?

@spolack
Copy link

spolack commented Oct 12, 2022

I think to properly solve that issue we also need to make sure that a AS-SET from e.g. RIPE can also reference a ARIN AS-SET.

@jwbensley
Copy link
Contributor

@spolack so we need to perform this action recursively on every AS-SET within an AS-SET, right?

@dwcarder
Copy link

It is possibly an expensive amount of round trips to implement a recursive query in that manner with a client such as bgpq4. It may be a better approach to consider adding such recursive query support in IRRd 4 where you have the potential efficiency of a proper backend database, then exposing this for clients like bgpq4 to use.

@jwbensley
Copy link
Contributor

jwbensley commented Oct 14, 2022

@dwcarder I agree that this should probably be added to IRRd, but I don't think it should only be added to IRRd, I think it should be added to bgpq4 too. Adding this to IRRd only helps people with access to an IRRd server with that capability. If we add it to a client to bgpq4 people can benefit from this option immediately, rather than relying on server upgrades (which may never happen).

I have a long journey coming up in the next few days, I'm happy to use that travel time to read through the bgpq4 source and try to familiarise my self with it, to see what it would take to write a PR for this feature - if such a PR isn't just going to be rejected?

@job
Copy link
Member Author

job commented Oct 14, 2022

I will carefully review each and every PR with the intent to merge them. I welcome contributions from the community.

@MattKobayashi
Copy link

I think this needs more traction again; a fake AS-GOOGLE AS-SET has recently been added to RIPE which is empty and breaking automated prefix automation.

This has also happened to the APNIC IRR database, and it appears to have been added by the same organisation. To what end, I'm not sure, but it seems to me that it's a coordinated effort.

@spolack
Copy link

spolack commented Oct 20, 2022

Today i observed someone created a Empty AS-FACEBOOK on the ripe database. Would be great if that would be implemented - I'm happy to assist with testing!

@jwbensley
Copy link
Contributor

jwbensley commented Oct 20, 2022

Update: I edited this to split the original idea into two parts...

Part 1

Let's discuss how we could implement a fix;

Having had a read through the code, it looks to me like this is the main loop where we are recursing and expanding the AS-SET tree: https://github.com/bgp/bgpq4/blob/main/expander.c#L1020

Here bgpq4 does one of two things

  1. bgpq4 performs an “!a4” or “!a6” query against the IRR server if supported, which returns all the prefixes in the AS-SET tree
  2. else it performs an “!i,1” query to get all the member ASNs/AS-SETs and then it uses “!gas” or “!6as” to get all the prefixes for those members

If we want to try to pull each AS-SET (and each member AS-SET) from it’s preferred authoritative source, over other potential sources, then I think we could have a CLI option which sets a flag, and if the flag is true, we have a third option which is does the following:

  • Store the current list of sources being used (i.e. any specified on the CLI, otherwise store a default of all “*”)
  • Start of loop:
    • Check if AS/RS-SET has already been queried, if true, skip
    • If the AS/RS-SET uses the "::" format to specify a source, set the IRRd source with an “!s” query
    • If the AS/RS-SET doesn't contain a source, use the CLI specified sources (if any), otherwise fall back to "*"
    • Perform a non-recursive “!i” query to get the members of this AS/RS-SET, and add them to the AS tree
    • for each member go to start of loop and recurse down the tree
  • Restore the list of sources from before the loop started
  • Perform a “!gas”/“!6as” for each ASN in the AS tree to get the prefix list

This allows us to get an AS-SET from it's source if one is specified.

Separately we could implement a 2nd CLI option for when no source is specified. That would be a separate feature I have put below as part 2, and for now just want to focus on part 1.

Does this sounds reasonable?

Have I missed something about the way bgpq4 works?

Part 2

We could have a CLI option which sets a flag, when true, if the AS-SET has no source specified we could, modify the loop above as follows:

  • Check if AS/RS-SET has already been queried, if true, skip
  • If the AS/RS-SET uses the "::" format to specify a source, set the IRRd source with an “!s” query
  • If the AS/RS-SET doesn't specify a source, query PeeringDB to get the preferred source for the AS/RS-SET being queried
  • Set the IRRd source with an “!s” query, based on PeeringDB response
  • If there is no PeeringDB specified source, use the CLI specified sources or fall back to "*"
  • Perform a non-recursive “!i” query to get the members of this AS/RS-SET, and add them to the AS tree
  • for each member go to start of loop and recurse down the tree

@jwbensley
Copy link
Contributor

@job / @dwcarder PR raised: #70

It's marked as a draft whilst @spolack and I perform some production testing, but lab testing seems to be all fine so I think you can start the review and if we find any issues I'll push fixes to the PR.

@spolack
Copy link

spolack commented Oct 26, 2022

PR #70 as of now works as designed. I did a few tests compiling lists of child-ASs and also prefix lists. No abnormalities i've seen so far.

I tried whether it works when recurring, but the RIPE database does not tolerate :: in a AS-SET member attribute. Afaik it should be conform to RPSL, so we might have to file a feature request. Dont have access to other IRR, nor a local irrd running, thats why didnt test further.

@job job closed this as completed in #70 Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants