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 9c4d665d6b933a8251b0aed137b800bc9fe76f42..63d22e5f3df43564fc374cabcd653c5b60eabb81 100644 (file)
--- 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 c9e5195d2f4c72d6af970db1f55888a43e3ae353..c531a39365a7e89fc3aa952041b3eff329bc2fda 100644 (file)
--- 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,