Re: relayd.conf.5: less SSL

2023-10-28 Thread Sebastian Benoit
Klemens Nanni(k...@openbsd.org) on 2023.10.26 13:28:42 +:
> On Tue, Oct 24, 2023 at 09:09:21AM +0200, Peter N. M. Hansteen wrote:
> > On Tue, Oct 24, 2023 at 06:54:30AM +, Klemens Nanni wrote:
> > > - parse.y still accepting undocumented "ssl" with a warning since 2014
> > > - more "SSL/TLS" instead of "TLS" in manual and code comments
> > 
> > my take would be that while it's fine to streamline the documentation to use
> > the modern terminology, I suspect there may still be ancient configurations
> > out there that use the "ssl" keyword, so removing the last bit of support 
> > for
> > that option should be accompanied by or preceded by a warning on relevant
> > mailing lists or at least in the commit message. 
> > 
> > And I think undeadly.org would be more than happy to help spread the word :)
> 
> current.html entry should do for a deprecated keyword we've been warning
> about for almost ten years...

Yes, please kick it where it belongs.

> I've checked faq/upgrade*.html for previous
> notes, but couldn't find any.

no, because it wasnt removed after 2 releases with the warning.

> Here's a first try, relayd regress is also happy.

ok benno@

> Index: usr.sbin/relayd/parse.y
> ===
> RCS file: /cvs/src/usr.sbin/relayd/parse.y,v
> retrieving revision 1.254
> diff -u -p -r1.254 parse.y
> --- usr.sbin/relayd/parse.y   3 Jul 2023 09:38:08 -   1.254
> +++ usr.sbin/relayd/parse.y   26 Oct 2023 06:07:08 -
> @@ -175,7 +175,7 @@ typedef struct {
>  %token   LOOKUP METHOD MODE NAT NO DESTINATION NODELAY NOTHING ON PARENT 
> PATH
>  %token   PFTAG PORT PREFORK PRIORITY PROTO QUERYSTR REAL REDIRECT RELAY 
> REMOVE
>  %token   REQUEST RESPONSE RETRY QUICK RETURN ROUNDROBIN ROUTE SACK 
> SCRIPT SEND
> -%token   SESSION SOCKET SPLICE SSL STICKYADDR STRIP STYLE TABLE TAG 
> TAGGED TCP
> +%token   SESSION SOCKET SPLICE STICKYADDR STRIP STYLE TABLE TAG TAGGED 
> TCP
>  %token   TIMEOUT TLS TO ROUTER RTLABEL TRANSPARENT URL WITH TTL RTABLE
>  %token   MATCH PARAMS RANDOM LEASTSTATES SRCHASH KEY CERTIFICATE 
> PASSWORD ECDHE
>  %token   EDH TICKETS CONNECTION CONNECTIONS CONTEXT ERRORS STATE CHANGES 
> CHECKS
> @@ -227,21 +227,12 @@ include : INCLUDE STRING{
>   }
>   ;
>  
> -ssltls   : SSL   {
> - log_warnx("%s:%d: %s",
> - file->name, yylval.lineno,
> - "please use the \"tls\" keyword"
> - " instead of \"ssl\"");
> - }
> - | TLS
> - ;
> -
>  opttls   : /*empty*/ { $$ = 0; }
> - | ssltls{ $$ = 1; }
> + | TLS   { $$ = 1; }
>   ;
>  
>  opttlsclient : /*empty*/ { $$ = 0; }
> - | WITH ssltls   { $$ = 1; }
> + | WITH TLS  { $$ = 1; }
>   ;
>  
>  http_type: HTTP  { $$ = 0; }
> @@ -905,7 +896,7 @@ hashkey   : /* empty */   {
>  
>  tablecheck   : ICMP  { table->conf.check = CHECK_ICMP; }
>   | TCP   { table->conf.check = CHECK_TCP; }
> - | ssltls{
> + | TLS   {
>   table->conf.check = CHECK_TCP;
>   conf->sc_conf.flags |= F_TLS;
>   table->conf.flags |= F_TLS;
> @@ -1114,7 +1105,7 @@ protopts_l  : protopts_l protoptsl nl
>   | protoptsl optnl
>   ;
>  
> -protoptsl: ssltls {
> +protoptsl: TLS {
>   if (!(proto->type == RELAY_PROTO_TCP ||
>   proto->type == RELAY_PROTO_HTTP)) {
>   yyerror("can set tls options only for "
> @@ -1122,7 +1113,7 @@ protoptsl   : ssltls {
>   YYERROR;
>   }
>   } tlsflags
> - | ssltls {
> + | TLS {
>   if (!(proto->type == RELAY_PROTO_TCP ||
>   proto->type == RELAY_PROTO_HTTP)) {
>   yyerror("can set tls options only for "
> @@ -2492,7 +2483,6 @@ lookup(char *s)
>   { "socket", SOCKET },
>   { "source-hash",SRCHASH },
>   { "splice", SPLICE },
> - { "ssl",SSL },
>   { "state",  STATE },
>   { "sticky-address", STICKYADDR },
>   { "strip",  STRIP },
> Index: usr.sbin/relayd/relay.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
> retrieving revision 1.257
> diff -u -p -r1.257 relay.c
> --- usr.sbin/relayd/relay.c   3 Sep 2023 10:22:03 -   1.257
> +++ usr.sbin/relayd/relay.c   26 Oct 2023 05:49:22 

Re: snmpd: don't move back in oid tree when on/below instance below region

2023-10-28 Thread Theo Buehler
On Sat, Oct 28, 2023 at 09:59:25AM +0200, Martijn van Duren wrote:
> Right now when we register a region with below that an instance we
> can revert back in the tree. When we request below the instance we
> currently use appl_region_next() to find the next region and simply
> set the to be find oid to the the oid of the new region. In the
> situation described above this means that we return the parent
> region and set the oid to that of the parent region, which is smaller
> than the instance oid.
> 
> The easiest fix is to increment the searched oid to nextsibling of the
> instance and let the normal appl_varbind_backend() logic handle it from
> there.

ok tb



snmpd: don't move back in oid tree when on/below instance below region

2023-10-28 Thread Martijn van Duren
Right now when we register a region with below that an instance we
can revert back in the tree. When we request below the instance we
currently use appl_region_next() to find the next region and simply
set the to be find oid to the the oid of the new region. In the
situation described above this means that we return the parent
region and set the oid to that of the parent region, which is smaller
than the instance oid.

The easiest fix is to increment the searched oid to nextsibling of the
instance and let the normal appl_varbind_backend() logic handle it from
there.

OK?

martijn@

Index: usr.sbin/snmpd/application.c
===
RCS file: /cvs/src/usr.sbin/snmpd/application.c,v
retrieving revision 1.25
diff -u -p -r1.25 application.c
--- usr.sbin/snmpd/application.c27 Oct 2023 10:32:11 -  1.25
+++ usr.sbin/snmpd/application.c28 Oct 2023 07:57:23 -
@@ -1368,19 +1368,17 @@ appl_varbind_backend(struct appl_varbind
return -1;
return 0;
}
-   if ((region = appl_region_next(ureq->aru_ctx,
-   &(vb->av_oid), region)) == NULL)
-   goto eomv;
vb->av_oid = region->ar_oid;
+   ober_oid_nextsibling(&(vb->av_oid));
vb->av_include = 1;
+   return appl_varbind_backend(ivb);
}
} else if (cmp == 0) {
if (region->ar_instance && next && !vb->av_include) {
-   if ((region = appl_region_next(ureq->aru_ctx,
-   &(vb->av_oid), region)) == NULL)
-   goto eomv;
vb->av_oid = region->ar_oid;
+   ober_oid_nextsibling(&(vb->av_oid));
vb->av_include = 1;
+   return appl_varbind_backend(ivb);
}
}
ivb->avi_region = region;
Index: regress/usr.sbin/snmpd/Makefile
===
RCS file: /cvs/src/regress/usr.sbin/snmpd/Makefile,v
retrieving revision 1.6
diff -u -p -r1.6 Makefile
--- regress/usr.sbin/snmpd/Makefile 27 Oct 2023 10:26:20 -  1.6
+++ regress/usr.sbin/snmpd/Makefile 28 Oct 2023 07:57:23 -
@@ -170,6 +170,9 @@ 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_instance_below_region_before_instance
+BACKEND_TARGETS+=  
backend_getnext_instance_below_region_on_instance
+BACKEND_TARGETS+=  
backend_getnext_instance_below_region_below_instance
 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: regress/usr.sbin/snmpd/backend.c
===
RCS file: /cvs/src/regress/usr.sbin/snmpd/backend.c,v
retrieving revision 1.1
diff -u -p -r1.1 backend.c
--- regress/usr.sbin/snmpd/backend.c24 Oct 2023 14:34:40 -  1.1
+++ regress/usr.sbin/snmpd/backend.c28 Oct 2023 07:57:23 -
@@ -2633,6 +2633,173 @@ backend_getnext_toomany(void)
 }
 
 void
+backend_getnext_instance_below_region_before_instance(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, 27),
+   .data.int32 = 1
+   },
+   };
+   struct searchrange searchrange[] = {
+   {
+   .start = OID_STRUCT(MIB_BACKEND_GETNEXT, 27),
+   .end = OID_STRUCT(MIB_BACKEND_GETNEXT, 27, 1, 0)
+   },
+   };
+   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, 27, 1),
+   "backend_getnext_instance_below_region_before_instance.1");
+   sessionid2 = agentx_open(ax_s, 0, 0,
+   OID_ARG(MIB_SUBAGENT_BACKEND_GETNEXT, 27, 2),
+   "backend_getnext_instance_below_region_before_instance.2");
+   agentx_register(ax_s, sessionid1, 0, 0, 127, 0,
+   OID_ARG(MIB_BACKEND_GETNEXT, 27), 0);
+   agentx_register(ax_s, sessionid2, 1, 0, 127, 0,
+   OID_ARG(MIB_BACKEND_GETNEXT, 27, 1, 0), 0);
+
+   salen = 

Re: libagentx: don't return responses >= searchrange.end

2023-10-28 Thread Theo Buehler
On Sat, Oct 28, 2023 at 09:41:55AM +0200, Martijn van Duren wrote:
> In most cases when a region is registered we have the full ownership.
> As soon as a region has been registered below prior mentioned region we
> could loose ownership halfway through. This case currently isn't fully
> tested and with indices we can return OIDs >= searchrange.end.
> The easiest way is to test this case is in agentx_varbind_finalize()
> and simply reset a varbind to EOMV if we're outside our range.

Makes sense.

> I've pulled apart the agentx_request_type cases for clarity and control.

Yes, that's more readable.

ok tb

> 
> OK?
> 
> martijn@
> 
> Index: agentx.c
> ===
> RCS file: /cvs/src/lib/libagentx/agentx.c,v
> retrieving revision 1.23
> diff -u -p -r1.23 agentx.c
> --- agentx.c  24 Oct 2023 08:54:52 -  1.23
> +++ agentx.c  28 Oct 2023 07:40:59 -
> @@ -2822,7 +2822,7 @@ getnext:
>   while (axo != NULL && axo->axo_cstate != AX_CSTATE_OPEN)
>   axo = RB_NEXT(axc_objects, &(axc->axc_objects), axo);
>   if (axo == NULL ||
> - ax_oid_cmp(&(axo->axo_oid), &(axv->axv_end)) > 0) {
> + ax_oid_cmp(&(axo->axo_oid), &(axv->axv_end)) >= 0) {
>   agentx_varbind_endofmibview(axv);
>   return;
>   }
> @@ -3349,19 +3349,53 @@ agentx_varbind_finalize(struct agentx_va
>  #endif
>   }
>   }
> - cmp = ax_oid_cmp(&(axv->axv_vb.avb_oid), );
> - if ((agentx_varbind_request(axv) == AGENTX_REQUEST_TYPE_GETNEXT &&
> - cmp >= 0) || cmp > 0) {
> + cmp = ax_oid_cmp(, &(axv->axv_vb.avb_oid));
> + switch (agentx_varbind_request(axv)) {
> + case AGENTX_REQUEST_TYPE_GET:
> + if (cmp != 0) {
>  #ifdef AX_DEBUG
> - agentx_log_axg_fatalx(axg, "indices not incremented");
> + agentx_log_axg_fatalx(axg, "index changed");
>  #else
> - agentx_log_axg_warnx(axg, "indices not incremented");
> - bcopy(&(axv->axv_start), &(axv->axv_vb.avb_oid),
> - sizeof(axv->axv_start));
> - axv->axv_error = AX_PDU_ERROR_GENERR;
> + agentx_log_axg_warnx(axg, "index changed");
> + bcopy(&(axv->axv_start), &(axv->axv_vb.avb_oid),
> + sizeof(axv->axv_start));
> + axv->axv_error = AX_PDU_ERROR_GENERR;
> + break;
>  #endif
> - } else
> + }
> + break;
> + case AGENTX_REQUEST_TYPE_GETNEXT:
> + if (cmp <= 0) {
> +#ifdef AX_DEBUG
> + agentx_log_axg_fatalx(axg, "indices not incremented");
> +#else
> + agentx_log_axg_warnx(axg, "indices not incremented");
> + bcopy(&(axv->axv_start), &(axv->axv_vb.avb_oid),
> + sizeof(axv->axv_start));
> + axv->axv_error = AX_PDU_ERROR_GENERR;
> + break;
> +#endif
> + }
> + /* FALLTHROUGH */
> + case AGENTX_REQUEST_TYPE_GETNEXTINCLUSIVE:
> + if (cmp < 0) {
> +#ifdef AX_DEBUG
> + agentx_log_axg_fatalx(axg, "index decremented");
> +#else
> + agentx_log_axg_warnx(axg, "index decremented");
> + bcopy(&(axv->axv_start), &(axv->axv_vb.avb_oid),
> + sizeof(axv->axv_start));
> + axv->axv_error = AX_PDU_ERROR_GENERR;
> + break;
> +#endif
> + }
> + if (axv->axv_end.aoi_idlen != 0 &&
> + ax_oid_cmp(, &(axv->axv_end)) >= 0) {
> + agentx_varbind_endofmibview(axv);
> + return;
> + }
>   bcopy(, &(axv->axv_vb.avb_oid), sizeof(oid));
> + }
>  done:
>   agentx_object_unlock(axv->axv_axo);
>   agentx_get_finalize(axv->axv_axg);
> 



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;
}



libagentx: don't return responses >= searchrange.end

2023-10-28 Thread Martijn van Duren
In most cases when a region is registered we have the full ownership.
As soon as a region has been registered below prior mentioned region we
could loose ownership halfway through. This case currently isn't fully
tested and with indices we can return OIDs >= searchrange.end.
The easiest way is to test this case is in agentx_varbind_finalize()
and simply reset a varbind to EOMV if we're outside our range.

I've pulled apart the agentx_request_type cases for clarity and control.

OK?

martijn@

Index: agentx.c
===
RCS file: /cvs/src/lib/libagentx/agentx.c,v
retrieving revision 1.23
diff -u -p -r1.23 agentx.c
--- agentx.c24 Oct 2023 08:54:52 -  1.23
+++ agentx.c28 Oct 2023 07:40:59 -
@@ -2822,7 +2822,7 @@ getnext:
while (axo != NULL && axo->axo_cstate != AX_CSTATE_OPEN)
axo = RB_NEXT(axc_objects, &(axc->axc_objects), axo);
if (axo == NULL ||
-   ax_oid_cmp(&(axo->axo_oid), &(axv->axv_end)) > 0) {
+   ax_oid_cmp(&(axo->axo_oid), &(axv->axv_end)) >= 0) {
agentx_varbind_endofmibview(axv);
return;
}
@@ -3349,19 +3349,53 @@ agentx_varbind_finalize(struct agentx_va
 #endif
}
}
-   cmp = ax_oid_cmp(&(axv->axv_vb.avb_oid), );
-   if ((agentx_varbind_request(axv) == AGENTX_REQUEST_TYPE_GETNEXT &&
-   cmp >= 0) || cmp > 0) {
+   cmp = ax_oid_cmp(, &(axv->axv_vb.avb_oid));
+   switch (agentx_varbind_request(axv)) {
+   case AGENTX_REQUEST_TYPE_GET:
+   if (cmp != 0) {
 #ifdef AX_DEBUG
-   agentx_log_axg_fatalx(axg, "indices not incremented");
+   agentx_log_axg_fatalx(axg, "index changed");
 #else
-   agentx_log_axg_warnx(axg, "indices not incremented");
-   bcopy(&(axv->axv_start), &(axv->axv_vb.avb_oid),
-   sizeof(axv->axv_start));
-   axv->axv_error = AX_PDU_ERROR_GENERR;
+   agentx_log_axg_warnx(axg, "index changed");
+   bcopy(&(axv->axv_start), &(axv->axv_vb.avb_oid),
+   sizeof(axv->axv_start));
+   axv->axv_error = AX_PDU_ERROR_GENERR;
+   break;
 #endif
-   } else
+   }
+   break;
+   case AGENTX_REQUEST_TYPE_GETNEXT:
+   if (cmp <= 0) {
+#ifdef AX_DEBUG
+   agentx_log_axg_fatalx(axg, "indices not incremented");
+#else
+   agentx_log_axg_warnx(axg, "indices not incremented");
+   bcopy(&(axv->axv_start), &(axv->axv_vb.avb_oid),
+   sizeof(axv->axv_start));
+   axv->axv_error = AX_PDU_ERROR_GENERR;
+   break;
+#endif
+   }
+   /* FALLTHROUGH */
+   case AGENTX_REQUEST_TYPE_GETNEXTINCLUSIVE:
+   if (cmp < 0) {
+#ifdef AX_DEBUG
+   agentx_log_axg_fatalx(axg, "index decremented");
+#else
+   agentx_log_axg_warnx(axg, "index decremented");
+   bcopy(&(axv->axv_start), &(axv->axv_vb.avb_oid),
+   sizeof(axv->axv_start));
+   axv->axv_error = AX_PDU_ERROR_GENERR;
+   break;
+#endif
+   }
+   if (axv->axv_end.aoi_idlen != 0 &&
+   ax_oid_cmp(, &(axv->axv_end)) >= 0) {
+   agentx_varbind_endofmibview(axv);
+   return;
+   }
bcopy(, &(axv->axv_vb.avb_oid), sizeof(oid));
+   }
 done:
agentx_object_unlock(axv->axv_axo);
agentx_get_finalize(axv->axv_axg);