#1642 snmpd doesn't accept authentication from smux sub-agent

[Irwin Tillman]

snmpd 5.2.2 and later doesn't accept authentication
from a smux sub-agent.

Broken starting in version 5.2.2.

Verified works fine in net-snmp versions 5.1.3, 5.1.4,
5.2.1.2.
Verified broken in net-snmp versions 5.2.2, 5.2.3,
5.3.0.1, 5.3.1.

Platform: Solaris 9 on SPARC.

When running net-snmp 5.1.3, 5.1.4, and 5.2.1.2, an
smux sub-agent in
freeradius 1.0.5 and 1.1.2 was able to talk to snmpd
fine.

In net-snmp 5.2.2, 5.2.3, 5.3.0.1, and 5.3.0.1,
snmpd is refusing to accept the connection from the
freeradius, apparently because snmpd thinks the smux
peer's password is wrong.

Debug of freeradius shows:

radiusd\[13113\]: SMUX\_CLOSE with reason: 5

I think that the '5' reason is referring to
SMUXC_AUTHENTICATIONFAILURE
as per net-snmp's ./agent/mibgroup/smux/smux.h

Debug of snmpd 5.3.0.1 shows (smux sub-agent password
changed to 'foobar' to protect the guilty):

at snmpd startup:
...
registered debug token smux, 1
registered debug token smux\_conf, 1
registered debug token smux\_init, 1
smux\_conf: parsing registration for:

.1.3.6.1.4.1.3317.1.3.1 foobar
smux_conf: password is: foobar

when freeradius's smux sub-agent tries to connect

to snmpd:
...
smux: [smux_accept] Calling accept()
[smux_accept] accepted fd 9 from 127.0.0.1:56878
smux: [smux_open_process] version 0, len 33, type 2
smux: [smux_open_process] smux peer:
SNMPv2-SMI::enterprises.3317.1.3.1
smux: [smux_open_process] len 21, type 6
smux: [smux_open_process] smux peer descr: radiusd
smux: [smux_open_process] len 12, type 4
smux: [smux_open_process] smux peer passwd: foobar
smux: [smux_open_process] len 0, type 4
refused smux peer: oid
SNMPv2-SMI::enterprises.3317.1.3.1, descr radiusd
smux: [smux_close] sending close to fd 9, reason 5
smux: [smux_accept] peer on 9 failed authentication

snmpd seems to be saying that snmpd initialized
password "foobar" for this OID,
but when SMUX client connected and supplied password
"foobar", snmpd
said that this is the wrong password.

/etc/snmp/snmpd.conf hasn't changed; the relevant
config line is:

smuxpeer .1.3.6.1.4.1.3317.1.3.1 foobar

It's 100% reproducible. I make no changes to the
config files;
I simply replace one version of net-snmp with another.

[Hoemig]

Logged In: YES
user_id=1552812

Hi,

I've the same problem. When I debug, the problem happends in
the Funktion: mib.c -> _add_string_to_oid(..)

New Callstack (buggy):
----------------------
smux_parse_peer_auth(..)
-> read_objid( cptr, aptr->sa_oid, &aptr->sa_oid_len)
-> _add_string_to_oid(..)

Old Callstack (working):
------------------------
smux_parse_peer_auth(..)
-> aptr->sa_oid_len = parse_miboid(cptr, aptr->sa_oid)

#############################################
When I use the config "smuxpeer .1.2.3.4 abc"

(Old Implementation)
aptr->sa_oid = "1234"

(New Implementation)
aptr->sa_oid = "123" (and the oid_compare in
smux_auth_peer failed every time)

I think the funktion "_add_string_to_oid(..)" doesn't
support strings with more informations than the OID !)

When I use the config "smuxpeer .1.2.3.4" the
aptr->sa_oid = "1234" in both versions (OLD/NEW)

When I patch the funktion: smux_parse_peer_auth(..)
to:

void
smux_parse_peer_auth(const char *token, char *cptr)
{
smux_peer_auth *aptr;

if \(\(aptr =
     \(smux\_peer\_auth \*\) calloc\(1,

sizeof(smux_peer_auth))) == NULL) {
snmp_log_perror("smux_parse_peer_auth: malloc");
return;
}
if (nauths == SMUX_MAX_PEERS) {
config_perror("Too many smuxpeers");
return;
}
aptr->sa_active_fd = -1;
if (!cptr) {
/*
* null passwords OK
*/
Auths[nauths++] = aptr;
DEBUGMSGTL(("smux_conf", "null password\n"));
return;
}

#if 0 /* OLD */
/*
* oid
*/
aptr->sa_oid_len = MAX_OID_LEN;
read_objid( cptr, aptr->sa_oid, &aptr->sa_oid_len );

DEBUGMSGTL\(\("smux\_conf", "parsing registration for:

%s\n", cptr));

cptr = skip\_token\(cptr\);
DEBUGMSGTL\(\("smux\_conf", "password is: %s\n", cptr ?

cptr : "NULL"));
/*
* password
*/
if (cptr)
strcpy(aptr->sa_passwd, cptr);

Auths\[nauths++\] = aptr;

#else /* NEW */
/*
* oid
*/
else
{
char *password_cptr = strchr(cptr,' ');
int cptr_len = strlen(cptr);
if (password_cptr)
*password_cptr = 0x0;

   aptr->sa\_oid\_len = MAX\_OID\_LEN;
   read\_objid\( cptr, aptr->sa\_oid, &aptr->sa\_oid\_len \);

   DEBUGMSGTL\(\("smux\_conf", "parsing registration for:

%s\n", cptr));

   if \(\(&password\_cptr - &cptr + 1\) < cptr\_len\)
   \{
      cptr = ++password\_cptr;
      DEBUGMSGTL\(\("smux\_conf", "password is: %s\n", cptr

? cptr : "NULL"));
}
/*
* password
*/
if (cptr)
strcpy(aptr->sa_passwd, cptr);

   Auths\[nauths++\] = aptr;
\}

#endif
}

It work fine. Test it.
My Version is 5.3.0.1

Greatings from Germany,

Alexander

(Excuse the bad English)

[Thomas Anders]

Logged In: YES
user_id=848638

Can anyone provide minimal source code for a smux subagent
serving a single scalar to reproduce the behaviour? Assumed
it was provided under the existing net-snmp license, I'll
look into extending "make test" to make sure we won't break
this again.

[Hoemig]

Logged In: YES
user_id=1552812

But that is not the only error.

For example:
------------
1.) Function: smux_snmp_process
if the recv(..) function returns "-1"
the function smux_peer_cleanup will be called.
But the socket-discriptor in the sdlist from the snmpd.c
for the select-call will be not removed.
So select crash.

2.) Function: smux_trap_process
You never receive TRAPs with the values.
Reason: The vartype will be set after the
snmp_set_var_value was call. It must be set
before.

3.) Funktion: smux_snmp_process
...
size_t length = SMUXMAXPKTSIZE
...
length = recv(...) /* return -1, when an error occurs
...
if (length < 0) /* will never match */
error-handling

process packet
   ...crash \(if length == -1\)...

\----------------------
length is never < 0 because size\_t => unsigned int.
length must be "ssize\_t"

4.) .... and lot more bugs

[Thomas Anders]

Logged In: YES
user_id=848638

SMUX is deprecated in favour of AgentX. Because of this the
SMUX code isn't well-maintained, so it's not surprising it
has issues. We'd like to fix them anyway. Can you propose
patches, please?

[Hoemig]

Logged In: YES
user_id=1552812

But that is not the only error.

For example:
------------
1.) Function: smux_snmp_process
if the recv(..) function returns "-1"
the function smux_peer_cleanup will be called.
But the socket-discriptor in the sdlist from the snmpd.c
for the select-call will be not removed.
So select crash.

2.) Function: smux_trap_process
You never receive TRAPs with the values.
Reason: The vartype will be set after the
snmp_set_var_value was call. It must be set
before.

3.) Funktion: smux_snmp_process
...
size_t length = SMUXMAXPKTSIZE
...
length = recv(...) /* return -1, when an error occurs
...
if (length < 0) /* will never match */
error-handling

process packet
   ...crash \(if length == -1\)...

\----------------------
length is never < 0 because size\_t => unsigned int.
length must be "ssize\_t"

4.) .... and lot more bugs

[Thomas Anders]

Logged In: YES
user_id=848638

Item 2 has already been fixed in 5.3.1 (see patch 1430824).
Can you please provide the patch you posted inline in
unified diff ("diff -u") format and send it to me?

[Thomas Anders]

Logged In: YES
user_id=848638

I've applied the attached fix for 5.2.x, 5.3.x and MAIN.
It'll appear in 5.2.4, 5.3.2 and 5.4. As for items 1 and 3
below, please open a separate bug on them.

[Thomas Anders]

smux auth patch

[Irwin Tillman]

Logged In: YES
user_id=1561046
Originator: YES

I see the release notes for release 5.2.4 indicate the fix is incorporated in that release.
But when I test 5.2.4, it still appears to exhibit the same problem.

[Thomas Anders]

Logged In: YES
user_id=848638
Originator: NO

Bummer! Can you also try 5.4, please, so we'll know whether the patch is broken in general or just in 5.2.x?

[Irwin Tillman]

Logged In: YES
user_id=1561046
Originator: YES

I tested it in 5.4 a few months ago, and verified it did work for me there.