laforge has submitted this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/21534 )

Change subject: Introduce 'osmo_tlv_prot' abstraction for validation of TLV 
protocols
......................................................................

Introduce 'osmo_tlv_prot' abstraction for validation of TLV protocols

This extends our existing TLV parser with the ability to
* validate that mandatory IEs of a given message are present
* validate that all present IEs are of required minimum length

Introducing this generic layer will help us to reduce open-coded
imperative verification across virtually all the protocols we
implement, as well as add validation to those protocols where we
don't properly perform related input validation yet.

Change-Id: If1e1d9adfa141ca86001dbd62a6a339f9bf9a912
---
M include/osmocom/gsm/tlv.h
M src/gsm/libosmogsm.map
M src/gsm/tlv_parser.c
3 files changed, 159 insertions(+), 1 deletion(-)

Approvals:
  fixeria: Looks good to me, approved
  pespin: Looks good to me, but someone else must approve
  Jenkins Builder: Verified



diff --git a/include/osmocom/gsm/tlv.h b/include/osmocom/gsm/tlv.h
index 254c21b..a307b3d 100644
--- a/include/osmocom/gsm/tlv.h
+++ b/include/osmocom/gsm/tlv.h
@@ -620,4 +620,54 @@
 int osmo_shift_lv(uint8_t **data, size_t *data_len,
                  uint8_t **value, size_t *value_len);

+#define MSG_DEF(name, mand_ies, flags) { name, mand_ies, ARRAY_SIZE(mand_ies), 
flags }
+
+struct osmo_tlv_prot_msg_def {
+       /*! human-readable name of message type (optional) */
+       const char *name;
+       /*! array of mandatory IEs */
+       const uint8_t *mand_ies;
+       /*! number of entries in 'mand_ies' above */
+       uint8_t mand_count;
+       /*! user-defined flags (like uplink/downlink/...) */
+       uint32_t flags;
+};
+struct osmo_tlv_prot_ie_def {
+       /*! minimum length of IE value part, in octets */
+       uint16_t min_len;
+       /*! huamn-readable name (optional) */
+       const char *name;
+};
+
+/*! Osmocom TLV protocol definition */
+struct osmo_tlv_prot_def {
+       /*! human-readable name of protocol */
+       const char *name;
+       /*! TLV parser definition (optional) */
+       const struct tlv_definition *tlv_def;
+       /*! definition of each message (8-bit message type) */
+       struct osmo_tlv_prot_msg_def msg_def[256];
+       /*! definition of IE for each 8-bit tag */
+       struct osmo_tlv_prot_ie_def ie_def[256];
+       /*! value_string array of message type names (legacy, if not populated 
in msg_def) */
+       const struct value_string *msgt_names;
+};
+
+const char *osmo_tlv_prot_msg_name(const struct osmo_tlv_prot_def *pdef, 
uint8_t msg_type);
+const char *osmo_tlv_prot_ie_name(const struct osmo_tlv_prot_def *pdef, 
uint8_t iei);
+
+int osmo_tlv_prot_validate_tp(const struct osmo_tlv_prot_def *pdef, uint8_t 
msg_type,
+                             const struct tlv_parsed *tp, int log_subsys, 
const char *log_pfx);
+
+int osmo_tlv_prot_parse(const struct osmo_tlv_prot_def *pdef,
+                       struct tlv_parsed *dec, unsigned int dec_multiples, 
uint8_t msg_type,
+                       const uint8_t *buf, unsigned int buf_len, uint8_t 
lv_tag, uint8_t lv_tag2,
+                       int log_subsys, const char *log_pfx);
+
+static inline uint32_t osmo_tlv_prot_msgt_flags(const struct osmo_tlv_prot_def 
*pdef, uint8_t msg_type)
+{
+       return pdef->msg_def[msg_type].flags;
+}
+
+
 /*! @} */
diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map
index f339120..bcded1c 100644
--- a/src/gsm/libosmogsm.map
+++ b/src/gsm/libosmogsm.map
@@ -580,6 +580,11 @@
 osmo_match_shift_tlv;
 osmo_shift_lv;

+osmo_tlv_prot_msg_name;
+osmo_tlv_prot_ie_name;
+osmo_tlv_prot_validate_tp;
+osmo_tlv_prot_parse;
+
 gan_msgt_vals;
 gan_pdisc_vals;

diff --git a/src/gsm/tlv_parser.c b/src/gsm/tlv_parser.c
index 159b42b..24edd0c 100644
--- a/src/gsm/tlv_parser.c
+++ b/src/gsm/tlv_parser.c
@@ -1,4 +1,4 @@
-/* (C) 2008-2017 by Harald Welte <[email protected]>
+/* (C) 2008-2020 by Harald Welte <[email protected]>
  * (C) 2016-2017 by sysmocom - s.f.m.c. GmbH
  *
  * All Rights Reserved
@@ -24,6 +24,7 @@
 #include <stdint.h>
 #include <errno.h>
 #include <osmocom/core/utils.h>
+#include <osmocom/core/logging.h>
 #include <osmocom/gsm/tlv.h>

 /*! \addtogroup tlv
@@ -627,4 +628,106 @@
        return -1;
 }

+static __thread char ienamebuf[32];
+static __thread char msgnamebuf[32];
+
+/*! get the message name for given msg_type in protocol pdef */
+const char *osmo_tlv_prot_msg_name(const struct osmo_tlv_prot_def *pdef, 
uint8_t msg_type)
+{
+       if (pdef->msg_def[msg_type].name) {
+               return pdef->msg_def[msg_type].name;
+       } else if (pdef->msgt_names) {
+               return get_value_string(pdef->msgt_names, msg_type);
+       } else {
+               snprintf(msgnamebuf, sizeof(msgnamebuf), "Unknown msg_type 
0x%02x", msg_type);
+               return msgnamebuf;
+       }
+}
+
+/*! get the IE name for given IEI in protocol pdef */
+const char *osmo_tlv_prot_ie_name(const struct osmo_tlv_prot_def *pdef, 
uint8_t iei)
+{
+       if (pdef->ie_def[iei].name) {
+               return pdef->ie_def[iei].name;
+       } else {
+               snprintf(ienamebuf, sizeof(ienamebuf), "Unknown IEI 0x%02x", 
iei);
+               return ienamebuf;
+       }
+}
+
+/*! Validate an already TLV-decoded message against the protocol definition.
+ *  \param[in] pdef protocol definition of given protocol
+ *  \param[in] msg_type message type of the parsed message
+ *  \param[in] tp TLV parser result
+ *  \param[in] log_subsys logging sub-system for log messages
+ *  \param[in] log_pfx prefix for log messages
+ *  \returns 0 in case of success; negative osmo_tlv_parser_error in case of 
error
+ */
+int osmo_tlv_prot_validate_tp(const struct osmo_tlv_prot_def *pdef, uint8_t 
msg_type,
+                             const struct tlv_parsed *tp, int log_subsys, 
const char *log_pfx)
+{
+       const struct osmo_tlv_prot_msg_def *msg_def= &pdef->msg_def[msg_type];
+       unsigned int num_err = 0;
+       unsigned int i;
+
+       if (msg_def->mand_ies) {
+               for (i = 0; i < msg_def->mand_count; i++) {
+                       uint8_t iei = msg_def->mand_ies[i];
+                       if (!TLVP_PRESENT(tp, iei)) {
+                               LOGP(log_subsys, LOGL_ERROR, "%s %s %s: Missing 
Mandatory IE: %s\n",
+                                    log_pfx, pdef->name, 
osmo_tlv_prot_msg_name(pdef, msg_type),
+                                    osmo_tlv_prot_ie_name(pdef, iei));
+                               num_err++;
+                       }
+               }
+       }
+
+       for (i = 0; i < ARRAY_SIZE(tp->lv); i++) {
+               uint16_t min_len;
+
+               if (!TLVP_PRESENT(tp, i))
+                       continue;
+
+               min_len = pdef->ie_def[i].min_len;
+               if (TLVP_LEN(tp, i) < min_len) {
+                       LOGP(log_subsys, LOGL_ERROR, "%s %s %s: Short IE %s: %u 
< %u\n", log_pfx,
+                            pdef->name, osmo_tlv_prot_msg_name(pdef, msg_type),
+                            osmo_tlv_prot_ie_name(pdef, i), TLVP_LEN(tp, i), 
min_len);
+                       num_err++;
+               }
+       }
+
+       return -num_err;
+}
+
+/*! Parse + Validate a TLV-encoded message against the protocol definition.
+ *  \param[in] pdef protocol definition of given protocol
+ *  \param[out] dec caller-allocated pointer to \ref tlv_parsed
+ *  \param[in] dec_multiples length of the tlv_parsed[] in \a dec.
+ *  \param[in] msg_type message type of the parsed message
+ *  \param[in] buf the input data buffer to be parsed
+ *  \param[in] buf_len length of the input data buffer
+ *  \param[in] lv_tag an initial LV tag at the start of the buffer
+ *  \param[in] lv_tag2 a second initial LV tag following the \a lv_tag
+ *  \param[in] log_subsys logging sub-system for log messages
+ *  \param[in] log_pfx prefix for log messages
+ *  \returns 0 in case of success; negative osmo_tlv_parser_error in case of 
error
+ */
+int osmo_tlv_prot_parse(const struct osmo_tlv_prot_def *pdef,
+                       struct tlv_parsed *dec, unsigned int dec_multiples, 
uint8_t msg_type,
+                       const uint8_t *buf, unsigned int buf_len, uint8_t 
lv_tag, uint8_t lv_tag2,
+                       int log_subsys, const char *log_pfx)
+{
+       int rc;
+
+       rc = tlv_parse2(dec, dec_multiples, pdef->tlv_def, buf, buf_len, 
lv_tag, lv_tag2);
+       if (rc < 0) {
+               LOGP(log_subsys, LOGL_ERROR, "%s %s %s: TLV parser error %d\n", 
log_pfx,
+                    pdef->name, osmo_tlv_prot_msg_name(pdef, msg_type), rc);
+               return rc;
+       }
+
+       return osmo_tlv_prot_validate_tp(pdef, msg_type, dec, log_subsys, 
log_pfx);
+}
+
 /*! @} */

--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/21534
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: If1e1d9adfa141ca86001dbd62a6a339f9bf9a912
Gerrit-Change-Number: 21534
Gerrit-PatchSet: 10
Gerrit-Owner: laforge <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-MessageType: merged

Reply via email to