From 923817e26984328a82275555e5bdd9e8e1f52d3e Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 4 Aug 2023 13:57:12 +0200 Subject: [PATCH 1/3] s4:torture/ndr: add tests for DCERPC_PKT_CO_CANCEL and DCERPC_PKT_ORPHANED The PDUs were generated by Windows clients. And we fail to parse them currently. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15446 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit c37adb762640b7df9731d6a60edce808aa8787f8) --- selftest/knownfail.d/samba4.local.ndr.dcerpc | 4 + source4/torture/ndr/dcerpc.c | 148 +++++++++++++++++++ source4/torture/ndr/ndr.c | 1 + source4/torture/wscript_build | 1 + 4 files changed, 154 insertions(+) create mode 100644 selftest/knownfail.d/samba4.local.ndr.dcerpc create mode 100644 source4/torture/ndr/dcerpc.c diff --git a/selftest/knownfail.d/samba4.local.ndr.dcerpc b/selftest/knownfail.d/samba4.local.ndr.dcerpc new file mode 100644 index 000000000000..9ba0e94617d4 --- /dev/null +++ b/selftest/knownfail.d/samba4.local.ndr.dcerpc @@ -0,0 +1,4 @@ +samba4.local.ndr.dcerpc.co_cancel.ncacn_packet_VALIDATE.ncacn_packet_co_cancel_check +samba4.local.ndr.dcerpc.orphaned.ncacn_packet_VALIDATE.ncacn_packet_orphaned_check +samba4.local.ndr.system.iconv.dcerpc.co_cancel.ncacn_packet_VALIDATE.ncacn_packet_co_cancel_check +samba4.local.ndr.system.iconv.dcerpc.orphaned.ncacn_packet_VALIDATE.ncacn_packet_orphaned_check diff --git a/source4/torture/ndr/dcerpc.c b/source4/torture/ndr/dcerpc.c new file mode 100644 index 000000000000..459817d49516 --- /dev/null +++ b/source4/torture/ndr/dcerpc.c @@ -0,0 +1,148 @@ +/* + Unix SMB/CIFS implementation. + test suite for dcerpc ndr operations + + Copyright (C) Stefan Metzmacher 2023 + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +*/ + +#include "includes.h" +#include "torture/ndr/ndr.h" +#include "librpc/gen_ndr/ndr_dcerpc.h" +#include "torture/ndr/proto.h" + +/* + * ncacn_packet: struct ncacn_packet + * rpc_vers : 0x05 (5) + * rpc_vers_minor : 0x00 (0) + * ptype : DCERPC_PKT_CO_CANCEL (18) + * pfc_flags : 0x06 (6) + * 0: DCERPC_PFC_FLAG_FIRST + * 1: DCERPC_PFC_FLAG_LAST + * 1: DCERPC_PFC_FLAG_PENDING_CANCEL_OR_HDR_SIGNING + * 0: DCERPC_PFC_FLAG_CONC_MPX + * 0: DCERPC_PFC_FLAG_DID_NOT_EXECUTE + * 0: DCERPC_PFC_FLAG_MAYBE + * 0: DCERPC_PFC_FLAG_OBJECT_UUID + * drep: ARRAY(4) + * [0] : 0x10 (16) + * [1] : 0x00 (0) + * [2] : 0x00 (0) + * [3] : 0x00 (0) + * frag_length : 0x0010 (16) + * auth_length : 0x0000 (0) + * call_id : 0x00000001 (1) + * u : union dcerpc_payload(case 18) + * co_cancel: struct dcerpc_co_cancel + * auth_info : DATA_BLOB length=0 + */ +static const uint8_t ncacn_packet_co_cancel_data[] = { + 0x05, 0x00, 0x12, 0x06, 0x10, 0x00, 0x00, 0x00, + 0x10, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, +}; + +static bool ncacn_packet_co_cancel_check(struct torture_context *tctx, + struct ncacn_packet *pkt) +{ + torture_assert_int_equal(tctx, pkt->rpc_vers, 5, "rpc_vers"); + torture_assert_int_equal(tctx, pkt->rpc_vers_minor, 0, "rpc_vers_minor"); + torture_assert_int_equal(tctx, pkt->ptype, DCERPC_PKT_CO_CANCEL, "ptype"); + torture_assert_int_equal(tctx, pkt->pfc_flags, + DCERPC_PFC_FLAG_LAST | + DCERPC_PFC_FLAG_PENDING_CANCEL_OR_HDR_SIGNING, + "pfc_flags"); + torture_assert_int_equal(tctx, pkt->drep[0], DCERPC_DREP_LE, "drep[0]"); + torture_assert_int_equal(tctx, pkt->drep[1], 0, "drep[1]"); + torture_assert_int_equal(tctx, pkt->drep[2], 0, "drep[2]"); + torture_assert_int_equal(tctx, pkt->drep[3], 0, "drep[3]"); + torture_assert_int_equal(tctx, pkt->frag_length, 16, "frag_length"); + torture_assert_int_equal(tctx, pkt->auth_length, 0, "auth_length"); + torture_assert_int_equal(tctx, pkt->call_id, 1, "call_id"); + torture_assert_int_equal(tctx, pkt->u.co_cancel.auth_info.length, 0, + "co_cancel.auth_info.length"); + return true; +} + +/* + * ncacn_packet: struct ncacn_packet + * rpc_vers : 0x05 (5) + * rpc_vers_minor : 0x00 (0) + * ptype : DCERPC_PKT_ORPHANED (19) + * pfc_flags : 0x03 (3) + * 1: DCERPC_PFC_FLAG_FIRST + * 1: DCERPC_PFC_FLAG_LAST + * 0: DCERPC_PFC_FLAG_PENDING_CANCEL_OR_HDR_SIGNING + * 0: DCERPC_PFC_FLAG_CONC_MPX + * 0: DCERPC_PFC_FLAG_DID_NOT_EXECUTE + * 0: DCERPC_PFC_FLAG_MAYBE + * 0: DCERPC_PFC_FLAG_OBJECT_UUID + * drep: ARRAY(4) + * [0] : 0x10 (16) + * [1] : 0x00 (0) + * [2] : 0x00 (0) + * [3] : 0x00 (0) + * frag_length : 0x0010 (16) + * auth_length : 0x0000 (0) + * call_id : 0x00000008 (8) + * u : union dcerpc_payload(case 19) + * orphaned: struct dcerpc_orphaned + * auth_info : DATA_BLOB length=0 + */ +static const uint8_t ncacn_packet_orphaned_data[] = { + 0x05, 0x00, 0x13, 0x03, 0x10, 0x00, 0x00, 0x00, + 0x10, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, +}; + +static bool ncacn_packet_orphaned_check(struct torture_context *tctx, + struct ncacn_packet *pkt) +{ + torture_assert_int_equal(tctx, pkt->rpc_vers, 5, "rpc_vers"); + torture_assert_int_equal(tctx, pkt->rpc_vers_minor, 0, "rpc_vers_minor"); + torture_assert_int_equal(tctx, pkt->ptype, DCERPC_PKT_ORPHANED, "ptype"); + torture_assert_int_equal(tctx, pkt->pfc_flags, + DCERPC_PFC_FLAG_FIRST|DCERPC_PFC_FLAG_LAST, + "pfc_flags"); + torture_assert_int_equal(tctx, pkt->drep[0], DCERPC_DREP_LE, "drep[0]"); + torture_assert_int_equal(tctx, pkt->drep[1], 0, "drep[1]"); + torture_assert_int_equal(tctx, pkt->drep[2], 0, "drep[2]"); + torture_assert_int_equal(tctx, pkt->drep[3], 0, "drep[3]"); + torture_assert_int_equal(tctx, pkt->frag_length, 16, "frag_length"); + torture_assert_int_equal(tctx, pkt->auth_length, 0, "auth_length"); + torture_assert_int_equal(tctx, pkt->call_id, 8, "call_id"); + torture_assert_int_equal(tctx, pkt->u.orphaned.auth_info.length, 0, + "orphaned.auth_info.length"); + return true; +} + +struct torture_suite *ndr_dcerpc_suite(TALLOC_CTX *ctx) +{ + struct torture_suite *suite = torture_suite_create(ctx, "dcerpc"); + struct torture_suite *co_cancel = torture_suite_create(ctx, "co_cancel"); + struct torture_suite *orphaned = torture_suite_create(ctx, "orphaned"); + + torture_suite_add_suite(suite, co_cancel); + torture_suite_add_ndr_pull_validate_test(co_cancel, + ncacn_packet, + ncacn_packet_co_cancel_data, + ncacn_packet_co_cancel_check); + + torture_suite_add_suite(suite, orphaned); + torture_suite_add_ndr_pull_validate_test(orphaned, + ncacn_packet, + ncacn_packet_orphaned_data, + ncacn_packet_orphaned_check); + + return suite; +} diff --git a/source4/torture/ndr/ndr.c b/source4/torture/ndr/ndr.c index 99d8f8adca4f..e2a2c33ca5af 100644 --- a/source4/torture/ndr/ndr.c +++ b/source4/torture/ndr/ndr.c @@ -759,6 +759,7 @@ struct torture_suite *torture_local_ndr(TALLOC_CTX *mem_ctx) { struct torture_suite *suite = torture_suite_create(mem_ctx, "ndr"); + torture_suite_add_suite(suite, ndr_dcerpc_suite(suite)); torture_suite_add_suite(suite, ndr_winreg_suite(suite)); torture_suite_add_suite(suite, ndr_atsvc_suite(suite)); torture_suite_add_suite(suite, ndr_lsa_suite(suite)); diff --git a/source4/torture/wscript_build b/source4/torture/wscript_build index d870a3ab8076..6bfc6aeae65c 100644 --- a/source4/torture/wscript_build +++ b/source4/torture/wscript_build @@ -47,6 +47,7 @@ if bld.CONFIG_SET('WITH_NTVFS_FILESERVER'): bld.SAMBA_SUBSYSTEM('TORTURE_NDR', source='''ndr/ndr.c + ndr/dcerpc.c ndr/winreg.c ndr/atsvc.c ndr/lsa.c -- 2.34.1 From 216978b600931ed2e36f2c551408c56400b7efca Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 7 Aug 2023 16:16:27 +0200 Subject: [PATCH 2/3] librpc/rpc: let dcerpc_read_ncacn_packet_next_vector() handle fragments without any payload DCERPC_PKT_CO_CANCEL and DCERPC_PKT_ORPHANED don't have any payload by default. In order to receive them via dcerpc_read_ncacn_packet_send/recv we need to allow fragments with frag_len == DCERPC_NCACN_PAYLOAD_OFFSET. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15446 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 5c724a3e156ae734e4d187bf9639d895bb011834) --- librpc/rpc/dcerpc_util.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/librpc/rpc/dcerpc_util.c b/librpc/rpc/dcerpc_util.c index c3997eb1b19f..e3c81b6194ae 100644 --- a/librpc/rpc/dcerpc_util.c +++ b/librpc/rpc/dcerpc_util.c @@ -565,9 +565,14 @@ static int dcerpc_read_ncacn_packet_next_vector(struct tstream_context *stream, ofs = state->buffer.length; - if (frag_len < ofs) { + if (frag_len <= ofs) { /* - * something is wrong, let the caller deal with it + * With frag_len == ofs, we are done, this is likely + * a DCERPC_PKT_CO_CANCEL and DCERPC_PKT_ORPHANED + * without any payload. + * + * Otherwise it's a broken packet and we + * let the caller deal with it. */ *_vector = NULL; *_count = 0; -- 2.34.1 From f70c2713b812f46692f02d7ec2f91a9eea965f49 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 4 Aug 2023 14:03:43 +0200 Subject: [PATCH 3/3] dcerpc.idl: fix definitions for DCERPC_PKT_CO_CANCEL and DCERPC_PKT_ORPHANED payload It seems commit 259129e8f4bc8cacd1850eba3f6551134835d079 was partly just fantasy... Windows clients just use 16 bytes for DCERPC_PKT_CO_CANCEL and DCERPC_PKT_ORPHANED pdus. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15446 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke Autobuild-User(master): Volker Lendecke Autobuild-Date(master): Tue Aug 8 08:57:46 UTC 2023 on atb-devel-224 (cherry picked from commit 9ec22e680249cfde06fb1a0a34fcc94d1f47002d) --- librpc/idl/dcerpc.idl | 2 -- selftest/knownfail.d/samba4.local.ndr.dcerpc | 4 ---- 2 files changed, 6 deletions(-) delete mode 100644 selftest/knownfail.d/samba4.local.ndr.dcerpc diff --git a/librpc/idl/dcerpc.idl b/librpc/idl/dcerpc.idl index bbb17f0b8c41..1850526042c6 100644 --- a/librpc/idl/dcerpc.idl +++ b/librpc/idl/dcerpc.idl @@ -276,12 +276,10 @@ interface dcerpc } dcerpc_auth3; typedef [public] struct { - [value(0)] uint32 _pad; [flag(NDR_REMAINING)] DATA_BLOB auth_info; } dcerpc_orphaned; typedef [public] struct { - [value(0)] uint32 _pad; [flag(NDR_REMAINING)] DATA_BLOB auth_info; } dcerpc_co_cancel; diff --git a/selftest/knownfail.d/samba4.local.ndr.dcerpc b/selftest/knownfail.d/samba4.local.ndr.dcerpc deleted file mode 100644 index 9ba0e94617d4..000000000000 --- a/selftest/knownfail.d/samba4.local.ndr.dcerpc +++ /dev/null @@ -1,4 +0,0 @@ -samba4.local.ndr.dcerpc.co_cancel.ncacn_packet_VALIDATE.ncacn_packet_co_cancel_check -samba4.local.ndr.dcerpc.orphaned.ncacn_packet_VALIDATE.ncacn_packet_orphaned_check -samba4.local.ndr.system.iconv.dcerpc.co_cancel.ncacn_packet_VALIDATE.ncacn_packet_co_cancel_check -samba4.local.ndr.system.iconv.dcerpc.orphaned.ncacn_packet_VALIDATE.ncacn_packet_orphaned_check -- 2.34.1