On 1/21/25 11:42 PM, Dumitru Ceara wrote:
> On 1/21/25 6:32 PM, Mark Michelson wrote:
>> From: Numan Siddique <[email protected]>
>>
>> When OVN's DNS caching feature is enabled, due to the OpenFlow rules
>> that OVN installs in Open vSwitch, it is possible for an attacker to
>> craft a UDP packet that can bypass egress ACL rules configured on the
>> same switch that has DNS caching configured.
>>
>> This patch fixes the issue by setting a register bit when OVN's DNS
>> responder replies to an incoming request. Then the flow that allows
>> egress ACL bypass only applies to packets that have this register bit
>> set. This gives the intended effect of allowing internally-generated DNS
>> responses to not be blocked by user-defined ACLs without potentially
>> compromising the security of the switch.
>>
>> Signed-off-by: Numan Siddique <[email protected]>
>> Signed-off-by: Mark Michelson <[email protected]>
>> ---
>>  controller/pinctrl.c         | 27 ++++++++++
>>  include/ovn/logical-fields.h |  1 +
>>  lib/logical-fields.c         |  3 ++
>>  northd/northd.c              |  3 +-
>>  tests/ovn.at                 | 97 ++++++++++++++++++++++++++++++++++++
>>  5 files changed, 130 insertions(+), 1 deletion(-)
>>
>> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
>> index 5046d637b..39444424e 100644
>> --- a/controller/pinctrl.c
>> +++ b/controller/pinctrl.c
>> @@ -389,6 +389,8 @@ static void pinctrl_handle_put_fdb(const struct flow *md,
>>                                     const struct flow *headers)
>>                                     OVS_REQUIRES(pinctrl_mutex);
>>  
>> +static void set_from_ctrl_flag_in_pkt_metadata(struct ofputil_packet_in *);
>> +
>>  COVERAGE_DEFINE(pinctrl_drop_put_mac_binding);
>>  COVERAGE_DEFINE(pinctrl_drop_buffered_packets_map);
>>  COVERAGE_DEFINE(pinctrl_drop_controller_event);
>> @@ -3604,6 +3606,10 @@ exit:
>>          union mf_subvalue sv;
>>          sv.u8_val = success;
>>          mf_write_subfield(&dst, &sv, &pin->flow_metadata);
>> +
>> +        /* Indicate that this packet is from ovn-controller. */
>> +        set_from_ctrl_flag_in_pkt_metadata(pin);
>> +
> 
> We could leverage this new flag for all ovn-controller generated
> packets.  E.g., for the functionality implemented by:
> 
> 744340f701b0 ("Allow LR to send RAs through localnet port.")
> https://github.com/ovn-org/ovn/commit/744340f701
> 
> But we can do that as a follow up.
> 
> The patch itself looks correct to me:
> 
> Acked-by: Dumitru Ceara <[email protected]>
> 

My ack applies to the following backports of this patch:
24.09:
https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/
23.09:
https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/
23.06:
https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/
23.03:
https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/
22.12:
https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/
22.09:
https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/
22.06:
https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/
22.03:
https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/

For branch 24.03 I have a small comment, I'll reply on that specific patch.

Regards,
Dumitru

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to