From 83663d4d15cc201095ae645510b1c20bab583329 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Sun, 9 Oct 2022 17:02:08 +0200 Subject: [PATCH] Fixed: [Trying to change the "read-only" node through snmpset](https://github.com/clicon/clixon/issues/376) --- CHANGELOG.md | 1 + apps/snmp/snmp_handler.c | 40 ++++++++++++++++++++++++++++++++-------- test/test_snmp_set.sh | 3 ++- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 03547b42..5e2d57bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,7 @@ Developers may need to change their code ### Corrected Bugs +* Fixed: [Trying to change the "read-only" node through snmpset](https://github.com/clicon/clixon/issues/376) * Fixed: [Trying to change the "config false" node through snmpset](https://github.com/clicon/clixon/issues/377) * Fixed by returning `SNMP_ERR_NOTWRITABLE` when trying to reserve object * Fixed: [Non-obvious behavior of clixon_snmp after snmpset command when transaction validation returns an error](https://github.com/clicon/clixon/issues/375) diff --git a/apps/snmp/snmp_handler.c b/apps/snmp/snmp_handler.c index ddc86914..216bb753 100644 --- a/apps/snmp/snmp_handler.c +++ b/apps/snmp/snmp_handler.c @@ -731,7 +731,8 @@ clixon_snmp_scalar_handler1(netsnmp_mib_handler *handler, case MODE_GETNEXT: /* 161 */ break; case MODE_SET_RESERVE1: /* 0 */ - if (!yang_config_ancestor(sh->sh_ys)){ + if (!yang_config_ancestor(sh->sh_ys) || + nhreg->modes == HANDLER_CAN_RONLY){ retval = SNMP_ERR_NOTWRITABLE; goto done;; } @@ -757,7 +758,7 @@ clixon_snmp_scalar_handler1(netsnmp_mib_handler *handler, goto done; if (ret == 0){ clicon_rpc_discard_changes(sh->sh_h); - retval = SNMP_ERR_COMMITFAILED; + netsnmp_request_set_error(request, SNMP_ERR_NOTWRITABLE); goto done; } break; @@ -929,9 +930,10 @@ snmp_table_get(clicon_handle h, * @param[in] oids OID of ultimate scalar value * @param[in] oidslen OID length of scalar * @param[in] reqinfo Agent transaction request structure - * @param[in] request The netsnmp request info structure. + * @param[in] request The netsnmp request info structure. + * @param[out] err Error code if failed (retval = 0) * @retval -1 Error - * @retval 0 Object not found + * @retval 0 Failed, err set * @retval 1 OK */ static int @@ -940,7 +942,8 @@ snmp_table_set(clicon_handle h, oid *oids, size_t oidslen, netsnmp_agent_request_info *reqinfo, - netsnmp_request_info *request) + netsnmp_request_info *request, + int *err) { int retval = -1; oid oidt[MAX_OID_LEN] = {0,}; /* Table / list oid */ @@ -1003,8 +1006,27 @@ snmp_table_set(clicon_handle h, } if (ys == NULL){ /* No leaf with matching OID */ + *err = SNMP_NOSUCHOBJECT; goto fail; } + if (!yang_config_ancestor(ys)){ + *err = SNMP_ERR_NOTWRITABLE; + goto fail; + } + { + char *modes_str = NULL; + int modes; + + if (yang_extension_value(ys, "max-access", IETF_YANG_SMIV2_NS, NULL, &modes_str) < 0) + goto done; + if (modes_str){ + modes = snmp_access_str2int(modes_str); + if (modes == HANDLER_CAN_RONLY){ + *err = SNMP_ERR_NOTWRITABLE; + goto fail; + } + } + } if (type_yang2asn1(ys, &asn1_type, 0) < 0) goto done; requestvb = request->requestvb; @@ -1041,6 +1063,7 @@ snmp_table_set(clicon_handle h, } if (oidilen != 0){ clicon_err(OE_YANG, 0, "Expected oidlen 0 but is %zu", oidilen); + *err = SNMP_NOSUCHOBJECT; goto fail; } if (yrowst){ @@ -1250,6 +1273,7 @@ clixon_snmp_table_handler1(netsnmp_mib_handler *handler, cbuf *cb = NULL; int ret; netsnmp_variable_list *requestvb; + int err = 0; clicon_debug(2, "%s", __FUNCTION__); if ((ret = snmp_common_handler(handler, nhreg, reqinfo, request, 1, &sh)) < 0) @@ -1304,10 +1328,10 @@ clixon_snmp_table_handler1(netsnmp_mib_handler *handler, case MODE_SET_ACTION: // 2 if ((ret = snmp_table_set(sh->sh_h, sh->sh_ys, requestvb->name, requestvb->name_length, - reqinfo, request)) < 0) + reqinfo, request, &err)) < 0) goto done; if (ret == 0){ - if ((ret = netsnmp_request_set_error(request, SNMP_NOSUCHINSTANCE)) != SNMPERR_SUCCESS){ + if ((ret = netsnmp_request_set_error(request, err)) != SNMPERR_SUCCESS){ clicon_err(OE_SNMP, ret, "netsnmp_request_set_error"); goto done; } @@ -1319,7 +1343,7 @@ clixon_snmp_table_handler1(netsnmp_mib_handler *handler, goto done; if (ret == 0){ clicon_rpc_discard_changes(sh->sh_h); - retval = SNMP_ERR_COMMITFAILED; + netsnmp_request_set_error(request, SNMP_ERR_COMMITFAILED); goto done; } break; diff --git a/test/test_snmp_set.sh b/test/test_snmp_set.sh index 54a13152..9907941d 100755 --- a/test/test_snmp_set.sh +++ b/test/test_snmp_set.sh @@ -239,7 +239,8 @@ testrun clixonExampleSleeper INTEGER -1 -1 -1 ${MIB}.1.2 testrun clixonExampleString STRING foobar foobar foobar ${MIB}.1.3 testrun ifPromiscuousMode INTEGER 1 1 true ${MIB}.1.10 # boolean testrun ifIpAddr IPADDRESS 1.2.3.4 1.2.3.4 1.2.3.4 ${MIB}.1.13 # InetAddress -testrun ifPhysAddress STRING ff:ee:dd:cc:bb:aa ff:ee:dd:cc:bb:aa ff:ee:dd:cc:bb:aa ${IFMIB}.2.2.1.6.1 +# XXX It was supposed to test writing hardware address type, but it is also read-only +#testrun ifPhysAddress STRING ff:ee:dd:cc:bb:aa ff:ee:dd:cc:bb:aa ff:ee:dd:cc:bb:aa ${IFMIB}.2.2.1.6.1 # Inline testrun for rowstatus complicated logic name=ifStackStatus