kytos-ng / of_core

Kytos Main OpenFlow Network Application (NApp)
MIT License
0 stars 5 forks source link

Action Set Field class should also consider the OXM_FIELD to decide the subclass #139

Open italovalcy opened 1 month ago

italovalcy commented 1 month ago

Hi,

Currently, the ActionSetField class is only considering the action type on the deserialization process, as defined here:

https://github.com/kytos-ng/of_core/blob/3545b1c23fa8f39937b686d3fb0d056a8fa996e5/v0x04/flow.py#L189

If one were to implement a new ActionSetField (for instance, ActionSetFieldIPv4Dst), we would have to also consider the OXM_FIELD to take the decision of the proper subclass to deserialize the OF Action data.

Use case: in the HackInSDN project we are defining new Action Set Fields to implement Custom Containment actions to mitigate cyberattacks. Some actions we are adding include: ActionSetFieldIPv4Dst, ActionSetFieldIPv6Dst, ActionSetFieldTCPDst, etc.

On our implementation we ended up overriding the OFActionSetField as showed below:

        self.new_actions = {
            "set_ipv4_dst": ActionSetIPv4Dst,
            OFActionSetField: ActionSetIPv4Dst,
        }

        for name, action in self.new_actions.items():
            Action.add_action_class(name, action)

Then after adding a flow with set_ipv4_dst and also set_vlan, we got the following error:

Jul 30 13:08:04 bcd69a5b0fea uvicorn.error:ERROR httptools_impl:441:  Exception in ASGI application#012Traceback (most recent call last):#012  File "/usr/local/lib/python3.9/dist-packages/uvicorn/protocols/http/httptools_impl.py", line 436, in run_asgi#012    result = await app(  # type: ignore[func-returns-value]#012  File "/usr/local/lib/python3.9/dist-packages/uvicorn/middleware/proxy_headers.py", line 78, in __call__#012    return await self.app(scope, receive, send)#012  File "/usr/local/lib/python3.9/dist-packages/starlette/applications.py", line 120, in __call__#012    await self.middleware_stack(scope, receive, send)#012  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/errors.py", line 184, in __call__#012    raise exc#012  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/errors.py", line 162, in __call__#012    await self.app(scope, receive, _send)#012  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/cors.py", line 84, in __call__#012    await self.app(scope, receive, send)#012  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/exceptions.py", line 79, in __call__#012    raise exc#012  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/exceptions.py", line 68, in __call__#012    await self.app(scope, receive, sender)#012  File "/usr/local/lib/python3.9/dist-packages/starlette/routing.py", line 716, in __call__#012    await route.handle(scope, receive, send)#012  File "/usr/local/lib/python3.9/dist-packages/starlette/routing.py", line 276, in handle#012    await self.app(scope, receive, send)#012  File "/usr/local/lib/python3.9/dist-packages/starlette/routing.py", line 68, in app#012    response = await run_in_threadpool(func, request)#012  File "/usr/local/lib/python3.9/dist-packages/starlette/concurrency.py", line 41, in run_in_threadpool#012    return await anyio.to_thread.run_sync(func, *args)#012  File "/usr/local/lib/python3.9/dist-packages/anyio/to_thread.py", line 31, in run_sync#012    return await get_asynclib().run_sync_in_worker_thread(#012  File "/usr/local/lib/python3.9/dist-packages/anyio/_backends/_asyncio.py", line 937, in run_sync_in_worker_thread#012    return await future#012  File "/usr/local/lib/python3.9/dist-packages/anyio/_backends/_asyncio.py", line 867, in run#012    result = context.run(func, *args)#012  File "//var/lib/kytos/napps/kytos/flow_manager/main.py", line 612, in add#012    return self._send_flow_mods_from_request(request, dpid, "add")#012  File "//var/lib/kytos/napps/kytos/flow_manager/main.py", line 677, in _send_flow_mods_from_request#012    self._install_flows(command, flows_dict, [switch], reraise_conn=not force)#012  File "//var/lib/kytos/napps/kytos/flow_manager/main.py", line 722, in _install_flows#012    flow_mod = build_flow_mod_from_command(flow, command)#012  File "//var/lib/kytos/napps/../napps/kytos/flow_manager/utils.py", line 56, in build_flow_mod_from_command#012    flow_mod = flow.as_of_add_flow_mod()#012  File "//var/lib/kytos/napps/../napps/kytos/of_core/flow.py", line 191, in as_of_add_flow_mod#012    return self._as_of_flow_mod(FlowModCommand.OFPFC_ADD)#012  File "//var/lib/kytos/napps/../napps/kytos/of_core/v0x04/flow.py", line 403, in _as_of_flow_mod#012    of_flow_mod.instructions = [instruction.as_of_instruction() for#012  File "//var/lib/kytos/napps/../napps/kytos/of_core/v0x04/flow.py", line 403, in <listcomp>#012    of_flow_mod.instructions = [instruction.as_of_instruction() for#012  File "//var/lib/kytos/napps/../napps/kytos/of_core/v0x04/flow.py", line 241, in as_of_instruction#012    of_actions = [action.as_of_action() for action in self.actions]#012  File "//var/lib/kytos/napps/../napps/kytos/of_core/v0x04/flow.py", line 241, in <listcomp>#012    of_actions = [action.as_of_action() for action in self.actions]#012  File "//var/lib/kytos/napps/../napps/talitarp/contention/of_core/v0x04/action.py", line 33, in as_of_action#012    return OFActionSetField(field=tlv)#012NameError: name 'OFActionSetField' is not defined

This error happened because our class ended up being responsible for deserializing all OFActionSetField packets.

It seems that using a ActionSetField Factory to proper identify the subclass responsible for each OXM fields would be proper solution.

cc @talitarp since she is also working on this

italovalcy commented 1 month ago

Workaround we created:

        self.new_actions = {
            "set_ipv4_dst": ActionSetIPv4Dst,
            OFActionSetField: ActionSetFieldFactory,
        }

        for name, action in self.new_actions.items():
            Action.add_action_class(name, action)

class ActionSetFieldFactory(OFActionSetField):
    _subclass_set_field = {
        OxmOfbMatchField.OFPXMT_OFB_VLAN_VID: ActionSetVlan,
        OxmOfbMatchField.OFPXMT_OFB_IPV4_DST: ActionSetIPv4Dst,
        OxmOfbMatchField.OFPXMT_OFB_IPV6_DST: ActionSetIPv6Dst,
        OxmOfbMatchField.OFPXMT_OFB_TCP_DST: ActionSetTCPDst,
        OxmOfbMatchField.OFPXMT_OFB_UDP_DST: ActionSetUDPDst,
        OxmOfbMatchField.OFPXMT_OFB_ETH_DST: ActionSetETHDst,
    }

    @classmethod
    def from_of_action(cls, of_action):
        """Return a high-level ActionSetField to call specific subclass."""
        subclass = cls._subclass_set_field.get(of_action.field.oxm_field)
        return subclass.from_of_action(of_action) if subclass else None

    @classmethod
    def add_set_field_subclass(cls, oxm_field, subclass):
        """Add a subclass for ActionSetField based on OXM Field."""
        cls._subclass_set_field[oxm_field] = subclass
italovalcy commented 1 month ago

I believe that we should include this solution into of_core directly (to avoid someone else having to depending on our Napp or ended up overriding our subclasses when using together)

italovalcy commented 1 month ago

Usage examples after deploying the solution above (https://github.com/hackinsdn/containment):