Ethereal-dev: Re: [ethereal-dev] packet-snmp.c patch to handle zero length context names

Note: This archive is from the project's previous web site, ethereal.com. This list is no longer active.

From: Guy Harris <gharris@xxxxxxxxxxxx>
Date: Tue, 4 Jul 2000 01:09:01 -0700
On Tue, Jul 04, 2000 at 09:51:38AM +0200, Juergen Schoenwaelder wrote:
> 
> >>>>> Guy Harris writes:
> 
> >> There is some confusion here. The community based security model
> >> is not used with the SNMPv3 message format.
> 
> Guy> ...but the SNMP dissector checks for SNMP_SEC_V1 and SNMP_SEC_V2C
> Guy> in the code after
> 
> Guy> 	case SNMP_VERSION_3:
> 
> Guy> that checks the SNMP version number in the SNMP packet, which I
> Guy> infer means either that
> 
> Guy> 	1) the values "reserved for SNMPv1" and "reserved for SNMPv2c"
> Guy> *are* used with the SNMPv3 message format, and the format of the
> Guy> msgSecurityParameters field in SNMPv3 packets when those
> Guy> "reserved" values are used is documented somewhere
> 
> Guy> or
> 
> Guy> 	2) the SNMPv3 dissection code should just treat the
> Guy> msgSecurityParameters field as opaque hex data.
> 
> The version number is the first element in every SNMP message. So you
> first decode the version number and based on its value, you either
> decode an SNMPv1/SNMPv2c or an SNMPv3 message. In other words, the
> SNMPv3 message format always uses the version number 3.

Yes, that's obvious.

I'm not asking about that.

I'm asking about the code in "packet-snmp.c" that, in the code after
"case SNMP_VERSION_3:", does

		switch(msgsec) {
		case SNMP_SEC_V1:
		case SNMP_SEC_V2C:
			ret = asn1_octet_string_decode (&asn1, 
			    &secparm, &secparm_length, &length);
			if (ret != ASN1_ERR_NOERROR) {
				dissect_snmp_parse_error(pd, offset, fd, tree, 
				    "Message Security Parameters", ret);
				return;
			}
			if (snmp_tree) {
				proto_tree_add_text(snmp_tree, NullTVB, offset,
				    length, "Message Security Parameters: %.*s",
				    secparm_length, secparm);
			}
			g_free(secparm);
			offset += length;
			break;
		case SNMP_SEC_USM:
			[handle the user security mode]
			break;
		default:
			ret = asn1_octet_string_decode (&asn1, 
			    &secparm, &secparm_length, &length);
			if (ret != ASN1_ERR_NOERROR) {
				dissect_snmp_parse_error(pd, offset, fd, tree, 
				    "Message Security Parameters", ret);
				return;
			}
			if (snmp_tree) {
				proto_tree_add_text(snmp_tree, NullTVB, offset,
				    length,
				    "Message Security Parameters Data"
				    " (%d bytes)", secparm_length);
			}
			g_free(secparm);
			offset += length;
			break;
		}

> For SNMPv3 messages, there currently only exists the user-based
> security model.

In other words, the code I list above should, instead, be:

		switch(msgsec) {
		case SNMP_SEC_USM:
			[handle the user-based security model]
			break;
		default:
			ret = asn1_octet_string_decode (&asn1, 
			    &secparm, &secparm_length, &length);
			if (ret != ASN1_ERR_NOERROR) {
				dissect_snmp_parse_error(pd, offset, fd, tree, 
				    "Message Security Parameters", ret);
				return;
			}
			if (snmp_tree) {
				proto_tree_add_text(snmp_tree, NullTVB, offset,
				    length,
				    "Message Security Parameters Data"
				    " (%d bytes)", secparm_length);
			}
			g_free(secparm);
			offset += length;
			break;
		}

as the *ONLY* value "msgsec" should have in that code, which is
dissecting SNMPv3 packets, should be SNMP_SEC_USM, as there only exists
the user-based security model.