packet-ldap: fix regression for SASL handling

commit 19b3376a24
("LDAP bogus malformed errors: decoding encrypted data")
introduced 2 problems:

- guint decr_len = tvb_reported_length(decr_tvb); was
  always called with decr_tvb==NULL

- dissect_ldap_payload() was not called if sasl_tree is NULL,
  it needs to be called even if the tree pointer are NULL
  in order to have the COL_INFO setup correctly.

I guess this should also be backported to stable branches
(together with 2e6d3b571b
 "LDAP: SASL Buffer doesn't include Length field")

https://gitlab.com/wireshark/wireshark/-/issues/17347

Signed-off-by: Stefan Metzmacher <metze@samba.org>


(cherry picked from commit 1d623fd541)
This commit is contained in:
Stefan Metzmacher 2021-04-06 20:52:17 +02:00 committed by Pascal Quantin
parent 0c1743656f
commit e49430b4d3
2 changed files with 31 additions and 33 deletions

View File

@ -1194,7 +1194,7 @@ static void
((strcmp(ldap_info->auth_mech, "GSS-SPNEGO") == 0) ||
/* auth_mech may have been set from the bind */
(strcmp(ldap_info->auth_mech, "GSSAPI") == 0))) {
tvbuff_t *gssapi_tvb, *plain_tvb = NULL, *decr_tvb= NULL;
tvbuff_t *gssapi_tvb = NULL;
int ver_len;
int tmp_length;
gssapi_encrypt_info_t gssapi_encrypt;
@ -1224,6 +1224,9 @@ static void
return;
}
if (gssapi_encrypt.gssapi_decrypted_tvb) {
tvbuff_t *decr_tvb = gssapi_encrypt.gssapi_decrypted_tvb;
proto_tree *enc_tree = NULL;
/*
* The LDAP payload (blob) was encrypted and we were able to decrypt it.
* The data was signed via a MIC token, sealed (encrypted), and "wrapped"
@ -1231,20 +1234,17 @@ static void
* one or more LDAPMessages such as searchRequest messages within this
* payload.
*/
col_set_str(pinfo->cinfo, COL_INFO, "SASL GSS-API Decrypted payload: ");
col_set_str(pinfo->cinfo, COL_INFO, "SASL GSS-API Privacy (decrypted): ");
if (sasl_tree) {
proto_tree *enc_tree;
guint decr_len = tvb_reported_length(decr_tvb);
decr_tvb = gssapi_encrypt.gssapi_decrypted_tvb;
enc_tree = proto_tree_add_subtree_format(sasl_tree, decr_tvb, 0, -1,
ett_ldap_payload, NULL, "GSS-API Decrypted payload (%d byte%s)",
ett_ldap_payload, NULL, "GSS-API Encrypted payload (%d byte%s)",
decr_len, plurality(decr_len, "", "s"));
dissect_ldap_payload(decr_tvb, pinfo, enc_tree, ldap_info, is_mscldap);
}
dissect_ldap_payload(decr_tvb, pinfo, enc_tree, ldap_info, is_mscldap);
}
else if (gssapi_encrypt.gssapi_data_encrypted) {
/*
@ -1257,6 +1257,9 @@ static void
proto_tree_add_item(sasl_tree, hf_ldap_gssapi_encrypted_payload, gssapi_tvb, ver_len, -1, ENC_NA);
}
else {
tvbuff_t *plain_tvb = tvb_new_subset_remaining(gssapi_tvb, ver_len);
proto_tree *plain_tree = NULL;
/*
* The payload was not encrypted (sealed) but was signed via a MIC token.
* If krb5_tok_id == KRB_TOKEN_CFX_WRAP, the payload was wrapped within
@ -1266,18 +1269,14 @@ static void
col_set_str(pinfo->cinfo, COL_INFO, "SASL GSS-API Integrity: ");
if (sasl_tree) {
guint plain_len;
proto_tree *plain_tree;
plain_tvb = tvb_new_subset_remaining(gssapi_tvb, ver_len);
plain_len = tvb_reported_length(plain_tvb);
guint plain_len = tvb_reported_length(plain_tvb);
plain_tree = proto_tree_add_subtree_format(sasl_tree, plain_tvb, 0, -1,
ett_ldap_payload, NULL, "GSS-API payload (%d byte%s)",
plain_len, plurality(plain_len, "", "s"));
dissect_ldap_payload(plain_tvb, pinfo, plain_tree, ldap_info, is_mscldap);
}
dissect_ldap_payload(plain_tvb, pinfo, plain_tree, ldap_info, is_mscldap);
}
}
} else {

View File

@ -4104,7 +4104,7 @@ static void
((strcmp(ldap_info->auth_mech, "GSS-SPNEGO") == 0) ||
/* auth_mech may have been set from the bind */
(strcmp(ldap_info->auth_mech, "GSSAPI") == 0))) {
tvbuff_t *gssapi_tvb, *plain_tvb = NULL, *decr_tvb= NULL;
tvbuff_t *gssapi_tvb = NULL;
int ver_len;
int tmp_length;
gssapi_encrypt_info_t gssapi_encrypt;
@ -4134,6 +4134,9 @@ static void
return;
}
if (gssapi_encrypt.gssapi_decrypted_tvb) {
tvbuff_t *decr_tvb = gssapi_encrypt.gssapi_decrypted_tvb;
proto_tree *enc_tree = NULL;
/*
* The LDAP payload (blob) was encrypted and we were able to decrypt it.
* The data was signed via a MIC token, sealed (encrypted), and "wrapped"
@ -4141,20 +4144,17 @@ static void
* one or more LDAPMessages such as searchRequest messages within this
* payload.
*/
col_set_str(pinfo->cinfo, COL_INFO, "SASL GSS-API Decrypted payload: ");
col_set_str(pinfo->cinfo, COL_INFO, "SASL GSS-API Privacy (decrypted): ");
if (sasl_tree) {
proto_tree *enc_tree;
guint decr_len = tvb_reported_length(decr_tvb);
decr_tvb = gssapi_encrypt.gssapi_decrypted_tvb;
enc_tree = proto_tree_add_subtree_format(sasl_tree, decr_tvb, 0, -1,
ett_ldap_payload, NULL, "GSS-API Decrypted payload (%d byte%s)",
ett_ldap_payload, NULL, "GSS-API Encrypted payload (%d byte%s)",
decr_len, plurality(decr_len, "", "s"));
dissect_ldap_payload(decr_tvb, pinfo, enc_tree, ldap_info, is_mscldap);
}
dissect_ldap_payload(decr_tvb, pinfo, enc_tree, ldap_info, is_mscldap);
}
else if (gssapi_encrypt.gssapi_data_encrypted) {
/*
@ -4167,6 +4167,9 @@ static void
proto_tree_add_item(sasl_tree, hf_ldap_gssapi_encrypted_payload, gssapi_tvb, ver_len, -1, ENC_NA);
}
else {
tvbuff_t *plain_tvb = tvb_new_subset_remaining(gssapi_tvb, ver_len);
proto_tree *plain_tree = NULL;
/*
* The payload was not encrypted (sealed) but was signed via a MIC token.
* If krb5_tok_id == KRB_TOKEN_CFX_WRAP, the payload was wrapped within
@ -4176,18 +4179,14 @@ static void
col_set_str(pinfo->cinfo, COL_INFO, "SASL GSS-API Integrity: ");
if (sasl_tree) {
guint plain_len;
proto_tree *plain_tree;
plain_tvb = tvb_new_subset_remaining(gssapi_tvb, ver_len);
plain_len = tvb_reported_length(plain_tvb);
guint plain_len = tvb_reported_length(plain_tvb);
plain_tree = proto_tree_add_subtree_format(sasl_tree, plain_tvb, 0, -1,
ett_ldap_payload, NULL, "GSS-API payload (%d byte%s)",
plain_len, plurality(plain_len, "", "s"));
dissect_ldap_payload(plain_tvb, pinfo, plain_tree, ldap_info, is_mscldap);
}
dissect_ldap_payload(plain_tvb, pinfo, plain_tree, ldap_info, is_mscldap);
}
}
} else {
@ -5633,7 +5632,7 @@ void proto_register_ldap(void) {
NULL, HFILL }},
/*--- End of included file: packet-ldap-hfarr.c ---*/
#line 2158 "./asn1/ldap/packet-ldap-template.c"
#line 2157 "./asn1/ldap/packet-ldap-template.c"
};
/* List of subtrees */
@ -5707,7 +5706,7 @@ void proto_register_ldap(void) {
&ett_ldap_T_warning,
/*--- End of included file: packet-ldap-ettarr.c ---*/
#line 2172 "./asn1/ldap/packet-ldap-template.c"
#line 2171 "./asn1/ldap/packet-ldap-template.c"
};
/* UAT for header fields */
static uat_field_t custom_attribute_types_uat_fields[] = {
@ -5896,7 +5895,7 @@ proto_reg_handoff_ldap(void)
/*--- End of included file: packet-ldap-dis-tab.c ---*/
#line 2344 "./asn1/ldap/packet-ldap-template.c"
#line 2343 "./asn1/ldap/packet-ldap-template.c"
dissector_add_uint_range_with_preference("tcp.port", TCP_PORT_RANGE_LDAP, ldap_handle);