From: Guy Harris <gharris@sonic.net>
Date: Tue, 10 Jan 2023 08:09:44 +0000 (-0800)
Subject: NPF: update handling of failure to set promiscuous mode.
X-Git-Tag: libpcap-1.10.3~4
X-Git-Url: https://git.tcpdump.org/libpcap/commitdiff_plain/16de5e6b4c24de40d3ae5e2cd000b57fe5e51f19

NPF: update handling of failure to set promiscuous mode.

Change comments and #defines to reflect more information we now know.

(cherry picked from commit b98941d35b20b01a1de58701a194bda6077d4914)
---

diff --git a/CHANGES b/CHANGES
index 9c4d665d..63d22e5f 100644
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,8 @@ Monthday, Month DD, YYYY:
       Sort the PUBHDR variable in Makefile.in in "ls" order.
       Fix typo in comment in pflog.h.
       Remove two no-longer-present files from .gitignore.
+      Update code and comments for handling failure to set promiscuous
+        mode based on new information.
     Building and testing:
       install: Fixed not to install the non-public pcap-util.h header.
       pcap-config: add a --version flag.
diff --git a/pcap-npf.c b/pcap-npf.c
index c9e5195d..c531a393 100644
--- a/pcap-npf.c
+++ b/pcap-npf.c
@@ -985,8 +985,6 @@ pcap_breakloop_npf(pcap_t *p)
 }
 
 /*
- * Vendor-specific error codes.
- *
  * These are NTSTATUS values:
  *
  *    https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-erref/87fba13e-bf06-450e-83b1-9241dc81e781
@@ -995,15 +993,34 @@ pcap_breakloop_npf(pcap_t *p)
  * mapped to Windows error values in userland; they're returned by
  * GetLastError().
  *
+ * Note that "driver" here includes the Npcap NPF driver, as various
+ * versions would take NT status values and set the "Customer" bit
+ * before returning the status code.  The commit message for the
+ * change that started doing that is
+ *
+ *    Returned a customer-defined NTSTATUS in OID requests to avoid
+ *    NTSTATUS-to-Win32 Error code translation.
+ *
+ * but I don't know why the goal was to avoid that translation.
+ *
  * Attempting to set non-promiscuous mode on a Microsoft Surface Pro's
- * Mobile Broadband Adapter returns an error; that error can safely be
- * ignored, as it's always in non-promiscuous mode.
+ * Mobile Broadband Adapter returns an error that appears to be
+ * NDIS_STATUS_NOT_SUPPORTED ORed with the "Customer" bit, so it's
+ * probably indicating that it doesn't support promiscuous mode,
+ * as one might expect, given that it's not going to promiscuously
+ * snoop for arbitrary mobile telecom network packets.  That error can
+ * safely be ignored, as it's always in non-promiscuous mode; an
+ * alternative would be to report the PCAP_WARNING_PROMISC_NOTSUP
+ * warning, which indicates that the activate call succeeded but
+ * that something happened that the user might want to know about.
  *
  * It is likely that there are other devices which throw spurious errors,
  * at which point this will need refactoring to efficiently check against
- * a list, but for now we can just check this one value.
+ * a list, but for now we can just check this one value.  Perhaps the
+ * right way to do this is compare against various NDIS errors with
+ * the "customer" bit ORed in.
  */
-#define NPF_SURFACE_MOBILE_NONPROMISC	0xe00000bb
+#define NT_STATUS_CUSTOMER_DEFINED	0x20000000
 
 static int
 pcap_activate_npf(pcap_t *p)
@@ -1295,6 +1312,13 @@ pcap_activate_npf(pcap_t *p)
 			 * Suppress spurious error generated by non-compiant
 			 * MS Surface mobile adapters.
 			 *
+			 * It appears to be reporting STATUS_NOT_SUPPORTED
+			 * (ndis.h defines NDIS_STATUS_NOT_SUPPORTED to
+			 * have the same value as the NT status value
+			 * STATUS_NOT_SUPPORTED), but with the NT status
+			 * value "Customer" bit set, probably by the
+			 * Npcap NPF driver.
+			 *
 			 * If we knew that this meant "promiscuous mode
 			 * isn't supported", we could add a "promiscuous
 			 * mode isn't supported" error code and return
@@ -1315,8 +1339,17 @@ pcap_activate_npf(pcap_t *p)
 			 * and rejecting it with an error could disrupt
 			 * attempts to capture, as many programs (tcpdump,
 			 * *shark) default to promiscuous mode.
+			 *
+			 * Alternatively, we could return the "promiscuous
+			 * mode not supported" *warning* value, so that
+			 * correct code will either ignore it or report
+			 * it and continue capturing.  (This may require
+			 * a pcap_init() flag to request that return
+			 * value, so that old incorrect programs that
+			 * assume a non-zero return from pcap_activate()
+			 * is an error don't break.)
 			 */
-			if (errcode != NPF_SURFACE_MOBILE_NONPROMISC)
+			if (errcode != (NDIS_STATUS_NOT_SUPPORTED|NT_STATUS_CUSTOMER_DEFINED))
 			{
 				pcap_fmt_errmsg_for_win32_err(p->errbuf,
 				    PCAP_ERRBUF_SIZE, errcode,