From 6c57d010739c1b4f5ce4905ecd7aaeaf6371a48b Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 28 May 2021 23:06:50 +0200 Subject: [PATCH] ospfd: validate input SRGB and SRLB The code was not checking if in these label ranges [a,b], a is smaller than b, which is assumed in several places, including when determining the size of the block as b-a+1. As a consequence, the results of a bad configuration can be unpredictable. Some effects observed were: 1) segfault 2) de-activation of SR due to label reservation failure. The added validation function also checks if the SR blocks are larger than some minimal size. RFC 8665 mandates that the blocks be srictly larger than zero. In this patch, the minimum sized is arbitrarily defined to be 16. Checking if ranges would fall outside [16,1048575] is omitted since the vty filtering takes care of that. Signed-off-by: Fredi Raspall --- ospfd/ospf_sr.c | 26 ++++++++++++++++++++++++++ ospfd/ospf_sr.h | 3 +++ 2 files changed, 29 insertions(+) diff --git a/ospfd/ospf_sr.c b/ospfd/ospf_sr.c index 60485a770f..7d96dc1ba8 100644 --- a/ospfd/ospf_sr.c +++ b/ospfd/ospf_sr.c @@ -2151,6 +2151,13 @@ static bool ranges_overlap(uint32_t r1_lower, uint32_t r1_upper, return !((r1_upper < r2_lower) || (r1_lower > r2_upper)); } + +/* tell if a range is valid */ +static bool sr_range_is_valid(uint32_t lower, uint32_t upper, uint32_t min_size) +{ + return (upper >= lower + min_size); +} + /** * Update SRGB and/or SRLB using new CLI values. * @@ -2268,12 +2275,25 @@ DEFUN(sr_global_label_range, sr_global_label_range_cmd, /* Get lower and upper bound for mandatory global-block */ gb_lower = strtoul(argv[idx_gb_low]->arg, NULL, 10); gb_upper = strtoul(argv[idx_gb_up]->arg, NULL, 10); + /* SRLB values are taken from vtysh if there, else use the known ones */ lb_upper = argc > idx_lb_up ? strtoul(argv[idx_lb_up]->arg, NULL, 10) : OspfSR.srlb.end; lb_lower = argc > idx_lb_low ? strtoul(argv[idx_lb_low]->arg, NULL, 10) : OspfSR.srlb.start; + /* check correctness of input SRGB */ + if (!sr_range_is_valid(gb_lower, gb_upper, MIN_SRGB_SIZE)) { + vty_out(vty, "Invalid SRGB range\n"); + return CMD_WARNING_CONFIG_FAILED; + } + + /* check correctness of SRLB */ + if (!sr_range_is_valid(lb_lower, lb_upper, MIN_SRLB_SIZE)) { + vty_out(vty, "Invalid SRLB range\n"); + return CMD_WARNING_CONFIG_FAILED; + } + /* Validate SRGB against SRLB */ if (ranges_overlap(gb_lower, gb_upper, lb_lower, lb_upper)) { vty_out(vty, @@ -2328,6 +2348,12 @@ DEFUN_HIDDEN(sr_local_label_range, sr_local_label_range_cmd, lower = strtoul(argv[idx_low]->arg, NULL, 10); upper = strtoul(argv[idx_up]->arg, NULL, 10); + /* check correctness of SRLB */ + if (!sr_range_is_valid(lower, upper, MIN_SRLB_SIZE)) { + vty_out(vty, "Invalid SRLB range\n"); + return CMD_WARNING_CONFIG_FAILED; + } + /* Check if values have changed */ if ((OspfSR.srlb.start == lower) && (OspfSR.srlb.end == upper)) diff --git a/ospfd/ospf_sr.h b/ospfd/ospf_sr.h index 1669562c33..b153a220f5 100644 --- a/ospfd/ospf_sr.h +++ b/ospfd/ospf_sr.h @@ -38,6 +38,9 @@ #define SET_LABEL(label) ((label << 8) & SET_LABEL_MASK) #define GET_LABEL(label) ((label >> 8) & GET_LABEL_MASK) +/* smallest configurable SRGB / SRLB sizes */ +#define MIN_SRLB_SIZE 16 +#define MIN_SRGB_SIZE 16 /* Segment Routing TLVs as per RFC 8665 */