From 9f4a68be73bb4e0c332616cdbaf6e7e7e2cf923a Mon Sep 17 00:00:00 2001 From: Vojtech Vilimek Date: Wed, 18 Oct 2023 13:30:14 +0200 Subject: [PATCH] SNMP: Minor changes in get, get_next handling --- proto/snmp/bgp_mib.c | 6 +- proto/snmp/snmp.c | 107 +++++++++++++++------------- proto/snmp/snmp.h | 5 ++ proto/snmp/snmp_utils.c | 29 +++++++- proto/snmp/snmp_utils.h | 5 +- proto/snmp/subagent.c | 151 ++++++++++++++++------------------------ proto/snmp/subagent.h | 9 ++- 7 files changed, 162 insertions(+), 150 deletions(-) diff --git a/proto/snmp/bgp_mib.c b/proto/snmp/bgp_mib.c index 647b21c36..2628ace1d 100644 --- a/proto/snmp/bgp_mib.c +++ b/proto/snmp/bgp_mib.c @@ -1237,12 +1237,12 @@ UNUSED, uint contid UNUSED, int byte_ord UNUSED, u8 state) if (oid_state_compare(oid, state) < 0 || state == BGP_INTERNAL_END) { vb->type = AGENTX_NO_SUCH_OBJECT; - return ((byte *) vb) + snmp_varbind_header_size(vb); + return pkt + snmp_varbind_header_size(vb); } else if (oid_state_compare(oid, state) > 0) { vb->type = AGENTX_NO_SUCH_INSTANCE; - return ((byte *) vb) + snmp_varbind_header_size(vb); + return pkt + snmp_varbind_header_size(vb); } switch (state) @@ -1262,7 +1262,7 @@ UNUSED, uint contid UNUSED, int byte_ord UNUSED, u8 state) default: vb->type = AGENTX_NO_SUCH_OBJECT; - pkt = ((byte *) vb) + snmp_varbind_header_size(vb); + pkt += snmp_varbind_header_size(vb); break; } diff --git a/proto/snmp/snmp.c b/proto/snmp/snmp.c index 1e23cac74..31f3391d6 100644 --- a/proto/snmp/snmp.c +++ b/proto/snmp/snmp.c @@ -172,6 +172,34 @@ snmp_down(struct snmp_proto *p) proto_notify_state(&p->p, PS_DOWN); } +/* this function is internal and shouldn't be used outside the snmp module */ +void +snmp_connected(sock *sk) +{ + struct snmp_proto *p = sk->data; + snmp_log("snmp_connected() connection created"); + + p->state = SNMP_OPEN; + + sk->rx_hook = snmp_rx; + sk->tx_hook = NULL; + //sk->tx_hook = snmp_tx; + + snmp_start_subagent(p); + + // TODO ping interval + tm_set(p->ping_timer, current_time() + p->timeout S); +} + +/* this function is internal and shouldn't be used outside the snmp module */ +void +snmp_reconnect(timer *tm) +{ + struct snmp_proto *p = tm->data; + ASSUME(p->sock); + snmp_connected(p->sock); +} + static void snmp_sock_err(sock *sk, int err) { @@ -188,27 +216,10 @@ snmp_sock_err(sock *sk, int err) snmp_log("changing proto_snmp state to LOCKED"); p->state = SNMP_LOCKED; - // TODO ping interval - tm_start(p->startup_timer, 4 S); + p->startup_timer->hook = snmp_startup_timeout; + tm_start(p->startup_timer, 4 S); // TODO make me configurable } -static void -snmp_connected(sock *sk) -{ - struct snmp_proto *p = sk->data; - snmp_log("snmp_connected() connection created"); - - p->state = SNMP_OPEN; - - sk->rx_hook = snmp_rx; - sk->tx_hook = NULL; - //sk->tx_hook = snmp_tx; - - snmp_start_subagent(p); - - // TODO ping interval - tm_set(p->ping_timer, p->timeout S); -} static void snmp_start_locked(struct object_lock *lock) @@ -217,25 +228,28 @@ snmp_start_locked(struct object_lock *lock) struct snmp_proto *p = lock->data; p->state = SNMP_LOCKED; + sock *s = p->sock; - sock *s = sk_new(p->p.pool); - s->type = SK_TCP_ACTIVE; - s->saddr = p->local_ip; - s->daddr = p->remote_ip; - s->dport = p->remote_port; - s->rbsize = SNMP_RX_BUFFER_SIZE; - s->tbsize = SNMP_TX_BUFFER_SIZE; - - //s->tos = IP_PREC_INTERNET_CONTROL - //s->rx_hook = snmp_connected; - s->tx_hook = snmp_connected; - s->err_hook = snmp_sock_err; - - p->sock = s; - s->data = p; - - p->to_send = 0; - p->errs = 0; + if (!s) + { + s = sk_new(p->p.pool); + s->type = SK_TCP_ACTIVE; + s->saddr = p->local_ip; + s->daddr = p->remote_ip; + s->dport = p->remote_port; + s->rbsize = SNMP_RX_BUFFER_SIZE; + s->tbsize = SNMP_TX_BUFFER_SIZE; + + //s->tos = IP_PREC_INTERNET_CONTROL + s->tx_hook = snmp_connected; + s->err_hook = snmp_sock_err; + + p->sock = s; + s->data = p; + + p->to_send = 0; + p->errs = 0; + } if (sk_open(s) < 0) { @@ -246,7 +260,8 @@ snmp_start_locked(struct object_lock *lock) } } -static void +/* this function is internal and shouldn't be used outside the snmp module */ +void snmp_startup(struct snmp_proto *p) { if (p->state != SNMP_INIT && @@ -289,7 +304,8 @@ snmp_startup(struct snmp_proto *p) */ } -static void +/* this function is internal and shouldn't be used outside the snmp module */ +void snmp_startup_timeout(timer *t) { snmp_log("startup timer triggered"); @@ -307,7 +323,7 @@ snmp_stop_timeout(timer *t) } static void -snmp_ping_timeout(struct timer *tm) +snmp_ping_timeout(timer *tm) { struct snmp_proto *p = tm->data; @@ -385,15 +401,6 @@ snmp_start(struct proto *P) } } - { - u32 *ptr = mb_alloc(p->p.pool, 4 * sizeof(u32)); - *ptr = 1; - ptr[2] = 4; - (void)ptr[1]; (void)ptr[0]; (void)ptr[2]; - mb_free(ptr); - log(L_INFO "testing alloc 3"); - } - snmp_log("values of context cf %u proto %u", cf->contexts, p->context_max); ASSUME(cf->contexts == p->context_max); @@ -532,7 +539,7 @@ snmp_shutdown(struct proto *P) p->startup_timer->hook = snmp_stop_timeout; - tm_set(p->startup_timer, p->timeout S); + tm_set(p->startup_timer, current_time() + p->timeout S); snmp_stop_subagent(p); diff --git a/proto/snmp/snmp.h b/proto/snmp/snmp.h index ae737ee48..60e460299 100644 --- a/proto/snmp/snmp.h +++ b/proto/snmp/snmp.h @@ -169,5 +169,10 @@ struct snmp_proto { }; //void snmp_tx(sock *sk); +void snmp_startup(struct snmp_proto *p); +void snmp_connected(sock *sk); +void snmp_startup_timeout(timer *tm); +void snmp_reconnect(timer *tm); + #endif diff --git a/proto/snmp/snmp_utils.c b/proto/snmp/snmp_utils.c index c678f89da..e658adc48 100644 --- a/proto/snmp/snmp_utils.c +++ b/proto/snmp/snmp_utils.c @@ -31,7 +31,7 @@ snmp_is_oid_empty(const struct oid *oid) * @pkt: first byte past packet end */ uint -snmp_pkt_len(byte *start, byte *end) +snmp_pkt_len(const byte *start, const byte *end) { snmp_log("snmp_pkt_len start 0x%p end 0x%p res %u", start, end, (end - start) - AGENTX_HEADER_SIZE); @@ -42,7 +42,8 @@ snmp_pkt_len(byte *start, byte *end) * * used for copying oid to in buffer oid @dest */ -void snmp_oid_copy(struct oid *dest, const struct oid *src) +void +snmp_oid_copy(struct oid *dest, const struct oid *src) { STORE_U8(dest->n_subid, src->n_subid); STORE_U8(dest->prefix, src->prefix); @@ -149,6 +150,28 @@ snmp_varbind_size(struct agentx_varbind *vb, int byte_ord) return hdr_size + snmp_str_size_from_len(LOAD_PTR(data, byte_ord)); } +/* test if the varbind has valid type */ +int +snmp_test_varbind(const struct agentx_varbind *vb) +{ + if (vb->type == AGENTX_INTEGER || + vb->type == AGENTX_OCTET_STRING || + vb->type == AGENTX_NULL || + vb->type == AGENTX_OBJECT_ID || + vb->type == AGENTX_IP_ADDRESS || + vb->type == AGENTX_COUNTER_32 || + vb->type == AGENTX_GAUGE_32 || + vb->type == AGENTX_TIME_TICKS || + vb->type == AGENTX_OPAQUE || + vb->type == AGENTX_COUNTER_64 || + vb->type == AGENTX_NO_SUCH_OBJECT || + vb->type == AGENTX_NO_SUCH_INSTANCE || + vb->type == AGENTX_END_OF_MIB_VIEW) + return 1; + else + return 0; +} + /* inline uint snmp_context_size(struct agentx_context *c) @@ -297,7 +320,7 @@ snmp_oid_ip4_index(struct oid *o, uint start, ip4_addr addr) STORE_U32(o->ids[start + 3], temp & 0xFF); } -void snmp_oid_dump(struct oid *oid) +void snmp_oid_dump(const struct oid *oid) { log(L_WARN "OID DUMP ========"); diff --git a/proto/snmp/snmp_utils.h b/proto/snmp/snmp_utils.h index 95fec8701..deeb7b9b5 100644 --- a/proto/snmp/snmp_utils.h +++ b/proto/snmp/snmp_utils.h @@ -3,7 +3,7 @@ #include "subagent.h" -uint snmp_pkt_len(byte *start, byte *end); +uint snmp_pkt_len(const byte *start, const byte *end); size_t snmp_str_size_from_len(uint len); size_t snmp_str_size(const char *str); int snmp_is_oid_empty(const struct oid *oid); @@ -14,6 +14,7 @@ size_t snmp_oid_sizeof(uint n_subid); uint snmp_varbind_hdr_size_from_oid(struct oid *oid); uint snmp_varbind_header_size(struct agentx_varbind *vb); uint snmp_varbind_size(struct agentx_varbind *vb, int byte_ord); +int snmp_test_varbind(const struct agentx_varbind *vb); //uint snmp_context_size(struct agentx_context *c); void snmp_oid_copy(struct oid *dest, const struct oid *src); @@ -40,7 +41,7 @@ byte *snmp_put_fbyte(byte *buf, u8 data); void snmp_oid_ip4_index(struct oid *o, uint start, ip4_addr addr); -void snmp_oid_dump(struct oid *oid); +void snmp_oid_dump(const struct oid *oid); //struct oid *snmp_prefixize(struct snmp_proto *p, struct oid *o, int byte_ord); diff --git a/proto/snmp/subagent.c b/proto/snmp/subagent.c index 200224658..692372b56 100644 --- a/proto/snmp/subagent.c +++ b/proto/snmp/subagent.c @@ -34,7 +34,7 @@ static uint parse_gets2_pdu(struct snmp_proto *p, byte *buf, uint size, uint *sk static uint parse_close_pdu(struct snmp_proto *p, byte *buf, uint size); static struct agentx_response *prepare_response(struct snmp_proto *p, struct snmp_pdu *c); static void response_err_ind(struct agentx_response *res, uint err, uint ind); -static uint update_packet_size(struct snmp_proto *p, byte *start, byte *end); +static uint update_packet_size(struct snmp_proto *p, const byte *start, byte *end); static struct oid *search_mib(struct snmp_proto *p, const struct oid *o_start, const struct oid *o_end, struct oid *o_curr, struct snmp_pdu *c, enum snmp_search_res *result); u32 snmp_internet[] = { SNMP_ISO, SNMP_ORG, SNMP_DOD, SNMP_INTERNET }; @@ -352,34 +352,6 @@ close_pdu(struct snmp_proto *p, u8 reason) } #if 0 -static void UNUSED -parse_testset_pdu(struct snmp_proto *p) -{ - sock *sk = p->sock; - sk_send(sk, 0); -} - -static void UNUSED -parse_commitset_pdu(struct snmp_proto *p) -{ - sock *sk = p->sock; - sk_send(sk, 0); -} - -static void UNUSED -parse_undoset_pdu(struct snmp_proto *p) -{ - sock *sk = p->sock; - sk_send(sk, 0); -} - -static void UNUSED -parse_cleanupset_pdu(struct snmp_proto *p) -{ - sock *sk = p->sock; - sk_send(sk, 0); -} - static void UNUSED addagentcaps_pdu(struct snmp_proto *p, struct oid *cap, char *descr, uint descr_len, struct agentx_context *c) @@ -673,7 +645,8 @@ snmp_get_mib_class(const struct oid *oid) } } -static void +/* return 0 if the created varbind type is END_OF_MIB_VIEW, 1 otherwise */ +static int snmp_get_next2(struct snmp_proto *p, struct oid *o_start, struct oid *o_end, struct snmp_pdu *c) { @@ -695,13 +668,13 @@ snmp_get_next2(struct snmp_proto *p, struct oid *o_start, struct oid *o_end, { /* TODO create NULL varbind */ c->error = AGENTX_RES_GEN_ERROR; - return; + return 0; } vb = snmp_create_varbind(c->buffer, o_start); vb->type = AGENTX_END_OF_MIB_VIEW; ADVANCE(c->buffer, c->size, snmp_varbind_header_size(vb)); - return; + return 0; case SNMP_SEARCH_OK: default: @@ -721,13 +694,11 @@ snmp_get_next2(struct snmp_proto *p, struct oid *o_start, struct oid *o_end, case AGENTX_NO_SUCH_INSTANCE: case AGENTX_END_OF_MIB_VIEW: vb->type = AGENTX_END_OF_MIB_VIEW; - break; + return 0; - default: /* intentionally left blank */ - break; + default: + return 1; } - - return; } if (c->size < snmp_varbind_hdr_size_from_oid(o_start)) @@ -739,22 +710,14 @@ snmp_get_next2(struct snmp_proto *p, struct oid *o_start, struct oid *o_end, vb = snmp_create_varbind(c->buffer, o_start); vb->type = AGENTX_END_OF_MIB_VIEW; ADVANCE(c->buffer, c->size, snmp_varbind_header_size(vb)); + return 0; } -static void +/* returns 0 if the created varbind has type EndOfMibView, 1 otherwise */ +static int snmp_get_bulk2(struct snmp_proto *p, struct oid *o_start, struct oid *o_end, struct agentx_bulk_state *state, struct snmp_pdu *c) { - if (state->index <= state->getbulk.non_repeaters) - { - return snmp_get_next2(p, o_start, o_end, c); - /* - * Here we don't need to do any overriding, not even in case no object was - * found, as the GetNext-PDU override is same as GetBulk-PDU override - * (to AGENTX_RES_END_OF_MIB_VIEW) - */ - } - struct oid *o_curr = NULL; struct oid *o_predecessor = NULL; enum snmp_search_res r; @@ -765,8 +728,10 @@ snmp_get_bulk2(struct snmp_proto *p, struct oid *o_start, struct oid *o_end, o_predecessor = o_curr; o_curr = search_mib(p, o_start, o_end, o_curr, c, &r); i++; - } while (o_curr && i <= state->repetition); - + } while (o_curr && i < state->repetition); + + // TODO check if the approach below works + // it need to generate varbinds that will be only of type EndOfMibView /* Object Identifier fall-backs */ if (!o_curr) o_curr = o_predecessor; @@ -779,7 +744,7 @@ snmp_get_bulk2(struct snmp_proto *p, struct oid *o_start, struct oid *o_end, if (c->size < sz) { c->error = AGENTX_RES_GEN_ERROR; - return; + return 0; } /* we need the varbind handle to be able to override it's type */ @@ -799,41 +764,15 @@ snmp_get_bulk2(struct snmp_proto *p, struct oid *o_start, struct oid *o_end, case AGENTX_NO_SUCH_INSTANCE: case AGENTX_END_OF_MIB_VIEW: vb->type = AGENTX_END_OF_MIB_VIEW; - break; - - default: /* intentionally left blank */ - break; - } -} - -static uint UNUSED -parse_close_pdu(struct snmp_proto UNUSED *p, byte UNUSED *req, uint UNUSED size) -{ - /* - snmp_log("parse_close_pdu()"); - - // byte *pkt = req; - // sock *sk = p->sock; + return 0; - if (size < sizeof(struct agentx_header)) - { - snmp_log("p_close early return"); - return 0; + default: + return 1; } - - // struct agentx_header *h = (void *) req; - ADVANCE(req, size, AGENTX_HEADER_SIZE); - //snmp_log("after header %p", req); - - p->state = SNMP_ERR; - - proto_notify_state(&p->p, PS_DOWN); - */ - return 0; } static inline uint -update_packet_size(struct snmp_proto *p, byte *start, byte *end) +update_packet_size(struct snmp_proto *p, const byte *start, byte *end) { /* work even for partial messages */ struct agentx_header *h = (void *) p->sock->tpos; @@ -906,6 +845,11 @@ parse_gets2_pdu(struct snmp_proto *p, byte * const pkt_start, uint size, uint *s c.context = ... */ + /* + * Get-Bulk processing stops if all the varbind have type END_OF_MIB_VIEW + * has_any is true if some varbind has type other than END_OF_MIB_VIEW + */ + int has_any = 0; struct agentx_bulk_state bulk_state = { }; if (h->type == AGENTX_GET_BULK_PDU) { @@ -926,22 +870,24 @@ parse_gets2_pdu(struct snmp_proto *p, byte * const pkt_start, uint size, uint *s .non_repeaters = LOAD_U32(bulk_info->non_repeaters, c.byte_ord), .max_repetitions = LOAD_U32(bulk_info->max_repetitions, c.byte_ord), }, - .index = 1, - .repetition = 1, + /* In contrast to the RFC, we use 0-based indices. */ + .index = 0, + .repetition = 0, }; } if (!p->partial_response && c.size < sizeof(struct agentx_response)) { snmp_manage_tbuf(p, &c); + // TODO renew pkt, pkt_start pointers context clen } struct agentx_response *response_header = prepare_response(p, &c); - uint ind = 1; + uint ind = 0; while (c.error == AGENTX_RES_NO_ERROR && size > 0 && pkt_size > 0) { - snmp_log("iter %u size %u remaining %u/%u", ind, c.buffer - sk->tpos, size, pkt_size); + snmp_log("iter %u size %u remaining %u/%u", ind + 1, c.buffer - sk->tpos, size, pkt_size); if (size < snmp_oid_sizeof(0)) goto partial; @@ -961,9 +907,9 @@ parse_gets2_pdu(struct snmp_proto *p, byte * const pkt_start, uint size, uint *s * we send processed part, otherwise we don't have anything to send and * need to wait for more data to be recieved. */ - if (sz > size && ind > 1) + if (sz > size && ind > 0) { - snmp_log("sz %u > %u size && ind %u > 1", sz, size, ind); + snmp_log("sz %u > %u size && ind %u > 1", sz, size, ind + 1); goto partial; /* send already processed part */ } else if (sz > size) @@ -988,9 +934,9 @@ parse_gets2_pdu(struct snmp_proto *p, byte * const pkt_start, uint size, uint *s goto send; } - if (sz > size && ind > 1) + if (sz > size && ind > 0) { - snmp_log("sz2 %u > %u size && ind %u > 1", sz, size, ind); + snmp_log("sz2 %u > %u size && ind %u > 1", sz, size, ind + 1); size += snmp_oid_size(o_start_b); goto partial; } @@ -1028,7 +974,14 @@ parse_gets2_pdu(struct snmp_proto *p, byte * const pkt_start, uint size, uint *s break; case AGENTX_GET_BULK_PDU: - snmp_get_bulk2(p, o_start, o_end, &bulk_state, &c); + if (ind >= bulk_state.getbulk.non_repeaters) + bulk_state.repeaters++; + + // store the o_start, o_end + + /* The behavior of GetBulk pdu in the first iteration is + * identical to GetNext pdu. */ + has_any = has_any || snmp_get_next2(p, o_start, o_end, &c); break; default: @@ -1043,11 +996,27 @@ parse_gets2_pdu(struct snmp_proto *p, byte * const pkt_start, uint size, uint *s ind++; } /* while (c.error == AGENTX_RES_NO_ERROR && size > 0) */ + if (h->type == AGENTX_GET_BULK_PDU) + { + for (bulk_state.repetition++; + has_any && bulk_state.repetition < bulk_state.getbulk.max_repetitions; + bulk_state.repetition++) + { + // TODO find propper start and end + struct oid *start = NULL; + struct oid *end = NULL; + has_any = 0; + for (bulk_state.index = 0; bulk_state.index < bulk_state.repeaters; + bulk_state.repeaters++) + has_any = has_any || snmp_get_bulk2(p, start, end, &bulk_state, &c); + } + } + send: snmp_log("gets2: sending response ..."); struct agentx_response *res = (void *) sk->tbuf; /* We update the error, index pair on the beginning of the packet. */ - response_err_ind(res, c.error, ind); + response_err_ind(res, c.error, ind + 1); uint s = update_packet_size(p, (byte *) response_header, c.buffer); snmp_log("sending response to Get-PDU, GetNext-PDU or GetBulk-PDU request (size %u)...", s); diff --git a/proto/snmp/subagent.h b/proto/snmp/subagent.h index 537e66045..26fb07a04 100644 --- a/proto/snmp/subagent.h +++ b/proto/snmp/subagent.h @@ -169,7 +169,7 @@ struct agentx_header { u32 session_id; /* AgentX sessionID established by Open-PDU */ u32 transaction_id; /* last transactionID seen/used */ u32 packet_id; /* last packetID seen/used */ - u32 payload; /* length of the packet without header */ + u32 payload; /* payload_length of the packet without header */ }; #define AGENTX_HEADER_SIZE sizeof(struct agentx_header) @@ -223,6 +223,7 @@ struct agentx_bulk_state { struct agentx_getbulk getbulk; u16 index; u16 repetition; + u32 repeaters; }; enum agentx_pdu { @@ -257,6 +258,12 @@ enum agentx_flags { AGENTX_NETWORK_BYTE_ORDER = 0x10, } PACKED; +#define AGENTX_FLAGS (AGENTX_FLAG_INSTANCE_REGISTRATION \ + | AGENTX_FLAG_NEW_INDEX \ + | AGENTX_FLAG_ANY_INDEX \ + | AGENTX_NON_DEFAULT_CONTEXT \ + | AGENTX_NETWORK_BYTE_ORDER) + /* CLOSE_PDU close reasons */ enum agentx_close_reasons { AGENTX_CLOSE_OTHER = 1, -- 2.47.2