Re: snmpd_metrics: differentiate between hrSWRunName and hrSWRunPath
On Wed, Oct 18, 2023 at 01:49:00PM +0200, Martijn van Duren wrote: > Right now we return the same value for both hrSWRunName and hrSWRunPath. > hrSWRunPath should return the full path of the binary, and hrSWRunName > a description of the running software. > > Afaik there's no proper way to retrieve the full path of the running > binary. No there isn't. It's been discussed many times. > However, in a lot of cases argv[0] can contain the full or > relative path. But if argv[0] gets overwritten (like most of our > daemons' children) it gives a more descriptive name, which is more in > line with hrSWRunName. > > netsnmp's snmpd uses argv[0] for hrSWRunPath and kinfo_proc's p_comm for > hrSWRunName, and snmptop defaults to hrSWRunName. top(1) also defaults > to p_comm, but contrary to top(1), snmptop allows us to switch between > hrSWRunName, and hrSWRunPath and toggling of hrSWRunParameters > independently, where top(1) toggles between p_comm and argv[]. > > So there's an argument to be made either way, but for this diff I stuck > with netsnmp's choices. I am fine with your choices and people feeling strongly about this had week to object. > > While here, change the buffer length from 128 to 129. HOST-RESOURCES-MIB > allows up to 128 characters in the response, so make room for the > terminating NUL. > > Thoughts? OK? ok tb
Re: snmpd_metrics: add HOST-RESOURCES-MIB:hrSWRunPerfTable
On Wed, Oct 18, 2023 at 11:54:06AM +0200, Martijn van Duren wrote:
> This diff adds the two entries from the hrSWRunPerfTable:
> hrSWRunPerfCPU, and hrSWRunPerfMem. This allows snmptop from the
> net-snmp package to work. Math stolen^Wcopied from top/machine.c.
>
> OK?
ok
>
> martijn@
>
> Index: mib.c
> ===
> RCS file: /cvs/src/libexec/snmpd/snmpd_metrics/mib.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 mib.c
> --- mib.c 4 Jul 2023 11:34:19 - 1.4
> +++ mib.c 18 Oct 2023 09:52:07 -
> @@ -69,6 +69,10 @@ struct eventconnev;
> const char *agentxsocket = NULL;
> int agentxfd = -1;
>
> +int pageshift;
> +#define pagetok(size) ((size) << pageshift)
> +
> +void pageshift_init(void);
> void snmp_connect(struct agentx *, void *, int);
> void snmp_tryconnect(int, short, void *);
> void snmp_read(int, short, void *);
> @@ -89,6 +93,7 @@ struct agentx_object *hrProcessorLoad;
> struct agentx_index *hrSWRunIdx;
> struct agentx_object *hrSWRunIndex, *hrSWRunName, *hrSWRunID, *hrSWRunPath;
> struct agentx_object *hrSWRunParameters, *hrSWRunType, *hrSWRunStatus;
> +struct agentx_object *hrSWRunPerfCPU, *hrSWRunPerfMem;
>
> void mib_hrsystemuptime(struct agentx_varbind *);
> void mib_hrsystemdate(struct agentx_varbind *);
> @@ -634,6 +639,7 @@ mib_hrswrun(struct agentx_varbind *vb)
> struct agentx_object*obj;
> enum agentx_request_type req;
> int32_t idx;
> + int32_t time;
> struct kinfo_proc *kinfo;
> char*s;
>
> @@ -711,6 +717,13 @@ mib_hrswrun(struct agentx_varbind *vb)
> agentx_varbind_integer(vb, 4);
> break;
> }
> + } else if (obj == hrSWRunPerfCPU) {
> + time = kinfo->p_rtime_sec * 100;
> + time += (kinfo->p_rtime_usec + 5000) / 1;
> + agentx_varbind_integer(vb, time);
> + } else if (obj == hrSWRunPerfMem) {
> + agentx_varbind_integer(vb, pagetok(kinfo->p_vm_tsize +
> + kinfo->p_vm_dsize + kinfo->p_vm_ssize));
> } else
> fatal("%s: Unexpected object", __func__);
> }
> @@ -3278,6 +3291,7 @@ main(int argc, char *argv[])
> kr_init();
> pf_init();
> timer_init();
> + pageshift_init();
>
> if (agentxsocket != NULL) {
> if (strlcpy(agentxsocketdir, agentxsocket,
> @@ -3375,6 +3389,10 @@ main(int argc, char *argv[])
> (hrSWRunType = agentx_object(host, AGENTX_OID(HRSWRUNTYPE),
> , 1, 0, mib_hrswrun)) == NULL ||
> (hrSWRunStatus = agentx_object(host, AGENTX_OID(HRSWRUNSTATUS),
> + , 1, 0, mib_hrswrun)) == NULL ||
> + (hrSWRunPerfCPU = agentx_object(host, AGENTX_OID(HRSWRUNPERFCPU),
> + , 1, 0, mib_hrswrun)) == NULL ||
> + (hrSWRunPerfMem = agentx_object(host, AGENTX_OID(HRSWRUNPERFMEM),
> , 1, 0, mib_hrswrun)) == NULL)
> fatal("agentx_object");
>
> @@ -4318,6 +4336,22 @@ main(int argc, char *argv[])
> log_setverbose(verbose);
>
> event_dispatch();
> +}
> +
> +#define LOG1024 10
> +void
> +pageshift_init(void)
> +{
> + long pagesize;
> +
> + if ((pagesize = sysconf(_SC_PAGESIZE)) == -1)
> + fatal("sysconf(_SC_PAGESIZE)");
> + while (pagesize > 1) {
> + pageshift++;
> + pagesize >>= 1;
> + }
> + /* we only need the amount of log(2)1024 for our conversion */
> + pageshift -= LOG1024;
> }
>
> void
>
Cruft: stop creating /usr/local/share/nls directories
The /usr/local/share/nls tree for localized message catalogs has
barely ever been used throughout the whole history of the ports
tree. The main user was shells/tcsh and that stopped four years
ago. Now, a single port is left that installs a single file there.
I would like to stop creating those directories by default in
mtree/4.4BSD.dist and the ports tree infrastructure.
Separate patches for src and ports attached.
OK?
--
Christian "naddy" Weisgerber na...@mips.inka.de
---
commit 44bc262d8170b10e6949c2836a820b7fee533de4 (local)
from: Christian Weisgerber
date: Tue Oct 24 14:09:57 2023 UTC
do not create /usr/local/share/nls and subdirectories by default
The share/nls/ paths are unused.
diff 85c70083e85910ddc7626ad658e5652c024b844a 44bc262d8170b10e6949c2836a820b7fee533de4
commit - 85c70083e85910ddc7626ad658e5652c024b844a
commit + 44bc262d8170b10e6949c2836a820b7fee533de4
blob - e72a9aa77292f4533044e3d97d6462141f8dfea3
blob + e3ce8a743046f036830fa1c4824ba426cad23ed6
--- etc/mtree/4.4BSD.dist
+++ etc/mtree/4.4BSD.dist
@@ -318,82 +318,6 @@ usr
misc
..
-# ./usr/local/share/nls
-nls
-C
-..
-da_DK.ISO_8859-1
-..
-de_AT.ISO_8859-1
-..
-de_CH.ISO_8859-1
-..
-de_DE.ISO_8859-1
-..
-el_GR.ISO_8859-7
-..
-en_AU.ISO_8859-1
-..
-en_CA.ISO_8859-1
-..
-en_GB.ISO_8859-1
-..
-en_US.ISO_8859-1
-..
-es_ES.ISO_8859-1
-..
-et_EE.ISO-8859-1
-..
-fi_FI.ISO_8859-1
-..
-fr_BE.ISO_8859-1
-..
-fr_CA.ISO_8859-1
-..
-fr_CH.ISO_8859-1
-..
-fr_FR.ISO_8859-1
-..
-hr_HR.ISO_8859-2
-..
-is_IS.ISO_8859-1
-..
-it_CH.ISO_8859-1
-..
-it_IT.ISO_8859-1
-..
-ja_JP.EUC
-..
-ko_KR.EUC
-..
-lt_LN.ASCII
-..
-lt_LN.ISO_8859-1
-..
-lt_LN.ISO_8859-2
-..
-nl_BE.ISO_8859-1
-..
-nl_NL.ISO_8859-1
-..
-no_NO.ISO_8859-1
-..
-pl_PL.ISO_8859-2
-..
-pt_PT.ISO_8859-1
-..
-ru_RU.CP866
-..
-ru_RU.ISO_8859-5
-..
-ru_RU.KOI8-R
-..
-sv_SE.ISO_8859-1
-..
-uk_UA.KOI8-U
-..
-..
-
# ./usr/local/share/pkgconfig
pkgconfig
..
---
commit f51da98f32c02018f7547f68db21d3c6ad474d99 (local)
from: Christian Weisgerber
date: Tue Oct 24 14:49:39 2023 UTC
do not create the ${PREFIX}/share/nls directory tree by default
The share/nls directories have been barely ever used throughout the
27-year history of the ports tree. Let the single port that still
installs a file there create the directory itself.
diff 16cf88f8fc810c890a483bf6ef62386b8610f8bd f51da98f32c02018f7547f68db21d3c6ad474d99
commit - 16cf88f8fc810c890a483bf6ef62386b8610f8bd
commit + f51da98f32c02018f7547f68db21d3c6ad474d99
blob - a189b247f6abf8b457c66bc4986770be81f639fe
blob + 32347397d0c4912d3b015aef226b9d2b652ac086
--- infrastructure/mk/bsd.port.mk
+++ infrastructure/mk/bsd.port.mk
@@ -2018,7 +2018,6 @@ _wrap_install_commands += ${_PBUILD} install -m ${BINM
_cat = {cat1,cat2,cat3,cat3f,cat3p,cat4,cat5,cat6,cat7,cat8,cat9,catl,catn}
_man = ${_cat:S/cat/man/g}
_treebase = ${WRKINST}${LOCALBASE}
-_nls = ${_treebase}/share/nls
_FAKE_TREE_LIST = \
${WRKINST}${BASESYSCONFDIR}/{firmware,rc.d} \
${_treebase}/bin \
@@ -2035,18 +2034,6 @@ _FAKE_TREE_LIST = \
${_treebase}/sbin \
${_treebase}/share/{dict,examples,misc,pkgconfig,skel} \
${_treebase}/share/doc/pkg-readmes \
- ${_nls}/{C,da_DK.ISO_8859-1,de_AT.ISO_8859-1,de_CH.ISO_8859-1} \
- ${_nls}/{de_DE.ISO_8859-1,el_GR.ISO_8859-7,en_AU.ISO_8859-1} \
- ${_nls}/{en_CA.ISO_8859-1,en_GB.ISO_8859-1,en_US.ISO_8859-1} \
- ${_nls}/{es_ES.ISO_8859-1,et_EE.ISO-8859-1,fi_FI.ISO_8859-1} \
- ${_nls}/{fr_BE.ISO_8859-1,fr_CA.ISO_8859-1,fr_CH.ISO_8859-1} \
- ${_nls}/fr_FR.ISO_8859-1 \
- ${_nls}/{hr_HR.ISO_8859-2,is_IS.ISO_8859-1,it_CH.ISO_8859-1} \
- ${_nls}/{it_IT.ISO_8859-1,ja_JP.EUC,ko_KR.EUC,lt_LN.ASCII} \
-
Re: ibuf free fd on close
> Thanks. Do we have a list of ports that use ibuf / imsg? Here's a list obtained from the nm output of all ports by grepping for '^ibuf_.* U' and '^imsg_.* U', respectively, then weeding out the obvious false positives:: ibuf: mail/opensmtpd-extras,-main net/ladvd imsg: audio/amused devel/got,-main mail/opensmtpd-extras,-main mail/pop3d mail/smtp-vilter mail/smtp-vilter,ldap net/gelatod net/gmid net/openmdns net/telescope net/thingsd sysutils/tabled sysutils/tmate
Re: ibuf free fd on close
On Tue, Oct 24, 2023 at 04:00:42PM +0200, Claudio Jeker wrote: > On Tue, Oct 24, 2023 at 03:50:47PM +0200, Theo Buehler wrote: > > On Tue, Oct 24, 2023 at 03:01:26PM +0200, Claudio Jeker wrote: > > > When I added ibuf_get_fd() the idea was to make sure that ibuf_free() will > > > close any fd still on the buffer. This way even if a fd is unexpectedly > > > passed nothing will happen. > > > > > > That code was disabled at start because userland was not fully ready. In > > > particular rpki-client did not handle that well. All of this is to my > > > knowledge fixed so there is no reason to keep the NOTYET :) > > > > > > With this users need to use ibuf_fd_get() to take the fd off the ibuf. > > > Code not doing so will break because ibuf_free() will close the fd which > > > is probably still in use somewhere else. > > > > Nothing in base outside of libutil seems to reach directly for the fd > > (checked by compiling with that struct member renamed in the public > > header). > > > > The internal uses are addressed by this diff, so > > > > ok tb > > > > I can put the fd rename through a bulk to catch some ports in a couple > > of days but I don't think there is a need to wait. > > Thanks. Do we have a list of ports that use ibuf / imsg? I don't think so. The list of ports using libutil is rather long. my sql is nonexistent but I get a few hundred.
Re: ibuf free fd on close
On Tue, Oct 24, 2023 at 03:50:47PM +0200, Theo Buehler wrote: > On Tue, Oct 24, 2023 at 03:01:26PM +0200, Claudio Jeker wrote: > > When I added ibuf_get_fd() the idea was to make sure that ibuf_free() will > > close any fd still on the buffer. This way even if a fd is unexpectedly > > passed nothing will happen. > > > > That code was disabled at start because userland was not fully ready. In > > particular rpki-client did not handle that well. All of this is to my > > knowledge fixed so there is no reason to keep the NOTYET :) > > > > With this users need to use ibuf_fd_get() to take the fd off the ibuf. > > Code not doing so will break because ibuf_free() will close the fd which > > is probably still in use somewhere else. > > Nothing in base outside of libutil seems to reach directly for the fd > (checked by compiling with that struct member renamed in the public > header). > > The internal uses are addressed by this diff, so > > ok tb > > I can put the fd rename through a bulk to catch some ports in a couple > of days but I don't think there is a need to wait. Thanks. Do we have a list of ports that use ibuf / imsg? -- :wq Claudio
Re: ibuf free fd on close
On Tue, Oct 24, 2023 at 03:01:26PM +0200, Claudio Jeker wrote: > When I added ibuf_get_fd() the idea was to make sure that ibuf_free() will > close any fd still on the buffer. This way even if a fd is unexpectedly > passed nothing will happen. > > That code was disabled at start because userland was not fully ready. In > particular rpki-client did not handle that well. All of this is to my > knowledge fixed so there is no reason to keep the NOTYET :) > > With this users need to use ibuf_fd_get() to take the fd off the ibuf. > Code not doing so will break because ibuf_free() will close the fd which > is probably still in use somewhere else. Nothing in base outside of libutil seems to reach directly for the fd (checked by compiling with that struct member renamed in the public header). The internal uses are addressed by this diff, so ok tb I can put the fd rename through a bulk to catch some ports in a couple of days but I don't think there is a need to wait.
ibuf free fd on close
When I added ibuf_get_fd() the idea was to make sure that ibuf_free() will
close any fd still on the buffer. This way even if a fd is unexpectedly
passed nothing will happen.
That code was disabled at start because userland was not fully ready. In
particular rpki-client did not handle that well. All of this is to my
knowledge fixed so there is no reason to keep the NOTYET :)
With this users need to use ibuf_fd_get() to take the fd off the ibuf.
Code not doing so will break because ibuf_free() will close the fd which
is probably still in use somewhere else.
--
:wq Claudio
Index: imsg-buffer.c
===
RCS file: /cvs/src/lib/libutil/imsg-buffer.c,v
retrieving revision 1.16
diff -u -p -r1.16 imsg-buffer.c
--- imsg-buffer.c 19 Jun 2023 17:19:50 - 1.16
+++ imsg-buffer.c 24 Oct 2023 12:55:44 -
@@ -294,10 +294,8 @@ ibuf_free(struct ibuf *buf)
{
if (buf == NULL)
return;
-#ifdef NOTYET
if (buf->fd != -1)
close(buf->fd);
-#endif
freezero(buf->buf, buf->size);
free(buf);
}
@@ -314,9 +312,7 @@ ibuf_fd_get(struct ibuf *buf)
int fd;
fd = buf->fd;
-#ifdef NOTYET
buf->fd = -1;
-#endif
return (fd);
}
@@ -480,11 +476,6 @@ static void
ibuf_dequeue(struct msgbuf *msgbuf, struct ibuf *buf)
{
TAILQ_REMOVE(>bufs, buf, entry);
-
- if (buf->fd != -1) {
- close(buf->fd);
- buf->fd = -1;
- }
msgbuf->queued--;
ibuf_free(buf);
Re: boot loaders: softraid volumes must be on RAID partitions
On Tue, Oct 24, 2023 at 01:44:08AM +, Klemens Nanni wrote:
> Rereading the code, I now question why it checks the 'a' label type at all.
>
> Taking your sd0d example through devboot():
>
> |#ifdef SOFTRAID
> | /*
> | * Determine the partition type for the 'a' partition of the
> | * boot device.
> | */
> | TAILQ_FOREACH(dip, , list)
> | if (dip->bios_info.bios_number == bootdev &&
> | (dip->bios_info.flags & BDI_BADLABEL) == 0)
> | part_type = dip->disklabel.d_partitions[0].p_fstype;
>
> Whatever sd0a's type is...
>
> |
> | /*
> | * See if we booted from a disk that is a member of a bootable
> | * softraid volume.
> | */
> | SLIST_FOREACH(bv, _volumes, sbv_link) {
> | if (bv->sbv_flags & BIOC_SCBOOTABLE)
> | SLIST_FOREACH(bc, >sbv_chunks, sbc_link)
> | if (bc->sbc_disk == bootdev)
>
> ... the boot loader sees we booted from a bootable softraid chunk,
> matching disk sd0 by number and not partition.
>
> | sr_boot_vol = bv->sbv_unit;
> | if (sr_boot_vol != -1)
> | break;
> | }
> |#endif
> |
> | if (sr_boot_vol != -1 && part_type != FS_BSDFFS) {
>
> With softraid chunk on sd0d, sd0a happens to be not FFS (likely unused),
> but that's still enough for the boot loader to stick to softraid on sd0!
>
> | *p++ = 's';
> | *p++ = 'r';
> | *p++ = '0' + sr_boot_vol;
> | } else if (bootdev & 0x100) {
>
> So why check the 'a' label type if that's not relevant?
The only reason I can see for that check is to better support the unusual
configuration of booting from a disk which happens to be part of a bootable
softraid volume, but which also has a regular FFS partition outside of the
RAID, and for whatever reason the user wants to automatically boot from that
instead.
Maybe that was useful before support for booting from softraid crypto volumes
was introduced, (when it was raid-1 only). It doesn't seem very useful now.
> It is confusing and
> Doubling down on the assumption that bootable chunks are always on 'a'
> like my diff did shows that's a mistake and dropping the check actually
> makes more sense to me now.
I agree it is confusing.
Effectively the code is there to treat part_type == FS_BSDFFS an 'edge case'
as described above.
> This boots with root on softraid on sd0a and sd0d.
>
> Thoughts?
> Am I missing something?
It works OK here, so I'm fine with removing this.
Re: snmpd [15/16]: When we have an error, all oids must be identical to the request
On Tue, Oct 17, 2023 at 03:31:20PM +0200, Martijn van Duren wrote: > RFC3416 section 4.2.1 (and others) tells us that if an error occurs the > varbindlist in the response must be identical to the original request. > > There might be other edge-cases here, but let's at least make sure that > the OIDs haven't been tampered with. ok
Re: snmpd [14/16]: Validate the returned error code
On Tue, Oct 17, 2023 at 03:28:12PM +0200, Martijn van Duren wrote: > > Certain error codes are only intended for certain request-types. Add an > appl_error_valid() function to test for this. ok tb
Re: snmpd [13/16]: registered instances must not return below OID
On Tue, Oct 17, 2023 at 03:25:29PM +0200, Martijn van Duren wrote:
> If a backend registers as an instance it must never return OIDs below
> their registration. Add a test for this in appl_varbind_valid().
>
> OK?
ok with a tiny nit inline
>
> martijn@
>
> diff --git a/application.c b/application.c
> index dfa7220..6ddeb39 100644
> --- a/application.c
> +++ b/application.c
> @@ -128,8 +128,8 @@ void appl_request_upstream_resolve(struct
> appl_request_upstream *);
> void appl_request_downstream_send(struct appl_request_downstream *);
> void appl_request_downstream_timeout(int, short, void *);
> void appl_request_upstream_reply(struct appl_request_upstream *);
> -int appl_varbind_valid(struct appl_varbind *, struct appl_varbind *, int,
> int,
> -int, const char **);
> +int appl_varbind_valid(struct appl_varbind *, struct appl_varbind_internal *,
> +int, int, int, const char **);
> int appl_varbind_backend(struct appl_varbind_internal *);
> void appl_varbind_error(struct appl_varbind_internal *, enum appl_error);
> void appl_report(struct snmp_message *, int32_t, struct ber_oid *,
> @@ -1073,9 +1073,9 @@ appl_response(struct appl_backend *backend, int32_t
> requestid,
>
> vb = vblist;
> for (i = 1; vb != NULL; vb = vb->av_next, i++) {
> -if (!appl_varbind_valid(vb, origvb == NULL ?
> - NULL : &(origvb->avi_varbind), next,
> -error != APPL_ERROR_NOERROR, backend->ab_range,
> )) {
> +if (!appl_varbind_valid(vb, origvb == NULL ? NULL : origvb,
could that not just be
if (!appl_varbind_valid(vb, origvb,
(also: use tabs for indent)
> + next, error != APPL_ERROR_NOERROR, backend->ab_range,
> + )) {
> smi_oid2string(&(vb->av_oid), oidbuf,
> sizeof(oidbuf), 0);
> log_warnx("%s: %"PRIu32" %s: %s",
> @@ -1173,8 +1173,9 @@ appl_response(struct appl_backend *backend, int32_t
> requestid,
> }
>
> int
> -appl_varbind_valid(struct appl_varbind *varbind, struct appl_varbind
> *request,
> -int next, int null, int range, const char **errstr)
> +appl_varbind_valid(struct appl_varbind *varbind,
> +struct appl_varbind_internal *request, int next, int null, int range,
> +const char **errstr)
> {
> int cmp;
> int eomv = 0;
> @@ -1259,24 +1260,32 @@ appl_varbind_valid(struct appl_varbind *varbind,
> struct appl_varbind *request,
> if (request == NULL)
> return 1;
>
> - cmp = ober_oid_cmp(&(request->av_oid), &(varbind->av_oid));
> - if (next && !eomv) {
> - if (request->av_include) {
> - if (cmp > 0) {
> - *errstr = "oid not incrementing";
> - return 0;
> + cmp = ober_oid_cmp(&(request->avi_varbind.av_oid), &(varbind->av_oid));
> + if (next) {
> + if (request->avi_region->ar_instance &&
> + ober_oid_cmp(&(request->avi_region->ar_oid),
> + &(varbind->av_oid)) != 0) {
> + *errstr = "oid below instance";
> + return 0;
> + }
> + if (!eomv) {
> + if (request->avi_varbind.av_include) {
> + if (cmp > 0) {
> + *errstr = "oid not incrementing";
> + return 0;
> + }
> + } else {
> + if (cmp >= 0) {
> + *errstr = "oid not incrementing";
> + return 0;
> + }
> }
> - } else {
> - if (cmp >= 0) {
> - *errstr = "oid not incrementing";
> + if (range && ober_oid_cmp(&(varbind->av_oid),
> + &(request->avi_varbind.av_oid_end)) > 0) {
> + *errstr = "end oid not honoured";
> return 0;
> }
> }
> - if (range && ober_oid_cmp(&(varbind->av_oid),
> - &(request->av_oid_end)) > 0) {
> - *errstr = "end oid not honoured";
> - return 0;
> - }
> } else {
> if (cmp != 0) {
> *errstr = "oids not equal";
>
Re: snmpd [12/16]: Make ab_range in application_agentx explicit 1
On Tue, Oct 17, 2023 at 03:21:45PM +0200, Martijn van Duren wrote: > appl_agentx_session doesn't set ab_range explicitly to 1, and thus > relies on malloc randomness to set it. Sit it explicitly. ok, but I haven't verified that all the session's members are now initialized. > > OK? > > martijn@ > > diff --git a/application_agentx.c b/application_agentx.c > index 680725d..79900d6 100644 > --- a/application_agentx.c > +++ b/application_agentx.c > @@ -548,6 +548,7 @@ appl_agentx_open(struct appl_agentx_connection *conn, > struct ax_pdu *pdu) > session->sess_backend.ab_cookie = session; > session->sess_backend.ab_retries = 0; > session->sess_backend.ab_fn = _agentx_functions; > + session->sess_backend.ab_range = 1; > RB_INIT(&(session->sess_backend.ab_requests)); > TAILQ_INSERT_TAIL(&(conn->conn_sessions), session, sess_conn_entry); > >
Re: snmpd [11/16]: When a request results in EOMV we must return the requesting OID
On Tue, Oct 17, 2023 at 03:17:19PM +0200, Martijn van Duren wrote: > According to RFC3416 section 4.2.2 and 4.2.3 case "(2)" when an > endOfMibView is returned the OID must be identical to originally > requested OID. Currently this can fail when the original request > is in a !last registered region and all subsequent regions also > return EOMV. > > Store the original OID and use that on EOMV. ok tb
Re: snmpd [10/16]: Make retries on open session where connection is closed return early
On Tue, Oct 17, 2023 at 03:13:57PM +0200, Martijn van Duren wrote: > Here's a special case unlikely to be found in the wild: > When opening 2 sessions on an agentx connection (already unusual) and > registering 2 overlapping regions on the different sessions, e.g. by > differing in priority (even more unusual) and we close the underlying > connection with an outstanding request to the dominant region we will > call appl_agentx_free(), which sequentially closes all sessions. > If the session with the outstanding request is closed before the > second session the request is retried before said session is cleaned > up and it will try to send it over a conn_ax which at that point has > been set to NULL, resulting in a SIGSEGV. > > Simply return early and let this second request be cancelled by the > cleanup of the second session. Makes total sense. ok tb
Re: snmpd [9/16]: Fix range handling with appl_unregister
On Tue, Oct 17, 2023 at 03:11:18PM +0200, Martijn van Duren wrote: > Right now (un)registering a region with range_subid set to !0 will > fail. Apparently nothing in the wild uses this, but let's fix it. > > This is the unregister part. ok tb
Re: snmpd [8/16]: Fix range handling with appl_register
On Tue, Oct 17, 2023 at 03:08:00PM +0200, Martijn van Duren wrote: > Right now (un)registering a region with range_subid set to !0 will > fail. Apparently nothing in the wild uses this, but let's fix it. > > This is the register part. ok tb
Re: snmpd [7/16]: Treat agentx-close-pdu with reasonByManager as a parseerror
On Tue, Oct 17, 2023 at 03:03:16PM +0200, Martijn van Duren wrote: > > RFC2741 section 6.2.2 says that reasonByManager can only be used by the > agentx master. Treat this reason as a parseerror. ok tb
Re: snmpd [6/16]: support close reason for appl_agentx_free
On Tue, Oct 17, 2023 at 02:59:52PM +0200, Martijn van Duren wrote: > appl_agentx_free() closes any potential open sessions before closing the > connection and cleaning up. This function is called from multiple > contexts and the current APPL_CLOSE_REASONSHUTDOWN is not always > applicable. Add a second reason parameter that can be passed onto > appl_agentx_forceclose(). ok tb
Re: snmpd [5/16]: Check context existence in appl_agentx_recv
On Tue, Oct 17, 2023 at 02:56:59PM +0200, Martijn van Duren wrote: > application.c checks the context where applicable, but not every > agentx-pdu goes through there (e.g. agentx-ping-pdu). Make sure > we always check the context in appl_agentx_recv() ok tb
Re: snmpd [4/16]: check agentx-pdu-header flags for validity
On Tue, Oct 17, 2023 at 02:54:07PM +0200, Martijn van Duren wrote: > RFC2741 section 6.1 specifies which PDUs can contain which header flags. > Check that that incoming agentx PDUs have valid flags in > appl_agentx_recv(). While here I cleaned up a few log messages some > minor restructuring to prevent the function growing too large. ok tb
Re: relayd.conf.5: less SSL
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 :) - Peter -- Peter N. M. Hansteen, member of the first RFC 1149 implementation team https://bsdly.blogspot.com/ https://www.bsdly.net/ https://www.nuug.no/ "Remember to set the evil bit on all malicious network traffic" delilah spamd[29949]: 85.152.224.147: disconnected after 42673 seconds.
relayd.conf.5: less SSL
Wanted to learn about TLS usage in relayd(8) and thought these SSL history bits in the TLS RELAYS section read out of place. Index: relayd.conf.5 === RCS file: /cvs/src/usr.sbin/relayd/relayd.conf.5,v retrieving revision 1.206 diff -u -p -r1.206 relayd.conf.5 --- relayd.conf.5 6 Jun 2023 15:16:52 - 1.206 +++ relayd.conf.5 24 Oct 2023 06:37:47 - @@ -728,8 +728,6 @@ In addition to plain TCP, .Xr relayd 8 supports the Transport Layer Security (TLS) cryptographic protocol for authenticated and encrypted relays. -TLS is the successor of the original Secure Sockets Layer (SSL) protocol, -but the term SSL is sometimes still used in modern TLS-based applications. .Xr relayd 8 can operate as a TLS client or server to offer a variety of options for different use cases related to TLS. There's more: - parse.y still accepting undocumented "ssl" with a warning since 2014 - more "SSL/TLS" instead of "TLS" in manual and code comments In comparison, httpd.conf(5) just has a few "SSL/TLS" mentions and nc(1) is "clean", so I'm inclined to dust off relayd a little (incl. parser). Feedback?
