Re: snmpd: reject OIDS equal to searchrange.end

2023-10-28 Thread Martijn van Duren
On Sat, 2023-10-28 at 09:45 +0200, Martijn van Duren wrote:
> RFC 2741 section 5.2 states that searchrange.end is non-inclusive.
> appl_varbind_valid() and appl_response() currently tests inclusive.
> The appl_varbind_valid() case is for backends that support
> searchrange.end (like agentx) and the appl_response() case for
> those who do not and need a fixup (basically everything else).
> 
> OK?
> 
> martijn
> 
Here's the regress part I forgot to add.

Index: Makefile
===
RCS file: /cvs/src/regress/usr.sbin/snmpd/Makefile,v
retrieving revision 1.6
diff -u -p -r1.6 Makefile
--- Makefile27 Oct 2023 10:26:20 -  1.6
+++ Makefile28 Oct 2023 07:49:31 -
@@ -170,6 +170,7 @@ BACKEND_TARGETS+=   backend_getnext_stale
 BACKEND_TARGETS+=  backend_getnext_inclusive_backwards
 BACKEND_TARGETS+=  backend_getnext_toofew
 BACKEND_TARGETS+=  backend_getnext_toomany
+BACKEND_TARGETS+=  backend_getnext_response_equal_end
 BACKEND_TARGETS+=  backend_getbulk_nonrep_zero_maxrep_one
 BACKEND_TARGETS+=  backend_getbulk_nonrep_zero_maxrep_two
 BACKEND_TARGETS+=  backend_getbulk_nonrep_one_maxrep_one
Index: backend.c
===
RCS file: /cvs/src/regress/usr.sbin/snmpd/backend.c,v
retrieving revision 1.1
diff -u -p -r1.1 backend.c
--- backend.c   24 Oct 2023 14:34:40 -  1.1
+++ backend.c   28 Oct 2023 07:49:31 -
@@ -2633,6 +2633,62 @@ backend_getnext_toomany(void)
 }
 
 void
+backend_getnext_response_equal_end(void)
+{
+   struct sockaddr_storage ss;
+   struct sockaddr *sa = (struct sockaddr *)
+   socklen_t salen;
+   int snmp_s, ax_s;
+   uint32_t sessionid1, sessionid2;
+   struct varbind varbind[] = {
+   {
+   .type = TYPE_NULL,
+   .name = OID_STRUCT(MIB_BACKEND_GETNEXT, 26),
+   .data.int32 = 1
+   },
+   };
+   struct searchrange searchrange[] = {
+   {
+   .start = OID_STRUCT(MIB_BACKEND_GETNEXT, 26),
+   .end = OID_STRUCT(MIB_BACKEND_GETNEXT, 26, 1, 1)
+   },
+   };
+   int32_t requestid;
+   char buf[1024];
+   size_t n;
+
+   ax_s = agentx_connect(axsocket);
+   sessionid1 = agentx_open(ax_s, 0, 0,
+   OID_ARG(MIB_SUBAGENT_BACKEND_GETNEXT, 26, 1),
+   "backend_getnext_end_equal.1");
+   sessionid2 = agentx_open(ax_s, 0, 0,
+   OID_ARG(MIB_SUBAGENT_BACKEND_GETNEXT, 26, 2),
+   "backend_getnext_end_equal.2");
+   agentx_register(ax_s, sessionid1, 0, 0, 127, 0,
+   OID_ARG(MIB_BACKEND_GETNEXT, 26), 0);
+   agentx_register(ax_s, sessionid2, 0, 0, 127, 0,
+   OID_ARG(MIB_BACKEND_GETNEXT, 26, 1, 1), 0);
+
+   salen = snmp_resolve(SOCK_DGRAM, hostname, servname, sa);
+   snmp_s = snmp_connect(SOCK_DGRAM, sa, salen);
+   requestid = snmpv2_getnext(snmp_s, community, 0, varbind, 1);
+
+   /* Fool agentx_getnext_handle() */
+   varbind[0].name.subid[varbind[0].name.n_subid++] = 1;
+   varbind[0].type = TYPE_INTEGER;
+   n = agentx_read(ax_s, buf, sizeof(buf), 1000);
+   agentx_getnext_handle(__func__, buf, n, 0, sessionid1, searchrange,
+   varbind, 1);
+   varbind[0].name = searchrange[0].end;
+   agentx_response(ax_s, buf, NOERROR, 0, varbind, 1);
+   varbind[0].type = TYPE_NULL;
+   varbind[0].name = OID_STRUCT(MIB_BACKEND_GETNEXT, 26),
+
+   snmpv2_response_validate(snmp_s, 1000, community, requestid, GENERR, 1,
+   varbind, 1);
+}
+
+void
 backend_getbulk_nonrep_zero_maxrep_one(void)
 {
struct sockaddr_storage ss;
Index: regress.h
===
RCS file: /cvs/src/regress/usr.sbin/snmpd/regress.h,v
retrieving revision 1.2
diff -u -p -r1.2 regress.h
--- regress.h   27 Oct 2023 10:26:20 -  1.2
+++ regress.h   28 Oct 2023 07:49:31 -
@@ -293,6 +293,7 @@ void backend_getnext_stale(void);
 void backend_getnext_inclusive_backwards(void);
 void backend_getnext_toofew(void);
 void backend_getnext_toomany(void);
+void backend_getnext_response_equal_end(void);
 void backend_getbulk_nonrep_zero_maxrep_one(void);
 void backend_getbulk_nonrep_zero_maxrep_two(void);
 void backend_getbulk_nonrep_one_maxrep_one(void);
Index: snmpd_regress.c
===
RCS file: /cvs/src/regress/usr.sbin/snmpd/snmpd_regress.c,v
retrieving revision 1.2
diff -u -p -r1.2 snmpd_regress.c
--- snmpd_regress.c 27 Oct 2023 10:26:20 -  1.2
+++ snmpd_regress.c 28 Oct 2023 07:49:31 -
@@ -143,6 +143,7 @@ const struct {
{ "backend_getnext_inclusive_backwards", 
backend_getnext_inclusive_backwards },
{ "backend_getnext_toofew", 

Re: snmpd: reject OIDS equal to searchrange.end

2023-10-28 Thread Theo Buehler
On Sat, Oct 28, 2023 at 09:45:25AM +0200, Martijn van Duren wrote:
> RFC 2741 section 5.2 states that searchrange.end is non-inclusive.
> appl_varbind_valid() and appl_response() currently tests inclusive.
> The appl_varbind_valid() case is for backends that support
> searchrange.end (like agentx) and the appl_response() case for
> those who do not and need a fixup (basically everything else).
> 
> OK?

ok tb

> 
> martijn
> 
> diff --git a/application.c b/application.c
> index 33143d6..b3a2603 100644
> --- a/application.c
> +++ b/application.c
> @@ -1100,7 +1100,7 @@ appl_response(struct appl_backend *backend, int32_t 
> requestid,
>*/
>   eomv |= !backend->ab_range && next &&
>   ober_oid_cmp(&(vb->av_oid),
> - &(origvb->avi_varbind.av_oid_end)) > 0;
> + &(origvb->avi_varbind.av_oid_end)) >= 0;
>   /* RFC 3584 section 4.2.2.1 */
>   if (ureq->aru_pduversion == SNMP_V1 &&
>   vb->av_value != NULL &&
> @@ -1283,7 +1283,7 @@ appl_varbind_valid(struct appl_varbind *varbind,
>   }
>   }
>   if (range && ober_oid_cmp(&(varbind->av_oid),
> - &(request->avi_varbind.av_oid_end)) > 0) {
> + &(request->avi_varbind.av_oid_end)) >= 0) {
>   *errstr = "end oid not honoured";
>   return 0;
>   }
> 



snmpd: reject OIDS equal to searchrange.end

2023-10-28 Thread Martijn van Duren
RFC 2741 section 5.2 states that searchrange.end is non-inclusive.
appl_varbind_valid() and appl_response() currently tests inclusive.
The appl_varbind_valid() case is for backends that support
searchrange.end (like agentx) and the appl_response() case for
those who do not and need a fixup (basically everything else).

OK?

martijn

diff --git a/application.c b/application.c
index 33143d6..b3a2603 100644
--- a/application.c
+++ b/application.c
@@ -1100,7 +1100,7 @@ appl_response(struct appl_backend *backend, int32_t 
requestid,
 */
eomv |= !backend->ab_range && next &&
ober_oid_cmp(&(vb->av_oid),
-   &(origvb->avi_varbind.av_oid_end)) > 0;
+   &(origvb->avi_varbind.av_oid_end)) >= 0;
/* RFC 3584 section 4.2.2.1 */
if (ureq->aru_pduversion == SNMP_V1 &&
vb->av_value != NULL &&
@@ -1283,7 +1283,7 @@ appl_varbind_valid(struct appl_varbind *varbind,
}
}
if (range && ober_oid_cmp(&(varbind->av_oid),
-   &(request->avi_varbind.av_oid_end)) > 0) {
+   &(request->avi_varbind.av_oid_end)) >= 0) {
*errstr = "end oid not honoured";
return 0;
}