From 461f1fe1a972d4604de58318a82a0b5b21ddbf68 Mon Sep 17 00:00:00 2001 From: Dirk Ziegelmeier Date: Fri, 3 Mar 2017 13:01:04 +0100 Subject: [PATCH] More SNMPv3 cleanups: Create own header snmpv3_dummy.h and move functions in there Decouple snmp_netconn.c from snmpv3_dummy.c (snmpv3_enginetime_timer) Make SNMP API more type-safe using enums --- src/apps/snmp/snmp_msg.c | 17 +++++------ src/apps/snmp/snmp_netconn.c | 5 ---- src/apps/snmp/snmp_snmpv2_usm.c | 20 +++++++------ src/apps/snmp/snmpv3_dummy.c | 19 +++++++++--- src/apps/snmp/snmpv3_dummy.h | 52 +++++++++++++++++++++++++++++++++ src/apps/snmp/snmpv3_mbedtls.c | 2 +- src/apps/snmp/snmpv3_priv.h | 8 ++++- src/include/lwip/apps/snmpv3.h | 43 ++++++++++++++------------- 8 files changed, 116 insertions(+), 50 deletions(-) create mode 100644 src/apps/snmp/snmpv3_dummy.h diff --git a/src/apps/snmp/snmp_msg.c b/src/apps/snmp/snmp_msg.c index db0d3e13..a7964fbe 100644 --- a/src/apps/snmp/snmp_msg.c +++ b/src/apps/snmp/snmp_msg.c @@ -770,7 +770,8 @@ snmp_parse_inbound_frame(struct snmp_request *request) s32_t s32_value; err_t err; #if LWIP_SNMP_V3 - u8_t auth, priv; + snmpv3_auth_algo_t auth; + snmpv3_priv_algo_t priv; #endif IF_PARSE_EXEC(snmp_pbuf_stream_init(&pbuf_stream, request->inbound_pbuf, 0, request->inbound_pbuf->tot_len)); @@ -1008,7 +1009,6 @@ snmp_parse_inbound_frame(struct snmp_request *request) if (request->msg_flags & SNMP_V3_AUTH_FLAG) { const u8_t zero_arr[SNMP_V3_MAX_AUTH_PARAM_LENGTH] = { 0 }; u8_t key[20]; - u8_t algo; u8_t hmac[LWIP_MAX(SNMP_V3_SHA_LEN, SNMP_V3_MD5_LEN)]; struct snmp_pbuf_stream auth_stream; @@ -1028,8 +1028,8 @@ snmp_parse_inbound_frame(struct snmp_request *request) /* Verify authentication */ IF_PARSE_EXEC(snmp_pbuf_stream_init(&auth_stream, request->inbound_pbuf, 0, request->inbound_pbuf->tot_len)); - IF_PARSE_EXEC(snmpv3_get_user((char*)request->msg_user_name, &algo, key, NULL, NULL)); - IF_PARSE_EXEC(snmpv3_auth(&auth_stream, request->inbound_pbuf->tot_len, key, algo, hmac)); + IF_PARSE_EXEC(snmpv3_get_user((char*)request->msg_user_name, &auth, key, NULL, NULL)); + IF_PARSE_EXEC(snmpv3_auth(&auth_stream, request->inbound_pbuf->tot_len, key, auth, hmac)); if(memcmp(request->msg_authentication_parameters, hmac, SNMP_V3_MAX_AUTH_PARAM_LENGTH)) { snmp_stats.wrongdigests++; @@ -1074,17 +1074,16 @@ snmp_parse_inbound_frame(struct snmp_request *request) /* Decrypt message */ u8_t key[20]; - u8_t algo; IF_PARSE_EXEC(snmp_asn1_dec_tlv(&pbuf_stream, &tlv)); IF_PARSE_ASSERT(tlv.type == SNMP_ASN1_TYPE_OCTET_STRING); parent_tlv_value_len -= SNMP_ASN1_TLV_HDR_LENGTH(tlv); IF_PARSE_ASSERT(parent_tlv_value_len > 0); - IF_PARSE_EXEC(snmpv3_get_user((char*)request->msg_user_name, NULL, NULL, &algo, key)); + IF_PARSE_EXEC(snmpv3_get_user((char*)request->msg_user_name, NULL, NULL, &priv, key)); if(snmpv3_crypt(&pbuf_stream, tlv.value_len, key, request->msg_privacy_parameters, request->msg_authoritative_engine_boots, - request->msg_authoritative_engine_time, algo, SNMP_V3_PRIV_MODE_DECRYPT) != ERR_OK) { + request->msg_authoritative_engine_time, priv, SNMP_V3_PRIV_MODE_DECRYPT) != ERR_OK) { snmp_stats.decryptionerrors++; request->msg_flags = SNMP_V3_AUTHNOPRIV; request->error_status = SNMP_ERR_DECRYIPTION_ERROR; @@ -1782,7 +1781,7 @@ snmp_complete_outbound_frame(struct snmp_request *request) /* Encrypt response */ if (request->version == SNMP_VERSION_3 && (request->msg_flags & SNMP_V3_PRIV_FLAG)) { u8_t key[20]; - u8_t algo; + snmpv3_priv_algo_t algo; /* complete missing length in PDU sequence */ OF_BUILD_EXEC(snmp_pbuf_stream_init(&request->outbound_pbuf_stream, request->outbound_pbuf, 0, request->outbound_pbuf->tot_len)); @@ -1800,7 +1799,7 @@ snmp_complete_outbound_frame(struct snmp_request *request) if (request->version == SNMP_VERSION_3 && (request->msg_flags & SNMP_V3_AUTH_FLAG)) { u8_t key[20]; - u8_t algo; + snmpv3_auth_algo_t algo; u8_t hmac[20]; OF_BUILD_EXEC(snmpv3_get_user((char*)request->msg_user_name, &algo, key, NULL, NULL)); diff --git a/src/apps/snmp/snmp_netconn.c b/src/apps/snmp/snmp_netconn.c index cc5cb59d..24c3e265 100644 --- a/src/apps/snmp/snmp_netconn.c +++ b/src/apps/snmp/snmp_netconn.c @@ -64,11 +64,6 @@ snmp_netconn_thread(void *arg) snmp_traps_handle = conn; -#if LWIP_SNMP_V3 - /* Start the engine time timer */ - snmpv3_enginetime_timer(NULL); -#endif - do { err = netconn_recv(conn, &buf); diff --git a/src/apps/snmp/snmp_snmpv2_usm.c b/src/apps/snmp/snmp_snmpv2_usm.c index 42c08069..9a5534fe 100644 --- a/src/apps/snmp/snmp_snmpv2_usm.c +++ b/src/apps/snmp/snmp_snmpv2_usm.c @@ -57,7 +57,7 @@ static void snmp_name_to_oid(const char *name, u32_t *oid, size_t len) } } -static const struct snmp_obj_id *snmp_auth_algo_to_oid(u8_t algo) +static const struct snmp_obj_id *snmp_auth_algo_to_oid(snmpv3_auth_algo_t algo) { if (algo == SNMP_V3_AUTH_ALGO_MD5) { return &usmHMACMD5AuthProtocol; @@ -69,7 +69,7 @@ static const struct snmp_obj_id *snmp_auth_algo_to_oid(u8_t algo) return &usmNoAuthProtocol; } -static const struct snmp_obj_id *snmp_priv_algo_to_oid(u8_t algo) +static const struct snmp_obj_id *snmp_priv_algo_to_oid(snmpv3_priv_algo_t algo) { if (algo == SNMP_V3_PRIV_ALGO_DES) { return &usmDESPrivProtocol; @@ -282,7 +282,7 @@ static snmp_err_t usmusertable_get_next_instance(const u32_t *column, struct snm static s16_t usmusertable_get_value(struct snmp_node_instance *cell_instance, void *value) { - u8_t u8; + snmpv3_user_storagetype_t storage_type; switch (SNMP_TABLE_GET_COLUMN_FROM_OID(cell_instance->instance_oid.id)) { case 3: /* usmUserSecurityName */ @@ -295,8 +295,9 @@ static s16_t usmusertable_get_value(struct snmp_node_instance *cell_instance, vo case 5: /* usmUserAuthProtocol */ { const struct snmp_obj_id *auth_algo; - snmpv3_get_user((const char*)cell_instance->reference.ptr, &u8, NULL, NULL, NULL); - auth_algo = snmp_auth_algo_to_oid(u8); + snmpv3_auth_algo_t auth_algo_val; + snmpv3_get_user((const char*)cell_instance->reference.ptr, &auth_algo_val, NULL, NULL, NULL); + auth_algo = snmp_auth_algo_to_oid(auth_algo_val); MEMCPY(value, auth_algo->id, auth_algo->len * sizeof(u32_t)); return auth_algo->len * sizeof(u32_t); } @@ -307,8 +308,9 @@ static s16_t usmusertable_get_value(struct snmp_node_instance *cell_instance, vo case 8: /* usmUserPrivProtocol */ { const struct snmp_obj_id *priv_algo; - snmpv3_get_user((const char*)cell_instance->reference.ptr, NULL, NULL, &u8, NULL); - priv_algo = snmp_priv_algo_to_oid(u8); + snmpv3_priv_algo_t priv_algo_val; + snmpv3_get_user((const char*)cell_instance->reference.ptr, NULL, NULL, &priv_algo_val, NULL); + priv_algo = snmp_priv_algo_to_oid(priv_algo_val); MEMCPY(value, priv_algo->id, priv_algo->len * sizeof(u32_t)); return priv_algo->len * sizeof(u32_t); } @@ -320,8 +322,8 @@ static s16_t usmusertable_get_value(struct snmp_node_instance *cell_instance, vo /* TODO: Implement usmUserPublic */ return 0; case 12: /* usmUserStorageType */ - snmpv3_get_user_storagetype((const char*)cell_instance->reference.ptr, &u8); - *(s32_t*)value = u8; + snmpv3_get_user_storagetype((const char*)cell_instance->reference.ptr, &storage_type); + *(s32_t*)value = storage_type; return sizeof(s32_t); case 13: /* usmUserStatus */ *(s32_t*)value = 1; /* active */ diff --git a/src/apps/snmp/snmpv3_dummy.c b/src/apps/snmp/snmpv3_dummy.c index 8a2ebe96..2bafb79a 100644 --- a/src/apps/snmp/snmpv3_dummy.c +++ b/src/apps/snmp/snmpv3_dummy.c @@ -35,6 +35,7 @@ #include "lwip/apps/snmpv3.h" #include "snmpv3_priv.h" +#include "snmpv3_dummy.h" #include #include "lwip/err.h" #include "lwip/def.h" @@ -132,7 +133,7 @@ snmpv3_enginetime_timer(void *arg) } err_t -snmpv3_set_user_auth_algo(const char *username, u8_t algo) +snmpv3_set_user_auth_algo(const char *username, snmpv3_auth_algo_t algo) { struct user_table_entry *p = get_user(username); @@ -158,7 +159,7 @@ snmpv3_set_user_auth_algo(const char *username, u8_t algo) } err_t -snmpv3_set_user_priv_algo(const char *username, u8_t algo) +snmpv3_set_user_priv_algo(const char *username, snmpv3_priv_algo_t algo) { struct user_table_entry *p = get_user(username); @@ -256,7 +257,7 @@ snmpv3_set_user_priv_key(const char *username, const char *password) * @return ERR_OK if the user was found, ERR_VAL if not. */ err_t -snmpv3_get_user_storagetype(const char *username, u8_t *type) +snmpv3_get_user_storagetype(const char *username, snmpv3_user_storagetype_t *type) { if (get_user(username) != NULL) { /* Found user in user table @@ -277,7 +278,7 @@ snmpv3_get_user_storagetype(const char *username, u8_t *type) * @param priv_key is a pointer to a pointer to a string. Implementation has to set this if user was found. */ err_t -snmpv3_get_user(const char* username, u8_t *auth_algo, u8_t *auth_key, u8_t *priv_algo, u8_t *priv_key) +snmpv3_get_user(const char* username, snmpv3_auth_algo_t *auth_algo, u8_t *auth_key, snmpv3_priv_algo_t *priv_algo, u8_t *priv_key) { const struct user_table_entry *p; @@ -374,4 +375,14 @@ snmpv3_reset_engine_time(void) enginetime = 0; } +/** + * Initialize dummy SNMPv3 implementation + */ +void +snmpv3_dummy_init(void) +{ + /* Start the engine time timer */ + snmpv3_enginetime_timer(NULL); +} + #endif /* LWIP_SNMP && LWIP_SNMP_V3 */ diff --git a/src/apps/snmp/snmpv3_dummy.h b/src/apps/snmp/snmpv3_dummy.h new file mode 100644 index 00000000..16ff9180 --- /dev/null +++ b/src/apps/snmp/snmpv3_dummy.h @@ -0,0 +1,52 @@ +/** + * @file + * Additional SNMPv3 functionality RFC3414 and RFC3826. + */ + +/* + * Copyright (c) 2016 Elias Oenal. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without modification, + * are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * 3. The name of the author may not be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR IMPLIED + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT + * SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, + * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT + * OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING + * IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY + * OF SUCH DAMAGE. + * + * Author: Elias Oenal + */ + +#ifndef LWIP_HDR_APPS_SNMP_V3_DUMMY_H +#define LWIP_HDR_APPS_SNMP_V3_DUMMY_H + +#include "lwip/apps/snmp_opts.h" +#include "lwip/err.h" + +#if LWIP_SNMP && LWIP_SNMP_V3 + +err_t snmpv3_set_user_auth_algo(const char *username, snmpv3_auth_algo_t algo); +err_t snmpv3_set_user_priv_algo(const char *username, snmpv3_priv_algo_t algo); +err_t snmpv3_set_user_auth_key(const char *username, const char *password); +err_t snmpv3_set_user_priv_key(const char *username, const char *password); + +void snmpv3_dummy_init(void); + +#endif /* LWIP_SNMP && LWIP_SNMP_V3 */ + +#endif /* LWIP_HDR_APPS_SNMP_V3_DUMMY_H */ diff --git a/src/apps/snmp/snmpv3_mbedtls.c b/src/apps/snmp/snmpv3_mbedtls.c index 2a016106..64ba8757 100644 --- a/src/apps/snmp/snmpv3_mbedtls.c +++ b/src/apps/snmp/snmpv3_mbedtls.c @@ -107,7 +107,7 @@ free_md: err_t snmpv3_crypt(struct snmp_pbuf_stream* stream, u16_t length, const u8_t* key, const u8_t* priv_param, const u32_t engine_boots, - const u32_t engine_time, u8_t algo, u8_t mode) + const u32_t engine_time, u8_t algo, snmpv3_priv_mode_t mode) { size_t i; mbedtls_cipher_context_t ctx; diff --git a/src/apps/snmp/snmpv3_priv.h b/src/apps/snmp/snmpv3_priv.h index ccf0e3ab..f3d03ce5 100644 --- a/src/apps/snmp/snmpv3_priv.h +++ b/src/apps/snmp/snmpv3_priv.h @@ -59,11 +59,17 @@ #define SNMP_V3_MD5_LEN 16 #define SNMP_V3_SHA_LEN 20 +typedef enum +{ + SNMP_V3_PRIV_MODE_DECRYPT = 0, + SNMP_V3_PRIV_MODE_ENCRYPT = 1 +} snmpv3_priv_mode_t; + s32_t snmpv3_get_engine_boots_internal(void); s32_t snmpv3_get_engine_time_internal(void); err_t snmpv3_auth(struct snmp_pbuf_stream* stream, u16_t length, const u8_t* key, u8_t algo, u8_t* hmac_out); err_t snmpv3_crypt(struct snmp_pbuf_stream* stream, u16_t length, const u8_t* key, - const u8_t* priv_param, const u32_t engine_boots, const u32_t engine_time, u8_t algo, u8_t mode); + const u8_t* priv_param, const u32_t engine_boots, const u32_t engine_time, u8_t algo, snmpv3_priv_mode_t mode); err_t snmpv3_build_priv_param(u8_t* priv_param); void snmpv3_enginetime_timer(void *arg); diff --git a/src/include/lwip/apps/snmpv3.h b/src/include/lwip/apps/snmpv3.h index 618ce7cb..459b2cc4 100644 --- a/src/include/lwip/apps/snmpv3.h +++ b/src/include/lwip/apps/snmpv3.h @@ -40,22 +40,28 @@ #if LWIP_SNMP && LWIP_SNMP_V3 -#define SNMP_V3_AUTH_ALGO_INVAL 0 -#define SNMP_V3_AUTH_ALGO_MD5 1 -#define SNMP_V3_AUTH_ALGO_SHA 2 +typedef enum +{ + SNMP_V3_AUTH_ALGO_INVAL = 0, + SNMP_V3_AUTH_ALGO_MD5 = 1, + SNMP_V3_AUTH_ALGO_SHA = 2 +} snmpv3_auth_algo_t; -#define SNMP_V3_PRIV_ALGO_INVAL 0 -#define SNMP_V3_PRIV_ALGO_DES 1 -#define SNMP_V3_PRIV_ALGO_AES 2 +typedef enum +{ + SNMP_V3_PRIV_ALGO_INVAL = 0, + SNMP_V3_PRIV_ALGO_DES = 1, + SNMP_V3_PRIV_ALGO_AES = 2 +} snmpv3_priv_algo_t; -#define SNMP_V3_PRIV_MODE_DECRYPT 0 -#define SNMP_V3_PRIV_MODE_ENCRYPT 1 - -#define SNMP_V3_USER_STORAGETYPE_OTHER 1 -#define SNMP_V3_USER_STORAGETYPE_VOLATILE 2 -#define SNMP_V3_USER_STORAGETYPE_NONVOLATILE 3 -#define SNMP_V3_USER_STORAGETYPE_PERMANENT 4 -#define SNMP_V3_USER_STORAGETYPE_READONLY 5 +typedef enum +{ + SNMP_V3_USER_STORAGETYPE_OTHER = 1, + SNMP_V3_USER_STORAGETYPE_VOLATILE = 2, + SNMP_V3_USER_STORAGETYPE_NONVOLATILE = 3, + SNMP_V3_USER_STORAGETYPE_PERMANENT = 4, + SNMP_V3_USER_STORAGETYPE_READONLY = 5 +} snmpv3_user_storagetype_t; /* * The following callback functions must be implemented by the application. @@ -71,16 +77,11 @@ void snmpv3_set_engine_boots(u32_t boots); u32_t snmpv3_get_engine_time(void); void snmpv3_reset_engine_time(void); -err_t snmpv3_get_user(const char* username, u8_t *auth_algo, u8_t *auth_key, u8_t *priv_algo, u8_t *priv_key); +err_t snmpv3_get_user(const char* username, snmpv3_auth_algo_t *auth_algo, u8_t *auth_key, snmpv3_priv_algo_t *priv_algo, u8_t *priv_key); u8_t snmpv3_get_amount_of_users(void); -err_t snmpv3_get_user_storagetype(const char *username, u8_t *storagetype); +err_t snmpv3_get_user_storagetype(const char *username, snmpv3_user_storagetype_t *storagetype); err_t snmpv3_get_username(char *username, u8_t index); -err_t snmpv3_set_user_auth_algo(const char *username, u8_t algo); -err_t snmpv3_set_user_priv_algo(const char *username, u8_t algo); -err_t snmpv3_set_user_auth_key(const char *username, const char *password); -err_t snmpv3_set_user_priv_key(const char *username, const char *password); - /* The following functions are provided by the SNMPv3 agent */ void snmpv3_engine_id_changed(void);