Attachment 18036 Details for Bug 15446 – Patches for v4-17-test

[patch] Patches for v4-17-test

bfixes-tmp417.txt (text/plain), 12.28 KB, created by Stefan Metzmacher on 2023-08-08 09:06:25 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2023-08-08 09:06:25 UTC

Size: 12.28 KB

patch

obsolete

>From 85983834f3c8c8def6280684ca8edb92830d63ee Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>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 <metze@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>(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 <http://www.gnu.org/licenses/>.
>+*/
>+
>+#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 e97dabb3a22e4b421087eef326a539e5d3e736a1 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>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 <metze@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>(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 e5b1609bbf30856fc382a8f0c3674743e436f5f5 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>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 <metze@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>
>Autobuild-User(master): Volker Lendecke <vl@samba.org>
>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
>

Flags:

vl: review+

Actions: View

Attachments on bug 15446: 18034 | 18035 | 18036