Issue with encoding multiple flow spec fragmentation flags

[pavel-odintsov]

Hello!

I've found another bug.

I've following announce:

 match fragment first-fragment last-fragment then discard

On opposite side with ExaBGP I'm getting:

update json { "exabgp": "3.5.0", "host" : "Ubuntu-1404-trusty-64-minimal", "counter": 1, "type": "update", "neighbor": { "address": { "local": "127.0.0.1", "peer": "127.0.0.1" }, "asn": { "local": "1", "peer": "1" }, "direction": "in", "message": { "update": { "attribute": { "origin": "incomplete", "extended-community": [ 9225060886715039744 ] }, "announce": { "ipv4 flow": { "no-nexthop": { "flow-11": { "fragment": [ "first-fragment" ], "string": "flow fragment first-fragment" } } } } } } } }

As you can see we saw only one flag here. But last-fragment have lost.

Same packet from GoBGP in Wireshark dissector looks like:

     Path Attribute - MP_REACH_NLRI
            Flags: 0x80, Optional: Optional, Non-transitive, Complete
                1... .... = Optional: Optional
                .0.. .... = Transitive: Non-transitive
                ..0. .... = Partial: Complete
                ...0 .... = Length: Regular length
            Type Code: MP_REACH_NLRI (14)
            Length: 9
            Address family identifier (AFI): IPv4 (1)
            Subsequent address family identifier (SAFI): Flow Spec Filter (133)
            Next hop network address (0 bytes)
            Number of Subnetwork points of attachment (SNPA): 0
            Network layer reachability information (4 bytes)
                FLOW_SPEC_NLRI (4 bytes)
                    NRLI length: 3
                    Filter: IP fragment filter ( FF)
                        Filter type: IP fragment filter (12)
                        Operator flags: 0x80, end-of-list, Value length: 1 byte: 1 <<
                            1... .... = end-of-list: Set
                            .0.. .... = and: Not set
                            ..00 .... = Value length: 1 byte: 1 << (0)
                            .... 0... = Reserved: Not set
                            .... .0.. = Reserved: Not set
                            .... ..0. = logical negation: Not set
                            .... ...0 = Match bit: Not set
                        Fragment Flag: 0x04, First fragment
                            .... 0... = Last fragment: Not set
                            .... .1.. = First fragment: Set
                            .... ..0. = Is a fragment: Not set
                            .... ...0 = Don't fragment: Not set

Actually we need some way to encode this cases. Because case "last-fragment" + "first-fragment" is actually useful for DDoS mitigation (for dropping whole fragmented traffic).

[fujita]

Thanks for the report! Fixed by https://github.com/osrg/gobgp/commit/633e14d4d768766b6f74236415661f364778daa1

[pavel-odintsov]

Perfect! From my point of view it's working correctly. But I have some issues with decoding this flags with ExaBGP. And I suppose bug on ExaBGP side now. Will share more details soon.

[fujita]

Can we close this? Or need more work?

[pavel-odintsov]

Hello!

I deeply thought about this case. End even discussed it with ExaBGP's author: https://github.com/Exa-Networks/exabgp/issues/381

Finally we have we have cases here: 1) Multiple fragmentation types which should be ored for match (as in your case: fragment first-fragment last-fragment). So our packet should match if packets is a last fragment or is a first fragment. 2) Multiple fragmentation flags which should be anded (each packet should be first-fragment and last-fragment in same time). You are encoding they into single element.

Case 2 is not very obviously and rarely used. Definitely, it very implementation specific and we need to check Juniper and Cisco implementations. But case 1 should be work on each flow spec implementation. I'm pretty sure we should encode this case as separate elements (one with last fragment and one with first-fragment). But you are doing it another way.

For fragmentation case 1 is not pretty interesting, really. Packets which are last-fragment and first-fragment really impossible.

So I suppose current implementation is incorrect and should be changed to behavior described above.

[fujita]

Is this the behavior that you expect?

[pavel-odintsov]

Yes, it is! ;)

On Saturday, 5 March 2016, FUJITA Tomonori notifications@github.com wrote:

Is this the behavior that you expect?

[image: 2016-03-05 13 27 12] https://cloud.githubusercontent.com/assets/726426/13545667/b01492a8-e2d6-11e5-941f-01c77bd1e526.png

— Reply to this email directly or view it on GitHub https://github.com/osrg/gobgp/issues/716#issuecomment-192567276.

Sincerely yours, Pavel Odintsov

[fujita]

Wish was granted, https://github.com/osrg/gobgp/commit/5f317e778f548fa696bfd57e314306d87a593fdb

[pavel-odintsov]

Great job!

Right now announce:

match fragment first-fragment last-fragment then discard

Encoded to this with ExaBGP:

update json { "exabgp": "3.5.0", "host" : "Ubuntu-1404-trusty-64-minimal", "counter": 1, "type": "update", "neighbor": { "address": { "local": "127.0.0.1", "peer": "127.0.0.1" }, "asn": { "local": "1", "peer": "1" }, "direction": "in", "message": { "update": { "attribute": { "origin": "incomplete", "extended-community": [ 9225060886715039744 ] }, "announce": { "ipv4 flow": { "no-nexthop": { "flow-11": { "fragment": [ "first-fragment", "last-fragment" ], "string": "flow fragment [ first-fragment last-fragment ]" } } } } } } } }

And multiple fragmentation type elements handled properly. Thanks!