batfish / pybatfish

Python client for Batfish: https://github.com/batfish/batfish
Apache License 2.0
211 stars 52 forks source link

Header Contraints with Discontiguous Source/Destination IPs #271

Closed supertylerc closed 5 years ago

supertylerc commented 5 years ago

With pybatfish.datamodel.flow.HeaderConstraint, you can specify ranges and discontiguous ranges of ports. However, I haven't been able to find a way to specify a discontiguous range of srcIps or dstIps. I've tried passing a list and a comma-separated string of IP addresses with /32 prefix lengths. Neither works.

Is this expected behavior? Is there a workaround, or do I need to iterate through them, create multiple HeaderConstraints, and ask multiple questions?

Thanks!

progwriter commented 5 years ago

@supertylerc The format for the specifying IPs is documented here https://github.com/batfish/batfish/blob/master/questions/Parameters.md#ip-specifier

You have a few of options at this time:

  1. Use wildcards, if that's appropriate
  2. Use exclusions. if the set of exclusions is small, that might be helpful
  3. Define an address book and refer to that in a query (you will need to create and address group, then a reference book, then upload the reference book to batfish)

https://pybatfish.readthedocs.io/en/latest/datamodel.html#pybatfish.datamodel.referencelibrary.AddressGroup https://pybatfish.readthedocs.io/en/latest/datamodel.html#pybatfish.datamodel.referencelibrary.ReferenceBook https://pybatfish.readthedocs.io/en/latest/api.html#pybatfish.client.commands.bf_put_reference_book

@ratulm might have insights on a simpler workflow

supertylerc commented 5 years ago

Ah, thanks! Looks like the address book solution might work. I'll give that a try in the next few days and report back. Would love to see a more abstract way to do it, but this definitely sounds like a better way than iterating through and trying to track multiple questions. :D Thanks again!

ratulm commented 5 years ago

hey, @supertylerc - thanks for the feedback. we are in the process of revising that grammar and will put this on the table as well.

supertylerc commented 5 years ago

So I tried using an address book, but I didn't have much success. :/ I'm probably doing something silly, but it's not immediately obvious to me. Here's what I've tried:

    try:
        src_group = AddressGroup('src_ips', ['10.0.0.0'])
        src_ref = ReferenceBook('src_ip_book', [src_group])
        bf_put_reference_book(src_ref)

        headers = HeaderConstraints(srcIps=src_ref,
                                    dstIps='1.0.0.1/32',
                                    ipProtocols=module.params['ip_protocols'],
                                    srcPorts=module.params['source_ports'],
                                    dstPorts=module.params['destination_ports'])

This is the error I'm getting:

    Failed to answer question: Coordinator returned failure: Invalid question mynetwork/__searchFilters_27c852d1-f2ac-408b-9d31-882318d5ec63: Could not parse JSON question: Cannot deserialize instance of `java.lang.String` out of START_OBJECT token
     at [Source: (String)"{"action":"permit","class":"org.batfish.question.searchfilters.SearchFiltersQuestion","differential":false,"filters":"my-filter","headers":{"applications":null,"dscps":null,"dstIps":"1.0.0.1/32","dstPorts":null,"ecns":null,"flowStates":null,"fragmentOffsets":null,"icmpCodes":null,"icmpTypes":null,"ipProtocols":null,"packetLengths":null,"srcIps":{"addressGroups":[{"addresses":["10.10.10.0"],"name":"src_ips"}],"interfaceGroups":[],"name":"src_ip_book"},"srcPorts":null,"tcpFlags":[null]},"ins"[truncated 2657 chars]; line: 1, column: 354] (through reference chain: org.batfish.question.searchfilters.SearchFiltersQuestion["headers"]->org.batfish.datamodel.PacketHeaderConstraints["srcIps"])

It seems I can successfully create the AddressGroup and the ReferenceBook. Also looks like I can successfully upload the ReferenceBook. On second look HeaderConstraints seems to accept it without issue, but for some reason the question doesn't like it. Not sure what piece I'm missing there. The question is bfq.searchFilters.

I've also tried specifying srcIps as srcIps='ref.addressbook("src_ips", "src_ip_book")',, but that doesn't work either. I've also tried srcIps='ref.addressbook(group="src_ips", book="src_ip_book")', which also doesn't work. In those cases, I get this as an error:

  File "/tmp/bf_filter_check.py", line 268, in run
    reference_snapshot=reference_snapshot)
  File "/home/travis/virtualenv/python2.7.14/lib/python2.7/site-packages/pybatfish/question/question.py", line 182, in answer
    extra_args=extra_args)
  File "/home/travis/virtualenv/python2.7.14/lib/python2.7/site-packages/pybatfish/client/internal.py", line 50, in _bf_answer_obj
    workhelper.execute(work_item, bf_session, background, extra_args)
  File "/home/travis/virtualenv/python2.7.14/lib/python2.7/site-packages/pybatfish/client/workhelper.py", line 138, in execute
    log = restv2helper.get_work_log(session, snapshot, work_item.id)
  File "/home/travis/virtualenv/python2.7.14/lib/python2.7/site-packages/pybatfish/client/restv2helper.py", line 321, in get_work_log
    return six.u(_get(session, url_tail, dict()).text)
  File "/home/travis/virtualenv/python2.7.14/lib/python2.7/site-packages/six.py", line 653, in u
    return unicode(s.replace(r'\\', r'\\\\'), "unicode_escape")
ratulm commented 5 years ago

hey, @supertylerc - it should be srcIps = 'ref.addressgroup(src_ips, src_ip_book)'

two differences from what you tried:

ratulm commented 5 years ago

PS: The cryptic error message you got in the second case has been fixed in https://github.com/batfish/pybatfish/pull/272

dhalperi commented 5 years ago

Hey @ratulm - is there a status update on this issue now that you've completed the work on Batfish specifiers? I think this is easily expressible now?

ratulm commented 5 years ago

The grammar changes, in which lists of ips/prefixes are easily expressible, are master-merged. But we need a bit more work to expose all that to questions, mainly because other specifiers had to be also ported to the new grammar and there are some compatibility issues with new and old grammars. Hopefully, we can put a bow on it next week.

ratulm commented 5 years ago

@supertylerc - your requested change has been merged into master batfish (and will make it to the docker image soon if you use that).

the grammar documentation is also up-to-date. please report back if you run into any surprises.

supertylerc commented 5 years ago

Thank you! Looking forward to it!

ratulm commented 5 years ago

@supertylerc - Docker images have been updated.