From: Martijn van Duren Subject: Re: snmpd/agentx compatibility issue To: tech@openbsd.org Date: Thu, 14 Mar 2024 10:39:19 +0100 On Sun, 2024-02-11 at 13:36 +0100, Martijn van Duren wrote: > Moving to tech@ > > On Sun, 2024-02-11 at 11:17 +0100, Steffen Christgau wrote: > > Hi, > > > > I use snmpd on OpenBSD 7.4 and try to get it work with PyAgentX3 [1] in > > order to provide additional data to the SNMP daemon. To test it, I use > > the minimal agent as provided in the PyAgentX3 readme (with the "print" > > function adjusted to Python 3). The respective config line for snmpd > > looks as simple as it could be: > > > > agentx > > > > Connection management and registration between the two processes work. > > Unfortunately, no data is returned when issuing a GET command using both > > OpenBSD's "snmp get" on the local machine and NET-SNMP's "snmpget" on a > > remote Linux machine. Instead the connection to the subagent is aborted. > > Same applies to a "walk". > > > > The reason for that appears to be a check of the packetID in the AgentX > > PDU header by snmpd. From what I understand from RFC 2257, it is > > expected that the packetID of a response must the same as in the > > corresponding request. PyAgentX3 correctly does that. However, snmpd > > drops the response PDU during the handling of AX_PDU_TYPE_RESPONSE in > > ax_recv (ax.c) because it does not know the packet id, i.e. it's not in > > ax_packetids. > > > > From my point of view, this is due to appl_agentx_get calling ax_get > > with the request_id externally provided from the client (?). In ax_get, > > ax_pdu_header is called with that request_id. In turn, ax_pdu_header > > only asks for new IDs in case it is non-zero: > > > > if (packetid == 0) > > packetid = ax_packetid(ax); > > /* possible fix > > else if (type == AX_PDU_TYPE_GET /*NEXT ?! */) > > ax_record_packetid(packetid) > > */ > > > > So essentially, the request ID is passed through to the agentx subagent > > but not recorded in the array of known IDs causing the refusal of the > > subagent's response and finally connection close of the agentx socket. > > > > Since I am not an agentx/snmp(d) expert, I'm not sure if that's a bug in > > snmpd or expected behavior. I would be happy if somebody could confirm > > either way. > > > > That being said, my current fix was to disable the check for known ids > > in the handling of AX_PDU_TYPE_RESPONSEs, but that's apparently a bad > > idea. With that, however, both walking tree and getting the respective > > OIDs work as intended. Maybe the pseudo code above would be a better fix. > > > > Regards, Steffen > > > > [1] https://github.com/rprinz08/pyagentx3 > > > Thanks for the report and analysis. I'm also quite confident this > explains the two elusive bug reports I've received. > > First of, your analysis is correct, but I'd prefer to remove the code > instead of creating an ax_record_packetid(). The reason this code was > originally added to ax.c was that I wanted to lift some burden out of > libagentx's agentx.c into ax.c to deduplicate a bit of code when it > came to administrative packets. When I copied this over to snmpd I > kept it, but the code was effectively never called, because ax_get, > and ax_getnext get their packetid from application.c's requestid. > ax_response simply copies the packetid, and other than that only > ax_close can be called, which closes the connection after that anyway. > So there's no reason to keep it around from that regard and ax.c is > supposed to be a translation layer between network and C format, so > this code also doesn't belong here from that perspective. > > The reason that this triggers here and why it made the other bugs so > elusive is because ax_response()'s call to ax_pdu_header() also called > ax_packetid() unconditionally, which meant that an incoming packetid of > 0 (what pyagentx3 does) would generated an (invalid) new outgoing > packetid and would enable the `if (ax->ax_packetids != NULL)` test for > incoming responses, making it reproducible. So how does this relate to > the earlier reported crashes? If once in a blue moon a requestid of 0 is > generated this code path would be enabled, resulting in EPROTO, which in > turn puts disables the backend. Where disabling a libexec backend for > snmpd results in snmpd shutting down. > > This means that it should also be removed from libagentx, but ax's > usage there is a bit more involved, so I need a little more time to > build a diff for there. > > OK? > > martijn@ > I've played around a bit with the libagentx bits and there's plenty to improve (it's almost as if one gets new insights with experience). But I reckon this is the most straightforward diff to fix the packetid issue. OK? martijn@ diff db9d9b4b3c06c791a4f448a5521c340160f4705e 3f7b601986d696996ff30f574183096979f23ec7 commit - db9d9b4b3c06c791a4f448a5521c340160f4705e commit + 3f7b601986d696996ff30f574183096979f23ec7 blob - 191892d6687fb364b54ff66dd5d572ba8346c77a blob + ad51662c9ca9cb82afa9cf5c532c4d9c141ebc4b --- agentx.c +++ agentx.c @@ -187,7 +187,7 @@ static void agentx_varbind_nosuchinstance(struct agent static void agentx_varbind_endofmibview(struct agentx_varbind *); static void agentx_varbind_error_type(struct agentx_varbind *, enum ax_pdu_error, int); -static int agentx_request(struct agentx *, uint32_t, +static struct agentx_request *agentx_request(struct agentx *, int (*)(struct ax_pdu *, void *), void *); static int agentx_request_cmp(struct agentx_request *, struct agentx_request *); @@ -447,7 +447,7 @@ static int agentx_session_start(struct agentx_session *axs) { struct agentx *ax = axs->axs_ax; - uint32_t packetid; + struct agentx_request *axr; #ifdef AX_DEBUG if (ax->ax_cstate != AX_CSTATE_OPEN || @@ -456,18 +456,20 @@ agentx_session_start(struct agentx_session *axs) agentx_log_ax_fatalx(ax, "%s: unexpected session open", __func__); #endif - packetid = ax_open(ax->ax_ax, axs->axs_timeout, &(axs->axs_oid), - &(axs->axs_descr)); - if (packetid == 0) { + if ((axr = agentx_request(ax, agentx_session_finalize, axs)) == NULL) + return -1; + if (ax_open(ax->ax_ax, axs->axs_timeout, axr->axr_packetid, + &(axs->axs_oid), &(axs->axs_descr)) == -1) { agentx_log_ax_warn(ax, "couldn't generate %s", ax_pdutype2string(AX_PDU_TYPE_OPEN)); agentx_reset(ax); return -1; } - axs->axs_packetid = packetid; + axs->axs_packetid = axr->axr_packetid; agentx_log_ax_info(ax, "opening session"); axs->axs_cstate = AX_CSTATE_WAITOPEN; - return agentx_request(ax, packetid, agentx_session_finalize, axs); + agentx_wantwrite(ax, ax->ax_fd); + return 0; } static int @@ -510,14 +512,17 @@ agentx_session_close(struct agentx_session *axs, enum ax_close_reason reason) { struct agentx *ax = axs->axs_ax; - uint32_t packetid; + struct agentx_request *axr; #ifdef AX_DEBUG if (axs->axs_cstate != AX_CSTATE_OPEN) agentx_log_ax_fatalx(ax, "%s: unexpected session close", __func__); #endif - if ((packetid = ax_close(ax->ax_ax, axs->axs_id, reason)) == 0) { + axr = agentx_request(ax, agentx_session_close_finalize, axs); + if (axr == NULL) + return -1; + if (ax_close(ax->ax_ax, axs->axs_id, axr->axr_packetid, reason) == -1) { agentx_log_axs_warn(axs, "couldn't generate %s", ax_pdutype2string(AX_PDU_TYPE_CLOSE)); agentx_reset(ax); @@ -528,8 +533,8 @@ agentx_session_close(struct agentx_session *axs, ax_closereason2string(reason)); axs->axs_cstate = AX_CSTATE_WAITCLOSE; - return agentx_request(ax, packetid, agentx_session_close_finalize, - axs); + agentx_wantwrite(ax, ax->ax_fd); + return 0; } static int @@ -920,7 +925,7 @@ agentx_agentcaps_start(struct agentx_agentcaps *axa) struct agentx_context *axc = axa->axa_axc; struct agentx_session *axs = axc->axc_axs; struct agentx *ax = axs->axs_ax; - uint32_t packetid; + struct agentx_request *axr; #ifdef AX_DEBUG if (axc->axc_cstate != AX_CSTATE_OPEN || @@ -930,9 +935,10 @@ agentx_agentcaps_start(struct agentx_agentcaps *axa) "%s: unexpected region registration", __func__); #endif - packetid = ax_addagentcaps(ax->ax_ax, axs->axs_id, - AGENTX_CONTEXT_CTX(axc), &(axa->axa_oid), &(axa->axa_descr)); - if (packetid == 0) { + if ((axr = agentx_request(ax, agentx_agentcaps_finalize, axa)) == NULL) + return -1; + if (ax_addagentcaps(ax->ax_ax, axs->axs_id, axr->axr_packetid, + AGENTX_CONTEXT_CTX(axc), &(axa->axa_oid), &(axa->axa_descr)) == -1) { agentx_log_axc_warn(axc, "couldn't generate %s", ax_pdutype2string(AX_PDU_TYPE_ADDAGENTCAPS)); agentx_reset(ax); @@ -941,8 +947,8 @@ agentx_agentcaps_start(struct agentx_agentcaps *axa) agentx_log_axc_info(axc, "agentcaps %s: opening", ax_oid2string(&(axa->axa_oid))); axa->axa_cstate = AX_CSTATE_WAITOPEN; - return agentx_request(ax, packetid, agentx_agentcaps_finalize, - axa); + agentx_wantwrite(ax, ax->ax_fd); + return 0; } static int @@ -983,7 +989,7 @@ agentx_agentcaps_close(struct agentx_agentcaps *axa) struct agentx_context *axc = axa->axa_axc; struct agentx_session *axs = axc->axc_axs; struct agentx *ax = axs->axs_ax; - uint32_t packetid; + struct agentx_request *axr; #ifdef AX_DEBUG if (axa->axa_cstate != AX_CSTATE_OPEN) @@ -995,9 +1001,11 @@ agentx_agentcaps_close(struct agentx_agentcaps *axa) if (axs->axs_cstate == AX_CSTATE_WAITCLOSE) return 0; - packetid = ax_removeagentcaps(ax->ax_ax, axs->axs_id, - AGENTX_CONTEXT_CTX(axc), &(axa->axa_oid)); - if (packetid == 0) { + axr = agentx_request(ax, agentx_agentcaps_close_finalize, axa); + if (axr == NULL) + return -1; + if (ax_removeagentcaps(ax->ax_ax, axs->axs_id, axr->axr_packetid, + AGENTX_CONTEXT_CTX(axc), &(axa->axa_oid)) == -1) { agentx_log_axc_warn(axc, "couldn't generate %s", ax_pdutype2string(AX_PDU_TYPE_REMOVEAGENTCAPS)); agentx_reset(ax); @@ -1005,8 +1013,8 @@ agentx_agentcaps_close(struct agentx_agentcaps *axa) } agentx_log_axc_info(axc, "agentcaps %s: closing", ax_oid2string(&(axa->axa_oid))); - return agentx_request(ax, packetid, - agentx_agentcaps_close_finalize, axa); + agentx_wantwrite(ax, ax->ax_fd); + return 0; } static int @@ -1181,7 +1189,7 @@ agentx_region_start(struct agentx_region *axr) struct agentx_context *axc = axr->axr_axc; struct agentx_session *axs = axc->axc_axs; struct agentx *ax = axs->axs_ax; - uint32_t packetid; + struct agentx_request *axreq; #ifdef AX_DEBUG if (axc->axc_cstate != AX_CSTATE_OPEN || @@ -1191,10 +1199,11 @@ agentx_region_start(struct agentx_region *axr) "%s: unexpected region registration", __func__); #endif - packetid = ax_register(ax->ax_ax, 0, axs->axs_id, + if ((axreq = agentx_request(ax, agentx_region_finalize, axr)) == NULL) + return -1; + if (ax_register(ax->ax_ax, 0, axs->axs_id, axreq->axr_packetid, AGENTX_CONTEXT_CTX(axc), axr->axr_timeout, axr->axr_priority, - 0, &(axr->axr_oid), 0); - if (packetid == 0) { + 0, &(axr->axr_oid), 0) == -1) { agentx_log_axc_warn(axc, "couldn't generate %s", ax_pdutype2string(AX_PDU_TYPE_REGISTER)); agentx_reset(ax); @@ -1203,7 +1212,8 @@ agentx_region_start(struct agentx_region *axr) agentx_log_axc_info(axc, "region %s: opening", ax_oid2string(&(axr->axr_oid))); axr->axr_cstate = AX_CSTATE_WAITOPEN; - return agentx_request(ax, packetid, agentx_region_finalize, axr); + agentx_wantwrite(ax, ax->ax_fd); + return 0; } static int @@ -1267,7 +1277,7 @@ agentx_region_close(struct agentx_region *axr) struct agentx_context *axc = axr->axr_axc; struct agentx_session *axs = axc->axc_axs; struct agentx *ax = axs->axs_ax; - uint32_t packetid; + struct agentx_request *axreq; #ifdef AX_DEBUG if (axr->axr_cstate != AX_CSTATE_OPEN) @@ -1279,10 +1289,12 @@ agentx_region_close(struct agentx_region *axr) if (axs->axs_cstate == AX_CSTATE_WAITCLOSE) return 0; - packetid = ax_unregister(ax->ax_ax, axs->axs_id, + axreq = agentx_request(ax, agentx_region_close_finalize, axr); + if (axreq == NULL) + return -1; + if (ax_unregister(ax->ax_ax, axs->axs_id, axreq->axr_packetid, AGENTX_CONTEXT_CTX(axc), axr->axr_priority, 0, &(axr->axr_oid), - 0); - if (packetid == 0) { + 0) == -1) { agentx_log_axc_warn(axc, "couldn't generate %s", ax_pdutype2string(AX_PDU_TYPE_UNREGISTER)); agentx_reset(ax); @@ -1290,8 +1302,8 @@ agentx_region_close(struct agentx_region *axr) } agentx_log_axc_info(axc, "region %s: closing", ax_oid2string(&(axr->axr_oid))); - return agentx_request(ax, packetid, agentx_region_close_finalize, - axr); + agentx_wantwrite(ax, ax->ax_fd); + return 0; } static int @@ -1683,7 +1695,7 @@ agentx_index_start(struct agentx_index *axi) struct agentx_context *axc = axr->axr_axc; struct agentx_session *axs = axc->axc_axs; struct agentx *ax = axs->axs_ax; - uint32_t packetid; + struct agentx_request *axreq; int flags = 0; #ifdef AX_DEBUG @@ -1705,10 +1717,11 @@ agentx_index_start(struct agentx_index *axi) return 0; } + if ((axreq = agentx_request(ax, agentx_index_finalize, axi)) == NULL) + return -1; /* We might be able to bundle, but if we fail we'd have to reorganise */ - packetid = ax_indexallocate(ax->ax_ax, flags, axs->axs_id, - AGENTX_CONTEXT_CTX(axc), &(axi->axi_vb), 1); - if (packetid == 0) { + if (ax_indexallocate(ax->ax_ax, flags, axs->axs_id, axreq->axr_packetid, + AGENTX_CONTEXT_CTX(axc), &(axi->axi_vb), 1) == -1) { agentx_log_axc_warn(axc, "couldn't generate %s", ax_pdutype2string(AX_PDU_TYPE_INDEXDEALLOCATE)); agentx_reset(ax); @@ -1724,8 +1737,8 @@ agentx_index_start(struct agentx_index *axi) else if (axi->axi_type == AXI_TYPE_NEW) agentx_log_axc_info(axc, "index %s: allocating new index", ax_oid2string(&(axi->axi_vb.avb_oid))); - - return agentx_request(ax, packetid, agentx_index_finalize, axi); + agentx_wantwrite(ax, ax->ax_fd); + return 0; } static int @@ -1883,7 +1896,7 @@ agentx_index_close(struct agentx_index *axi) struct agentx_context *axc = axr->axr_axc; struct agentx_session *axs = axc->axc_axs; struct agentx *ax = axs->axs_ax; - uint32_t packetid; + struct agentx_request *axreq; #ifdef AX_DEBUG if (axi->axi_cstate != AX_CSTATE_OPEN) @@ -1895,10 +1908,12 @@ agentx_index_close(struct agentx_index *axi) if (axs->axs_cstate == AX_CSTATE_WAITCLOSE) return 0; + axreq = agentx_request(ax, agentx_index_close_finalize, axi); + if (axreq == NULL) + return -1; /* We might be able to bundle, but if we fail we'd have to reorganise */ - packetid = ax_indexdeallocate(ax->ax_ax, axs->axs_id, - AGENTX_CONTEXT_CTX(axc), &(axi->axi_vb), 1); - if (packetid == 0) { + if (ax_indexdeallocate(ax->ax_ax, axs->axs_id, axreq->axr_packetid, + AGENTX_CONTEXT_CTX(axc), &(axi->axi_vb), 1) == -1) { agentx_log_axc_warn(axc, "couldn't generate %s", ax_pdutype2string(AX_PDU_TYPE_INDEXDEALLOCATE)); agentx_reset(ax); @@ -1906,8 +1921,8 @@ agentx_index_close(struct agentx_index *axi) } agentx_log_axc_info(axc, "index %s: deallocating", ax_oid2string(&(axi->axi_vb.avb_oid))); - return agentx_request(ax, packetid, agentx_index_close_finalize, - axi); + agentx_wantwrite(ax, ax->ax_fd); + return 0; } static int @@ -2156,7 +2171,7 @@ agentx_object_start(struct agentx_object *axo) char oids[1024]; size_t i; int needregister = 0; - uint32_t packetid; + struct agentx_request *axreq; uint8_t flags = AX_PDU_FLAG_INSTANCE_REGISTRATION; #ifdef AX_DEBUG @@ -2196,10 +2211,11 @@ agentx_object_start(struct agentx_object *axo) oid.aoi_id[oid.aoi_idlen++] = axo->axo_index[i]->axi_vb.avb_data.avb_int32; } - packetid = ax_register(ax->ax_ax, flags, axs->axs_id, + if ((axreq = agentx_request(ax, agentx_object_finalize, axo)) == NULL) + return -1; + if (ax_register(ax->ax_ax, flags, axs->axs_id, axreq->axr_packetid, AGENTX_CONTEXT_CTX(axc), axo->axo_timeout, - AX_PRIORITY_DEFAULT, 0, &oid, 0); - if (packetid == 0) { + AX_PRIORITY_DEFAULT, 0, &oid, 0) == -1) { agentx_log_axc_warn(axc, "couldn't generate %s", ax_pdutype2string(AX_PDU_TYPE_REGISTER)); agentx_reset(ax); @@ -2209,7 +2225,8 @@ agentx_object_start(struct agentx_object *axo) agentx_log_axc_info(axc, "object %s (%s %s): opening", oids, flags ? "instance" : "region", ax_oid2string(&(oid))); axo->axo_cstate = AX_CSTATE_WAITOPEN; - return agentx_request(ax, packetid, agentx_object_finalize, axo); + agentx_wantwrite(ax, ax->ax_fd); + return 0; } static int @@ -2312,7 +2329,7 @@ agentx_object_close(struct agentx_object *axo) char oids[1024]; size_t i; int needclose = 0; - uint32_t packetid; + struct agentx_request *axr; uint8_t flags = 1; #ifdef AX_DEBUG @@ -2353,9 +2370,11 @@ agentx_object_close(struct agentx_object *axo) oid.aoi_id[oid.aoi_idlen++] = axo->axo_index[i]->axi_vb.avb_data.avb_int32; } - packetid = ax_unregister(ax->ax_ax, axs->axs_id, - AGENTX_CONTEXT_CTX(axc), AX_PRIORITY_DEFAULT, 0, &oid, 0); - if (packetid == 0) { + axr = agentx_request(ax, agentx_object_close_finalize, axo); + if (axr == NULL) + return -1; + if (ax_unregister(ax->ax_ax, axs->axs_id, axr->axr_packetid, + AGENTX_CONTEXT_CTX(axc), AX_PRIORITY_DEFAULT, 0, &oid, 0) == -1) { agentx_log_axc_warn(axc, "couldn't generate %s", ax_pdutype2string(AX_PDU_TYPE_UNREGISTER)); agentx_reset(ax); @@ -2364,8 +2383,8 @@ agentx_object_close(struct agentx_object *axo) strlcpy(oids, ax_oid2string(&(axo->axo_oid)), sizeof(oids)); agentx_log_axc_info(axc, "object %s (%s %s): closing", oids, flags ? "instance" : "region", ax_oid2string(&(oid))); - return agentx_request(ax, packetid, agentx_object_close_finalize, - axo); + agentx_wantwrite(ax, ax->ax_fd); + return 0; } static int @@ -3906,40 +3925,25 @@ agentx_varbind_set_index_ipaddress(struct agentx_varbi #endif } -static int -agentx_request(struct agentx *ax, uint32_t packetid, - int (*cb)(struct ax_pdu *, void *), void *cookie) +static struct agentx_request * +agentx_request(struct agentx *ax, int (*cb)(struct ax_pdu *, void *), + void *cookie) { struct agentx_request *axr; -#ifdef AX_DEBUG - if (ax->ax_ax->ax_wblen == 0) - agentx_log_ax_fatalx(ax, "%s: no data to be written", - __func__); -#endif - if ((axr = calloc(1, sizeof(*axr))) == NULL) { agentx_log_ax_warn(ax, "couldn't create request context"); agentx_reset(ax); - return -1; + return NULL; } - axr->axr_packetid = packetid; axr->axr_cb = cb; axr->axr_cookie = cookie; - if (RB_INSERT(ax_requests, &(ax->ax_requests), axr) != NULL) { -#ifdef AX_DEBUG - agentx_log_ax_fatalx(ax, "%s: duplicate packetid", __func__); -#else - agentx_log_ax_warnx(ax, "%s: duplicate packetid", __func__); - free(axr); - agentx_reset(ax); - return -1; -#endif - } + do { + axr->axr_packetid = arc4random(); + } while (RB_INSERT(ax_requests, &(ax->ax_requests), axr) != NULL); - agentx_wantwrite(ax, ax->ax_fd); - return 0; + return axr; } static int @@ -4023,7 +4027,7 @@ agentx_read(struct agentx *ax) break; } if (axs == NULL) { - agentx_log_ax_warnx(ax, "received unexpected session: %d", + agentx_log_ax_warnx(ax, "received unexpected session: %u", pdu->ap_header.aph_sessionid); ax_pdu_free(pdu); agentx_reset(ax); blob - 4377a82d6803d1d36c11f438f9ffbbadb944b91f blob + 1fff36032df3331f50f005473f1cd450ff9c20a1 --- ax.c +++ ax.c @@ -37,8 +37,7 @@ static int ax_pdu_need(struct ax *, size_t); static int ax_pdu_header(struct ax *, enum ax_pdu_type, uint8_t, uint32_t, uint32_t, uint32_t, struct ax_ostring *); -static uint32_t ax_packetid(struct ax *); -static uint32_t ax_pdu_queue(struct ax *); +static int ax_pdu_queue(struct ax *); static int ax_pdu_add_uint16(struct ax *, uint16_t); static int ax_pdu_add_uint32(struct ax *, uint32_t); static int ax_pdu_add_uint64(struct ax *, uint64_t); @@ -89,7 +88,6 @@ ax_free(struct ax *ax) close(ax->ax_fd); free(ax->ax_rbuf); free(ax->ax_wbuf); - free(ax->ax_packetids); free(ax); } @@ -103,11 +101,10 @@ ax_recv(struct ax *ax) struct ax_pdu_searchrangelist *srl = NULL; struct ax_pdu_varbindlist *vbl; struct ax_searchrange *sr; - size_t rbsize, packetidx = 0, i, rawlen; + size_t rbsize, rawlen; ssize_t nread; uint8_t *u8; uint8_t *rbuf; - int found; /* Only read a single packet at a time to make sure libevent triggers */ if (ax->ax_rblen < AX_PDU_HEADER) { @@ -267,24 +264,6 @@ ax_recv(struct ax *ax) } break; case AX_PDU_TYPE_RESPONSE: - if (ax->ax_packetids != NULL) { - found = 0; - for (i = 0; ax->ax_packetids[i] != 0; i++) { - if (ax->ax_packetids[i] == - pdu->ap_header.aph_packetid) { - packetidx = i; - found = 1; - } - } - if (found) { - ax->ax_packetids[packetidx] = - ax->ax_packetids[i - 1]; - ax->ax_packetids[i - 1] = 0; - } else { - errno = EPROTO; - goto fail; - } - } if (rawlen < 8) { errno = EPROTO; goto fail; @@ -397,35 +376,35 @@ ax_send(struct ax *ax) return ax->ax_wblen; } -uint32_t -ax_open(struct ax *ax, uint8_t timeout, struct ax_oid *oid, +int +ax_open(struct ax *ax, uint8_t timeout, uint32_t packetid, struct ax_oid *oid, struct ax_ostring *descr) { - if (ax_pdu_header(ax, AX_PDU_TYPE_OPEN, 0, 0, 0, 0, + if (ax_pdu_header(ax, AX_PDU_TYPE_OPEN, 0, 0, 0, packetid, NULL) == -1) - return 0; + return -1; ax_pdu_need(ax, 4); ax->ax_wbuf[ax->ax_wbtlen++] = timeout; memset(&(ax->ax_wbuf[ax->ax_wbtlen]), 0, 3); ax->ax_wbtlen += 3; if (ax_pdu_add_oid(ax, oid, 0) == -1) - return 0; + return -1; if (ax_pdu_add_str(ax, descr) == -1) - return 0; + return -1; return ax_pdu_queue(ax); } -uint32_t -ax_close(struct ax *ax, uint32_t sessionid, +int +ax_close(struct ax *ax, uint32_t sessionid, uint32_t packetid, enum ax_close_reason reason) { - if (ax_pdu_header(ax, AX_PDU_TYPE_CLOSE, 0, sessionid, 0, 0, + if (ax_pdu_header(ax, AX_PDU_TYPE_CLOSE, 0, sessionid, 0, packetid, NULL) == -1) - return 0; + return -1; if (ax_pdu_need(ax, 4) == -1) - return 0; + return -1; ax->ax_wbuf[ax->ax_wbtlen++] = (uint8_t)reason; memset(&(ax->ax_wbuf[ax->ax_wbtlen]), 0, 3); ax->ax_wbtlen += 3; @@ -433,119 +412,120 @@ ax_close(struct ax *ax, uint32_t sessionid, return ax_pdu_queue(ax); } -uint32_t +int ax_indexallocate(struct ax *ax, uint8_t flags, uint32_t sessionid, - struct ax_ostring *context, struct ax_varbind *vblist, size_t nvb) + uint32_t packetid, struct ax_ostring *context, struct ax_varbind *vblist, + size_t nvb) { if (flags & ~(AX_PDU_FLAG_NEW_INDEX | AX_PDU_FLAG_ANY_INDEX)) { errno = EINVAL; - return 0; + return -1; } if (ax_pdu_header(ax, AX_PDU_TYPE_INDEXALLOCATE, flags, - sessionid, 0, 0, context) == -1) - return 0; + sessionid, 0, packetid, context) == -1) + return -1; if (ax_pdu_add_varbindlist(ax, vblist, nvb) == -1) - return 0; + return -1; return ax_pdu_queue(ax); } -uint32_t -ax_indexdeallocate(struct ax *ax, uint32_t sessionid, +int +ax_indexdeallocate(struct ax *ax, uint32_t sessionid, uint32_t packetid, struct ax_ostring *context, struct ax_varbind *vblist, size_t nvb) { if (ax_pdu_header(ax, AX_PDU_TYPE_INDEXDEALLOCATE, 0, - sessionid, 0, 0, context) == -1) - return 0; + sessionid, 0, packetid, context) == -1) + return -1; if (ax_pdu_add_varbindlist(ax, vblist, nvb) == -1) - return 0; + return -1; return ax_pdu_queue(ax); } -uint32_t -ax_addagentcaps(struct ax *ax, uint32_t sessionid, +int +ax_addagentcaps(struct ax *ax, uint32_t sessionid, uint32_t packetid, struct ax_ostring *context, struct ax_oid *id, struct ax_ostring *descr) { if (ax_pdu_header(ax, AX_PDU_TYPE_ADDAGENTCAPS, 0, - sessionid, 0, 0, context) == -1) - return 0; + sessionid, 0, packetid, context) == -1) + return -1; if (ax_pdu_add_oid(ax, id, 0) == -1) - return 0; + return -1; if (ax_pdu_add_str(ax, descr) == -1) - return 0; + return -1; return ax_pdu_queue(ax); } -uint32_t -ax_removeagentcaps(struct ax *ax, uint32_t sessionid, +int +ax_removeagentcaps(struct ax *ax, uint32_t sessionid, uint32_t packetid, struct ax_ostring *context, struct ax_oid *id) { if (ax_pdu_header(ax, AX_PDU_TYPE_REMOVEAGENTCAPS, 0, - sessionid, 0, 0, context) == -1) - return 0; + sessionid, 0, packetid, context) == -1) + return -1; if (ax_pdu_add_oid(ax, id, 0) == -1) - return 0; + return -1; return ax_pdu_queue(ax); } -uint32_t -ax_register(struct ax *ax, uint8_t flags, uint32_t sessionid, +int +ax_register(struct ax *ax, uint8_t flags, uint32_t sessionid, uint32_t packetid, struct ax_ostring *context, uint8_t timeout, uint8_t priority, uint8_t range_subid, struct ax_oid *subtree, uint32_t upperbound) { if (flags & ~(AX_PDU_FLAG_INSTANCE_REGISTRATION)) { errno = EINVAL; - return 0; + return -1; } if (ax_pdu_header(ax, AX_PDU_TYPE_REGISTER, flags, - sessionid, 0, 0, context) == -1) - return 0; + sessionid, 0, packetid, context) == -1) + return -1; if (ax_pdu_need(ax, 4) == -1) - return 0; + return -1; ax->ax_wbuf[ax->ax_wbtlen++] = timeout; ax->ax_wbuf[ax->ax_wbtlen++] = priority; ax->ax_wbuf[ax->ax_wbtlen++] = range_subid; ax->ax_wbuf[ax->ax_wbtlen++] = 0; if (ax_pdu_add_oid(ax, subtree, 0) == -1) - return 0; + return -1; if (range_subid != 0) { if (ax_pdu_add_uint32(ax, upperbound) == -1) - return 0; + return -1; } return ax_pdu_queue(ax); } -uint32_t -ax_unregister(struct ax *ax, uint32_t sessionid, +int +ax_unregister(struct ax *ax, uint32_t sessionid, uint32_t packetid, struct ax_ostring *context, uint8_t priority, uint8_t range_subid, struct ax_oid *subtree, uint32_t upperbound) { if (ax_pdu_header(ax, AX_PDU_TYPE_UNREGISTER, 0, - sessionid, 0, 0, context) == -1) - return 0; + sessionid, 0, packetid, context) == -1) + return -1; if (ax_pdu_need(ax, 4) == -1) - return 0; + return -1; ax->ax_wbuf[ax->ax_wbtlen++] = 0; ax->ax_wbuf[ax->ax_wbtlen++] = priority; ax->ax_wbuf[ax->ax_wbtlen++] = range_subid; ax->ax_wbuf[ax->ax_wbtlen++] = 0; if (ax_pdu_add_oid(ax, subtree, 0) == -1) - return 0; + return -1; if (range_subid != 0) { if (ax_pdu_add_uint32(ax, upperbound) == -1) - return 0; + return -1; } return ax_pdu_queue(ax); @@ -567,9 +547,7 @@ ax_response(struct ax *ax, uint32_t sessionid, uint32_ if (ax_pdu_add_varbindlist(ax, vblist, nvb) == -1) return -1; - if (ax_pdu_queue(ax) == 0) - return -1; - return 0; + return ax_pdu_queue(ax); } void @@ -954,23 +932,22 @@ ax_oid_add(struct ax_oid *oid, uint32_t value) return 0; } -static uint32_t +static int ax_pdu_queue(struct ax *ax) { struct ax_pdu_header header; - uint32_t packetid, plength; + uint32_t plength; size_t wbtlen = ax->ax_wbtlen; header.aph_flags = ax->ax_byteorder == AX_BYTE_ORDER_BE ? AX_PDU_FLAG_NETWORK_BYTE_ORDER : 0; - packetid = ax_pdutoh32(&header, &(ax->ax_wbuf[ax->ax_wblen + 12])); plength = (ax->ax_wbtlen - ax->ax_wblen) - AX_PDU_HEADER; ax->ax_wbtlen = ax->ax_wblen + 16; (void)ax_pdu_add_uint32(ax, plength); ax->ax_wblen = ax->ax_wbtlen = wbtlen; - return packetid; + return 0; } static int @@ -993,8 +970,6 @@ ax_pdu_header(struct ax *ax, enum ax_pdu_type type, ui flags |= AX_PDU_FLAG_NETWORK_BYTE_ORDER; ax->ax_wbuf[ax->ax_wbtlen++] = flags; ax->ax_wbuf[ax->ax_wbtlen++] = 0; - if (packetid == 0) - packetid = ax_packetid(ax); if (ax_pdu_add_uint32(ax, sessionid) == -1 || ax_pdu_add_uint32(ax, transactionid) == -1 || ax_pdu_add_uint32(ax, packetid) == -1 || @@ -1009,40 +984,6 @@ ax_pdu_header(struct ax *ax, enum ax_pdu_type type, ui return 0; } -static uint32_t -ax_packetid(struct ax *ax) -{ - uint32_t packetid, *packetids; - size_t npackets = 0, i; - int found; - - if (ax->ax_packetids != NULL) { - for (npackets = 0; ax->ax_packetids[npackets] != 0; npackets++) - continue; - } - if (ax->ax_packetidsize == 0 || npackets == ax->ax_packetidsize - 1) { - packetids = recallocarray(ax->ax_packetids, ax->ax_packetidsize, - ax->ax_packetidsize + 25, sizeof(*packetids)); - if (packetids == NULL) - return 0; - ax->ax_packetidsize += 25; - ax->ax_packetids = packetids; - } - do { - found = 0; - packetid = arc4random(); - for (i = 0; ax->ax_packetids[i] != 0; i++) { - if (ax->ax_packetids[i] == packetid) { - found = 1; - break; - } - } - } while (packetid == 0 || found); - ax->ax_packetids[npackets] = packetid; - - return packetid; -} - static int ax_pdu_add_uint16(struct ax *ax, uint16_t value) { blob - f4de378f238abece8162483a3f3c9e731666e23e blob + 0f9aeda495ab58dacca43e89b5c098fef806fb30 --- ax.h +++ ax.h @@ -122,8 +122,6 @@ struct ax { size_t ax_wblen; size_t ax_wbtlen; size_t ax_wbsize; - uint32_t *ax_packetids; - size_t ax_packetidsize; }; #ifndef AX_PRIMITIVE @@ -203,21 +201,21 @@ struct ax *ax_new(int); void ax_free(struct ax *); struct ax_pdu *ax_recv(struct ax *); ssize_t ax_send(struct ax *); -uint32_t ax_open(struct ax *, uint8_t, struct ax_oid *, +int ax_open(struct ax *, uint8_t, uint32_t, struct ax_oid *, struct ax_ostring *); -uint32_t ax_close(struct ax *, uint32_t, enum ax_close_reason); -uint32_t ax_indexallocate(struct ax *, uint8_t, uint32_t, +int ax_close(struct ax *, uint32_t, uint32_t, enum ax_close_reason); +int ax_indexallocate(struct ax *, uint8_t, uint32_t, uint32_t, struct ax_ostring *, struct ax_varbind *, size_t); -uint32_t ax_indexdeallocate(struct ax *, uint32_t, +int ax_indexdeallocate(struct ax *, uint32_t, uint32_t, struct ax_ostring *, struct ax_varbind *, size_t); -uint32_t ax_addagentcaps(struct ax *, uint32_t, struct ax_ostring *, +int ax_addagentcaps(struct ax *, uint32_t, uint32_t, struct ax_ostring *, struct ax_oid *, struct ax_ostring *); -uint32_t ax_removeagentcaps(struct ax *, uint32_t, +int ax_removeagentcaps(struct ax *, uint32_t, uint32_t, struct ax_ostring *, struct ax_oid *); -uint32_t ax_register(struct ax *, uint8_t, uint32_t, +int ax_register(struct ax *, uint8_t, uint32_t, uint32_t, struct ax_ostring *, uint8_t, uint8_t, uint8_t, struct ax_oid *, uint32_t); -uint32_t ax_unregister(struct ax *, uint32_t, struct ax_ostring *, +int ax_unregister(struct ax *, uint32_t, uint32_t, struct ax_ostring *, uint8_t, uint8_t, struct ax_oid *, uint32_t); int ax_response(struct ax *, uint32_t, uint32_t, uint32_t, uint32_t, uint16_t, uint16_t, struct ax_varbind *, size_t);