Pantum M7300FDW

Pantum M7300FDW

[Alexander Pevzner]

[-- Attachment #1: Type: text/plain, Size: 3742 bytes --]

Hi, everybody!

Some time ago, I initiated a discussion regarding workarounds for Pantum 
M7300FDW and similar devices.

I would like to remind you that this particular device violates the IPP 
protocol by using named attributes within collections instead of 
utilizing IPP_TAG_MEMBERNAME.

Previously, I proposed a patch to CUPS that allowed it to accept such 
malformed messages. However, the patch was promptly rejected 
(https://github.com/OpenPrinting/cups/pull/826). Therefore, I have 
decided to bring this matter up for discussion here.

This printer is listed as certified/compatible at Mopria site, at the 
Apple list of compatible printers an in the IPP.Everwhere list of 
compatible printers.

It's firmware is dated 2016, and there is no firmware updates since that 
time. It is worth noting that the  CUPS IPP parser was strengthened in 
2019 (https://github.com/apple/cups/issues/5630), suggesting that the 
printer may have received its certification before this enhancement.

Despite its older firmware release date, this printer is still being 
sold in stores.

I've performed several attempts to connect Pantum representatives, using 
both official support channel and my own unofficial channel, but didn't 
succeed.

There are probably other Pantum models that have the similar defect, but 
I don't have an exact list. Also, according to Zdenek, there is some 
Canon model with the similar defect.

During the previous discussion I was asked several questions, so now 
I've performed some investigations and now ready to answer.

The Windows 11 IPP class driver doesn't print on this device.

Also, Mopria Print for Android doesn't work too.

Pantum provides drivers for Linux, and with these drivers printer works. 
Under the hood this driver uses IPP protocol with "extensions" similar 
to whatever I purpose.

This case is important for us, because here in Moscow we have thousands 
of computer classes in all Moscow public schools with thousands of 
not-so-qualified users (school teachers), all moving to Linux, so "make 
printing just work" becomes really essential for us.

Please note that I have revisited my patch since our last discussion. 
The updated version is more precise, affecting only IPP response parsing 
when CUPS communicates with the hardware printer. IPP certification 
tools and requests sent to the CUPS server remain unaffected.

While I am not rushing to create a new PR at the moment, I would 
appreciate it if someone could review the attached patch.

If a patch like this is deemed unacceptable, please provide guidance on 
addressing such problems. My objective remains unchanged: maintaining 
the quality and security of the existing CUPS implementation while 
enabling sometimes non-conforming hardware to function out of the box.

t is worth mentioning that my patch has been tested on our range of 
printers and functions effectively.

P.S. I observed that my patch to ipp-usb, which enables devices with 
similar issues, was not accepted by RedHat.

Zdenek, if you choose to pursue this route, it may be safer to blacklist 
this device in your ipp-usb build. Otherwise, ipp-usb may hold the 
device while still providing eSCL but preventing access by proprietary 
printing drivers.

In the latest version of ipp-usb, I have transformed this workaround 
into a quirk with three options: "reject" - the old behavior, "accept" - 
akin to 0.99.24 behavior, and "sanitize" - ipp-usb rectifies all broken 
IPP responses. With the "sanitize" option, this device prints over USB 
using unmodified CUPS.

Nevertheless, I am eager for these modifications to be implemented as 
they resolve network printing issues that ipp-usb cannot address.

-- 

	Wishes, Alexander Pevzner (pzz@apevzner.com)

[-- Attachment #2: pantum-m7300fdw.patch --]
[-- Type: text/x-patch, Size: 10395 bytes --]

diff --git a/backend/ipp.c b/backend/ipp.c
index 2bbdefbd1..f1d5315f3 100644
--- a/backend/ipp.c
+++ b/backend/ipp.c
@@ -678,6 +678,7 @@ main(int  argc,				/* I - Number of command-line args */
   http = httpConnect2(hostname, port, addrlist, AF_UNSPEC, cupsEncryption(), 1,
                       0, NULL);
   httpSetTimeout(http, 30.0, timeout_cb, NULL);
+  httpSetIppResponseFlags(http, IPP_FLG_WITH_HW_WORKAROUNDS);
 
  /*
   * See if the printer supports SNMP...
@@ -2542,6 +2543,7 @@ monitor_printer(
   http = httpConnect2(monitor->hostname, monitor->port, NULL, AF_UNSPEC,
                       monitor->encryption, 1, 0, NULL);
   httpSetTimeout(http, 30.0, timeout_cb, NULL);
+  httpSetIppResponseFlags(http, IPP_FLG_WITH_HW_WORKAROUNDS);
   if (username[0])
     cupsSetUser(username);
 
diff --git a/cups/dest.c b/cups/dest.c
index bedd4e170..141e5c857 100644
--- a/cups/dest.c
+++ b/cups/dest.c
@@ -726,6 +726,9 @@ cupsConnectDest(
     encryption = HTTP_ENCRYPTION_IF_REQUESTED;
 
   http = httpConnect2(hostname, port, addrlist, AF_UNSPEC, encryption, 1, 0, NULL);
+  if (flags & CUPS_DEST_FLAGS_DEVICE)
+    httpSetIppResponseFlags(http, IPP_FLG_WITH_HW_WORKAROUNDS);
+
   httpAddrFreeList(addrlist);
 
  /*
diff --git a/cups/http-private.h b/cups/http-private.h
index 5f77b8ef0..bc73044d0 100644
--- a/cups/http-private.h
+++ b/cups/http-private.h
@@ -307,6 +307,9 @@ struct _http_s				/**** HTTP connection structure ****/
 					/* Allocated field values */
   			*default_fields[HTTP_FIELD_MAX];
 					/* Default field values, if any */
+
+  /**** New in CUPS 2.4 *****/
+  unsigned              ipp_rsp_flags;	/* IPP_FLG_* for parsing IPP responses */
 };
 #  endif /* !_HTTP_NO_PRIVATE */
 
diff --git a/cups/http.c b/cups/http.c
index 198087f27..178801d73 100644
--- a/cups/http.c
+++ b/cups/http.c
@@ -2652,6 +2652,37 @@ httpSetKeepAlive(
     http->keep_alive = keep_alive;
 }
 
+/*
+ * 'httpIppResponseFlags()' - Get IPP_FLG_* flags for parsing IPP responses
+ * received from this HTTP connection
+ *
+ * @since CUPS 2.4@
+ */
+unsigned				/* IPP_FLG_* flags for IPP responses */
+httpIppResponseFlags(
+    http_t           *http)		/* I - HTTP connection */
+{
+  if (http)
+    return http->ipp_rsp_flags;
+
+  return 0;
+}
+
+/*
+ * 'httpSetIppResponseFlags' - Set IPP_FLG_* flags for parsing IPP responses
+ * received from this HTTP connection
+ *
+ * @since CUPS 2.4@
+ */
+void
+httpSetIppResponseFlags(
+    http_t           *http,		/* I - HTTP connection */
+    unsigned         flags)		/* IPP_FLG_* flags for IPP responses */
+{
+  if (http)
+    http->ipp_rsp_flags = flags;
+}
+
 
 /*
  * 'httpSetLength()' - Set the content-length and content-encoding.
diff --git a/cups/http.h b/cups/http.h
index fbca93f2f..e9b36ad46 100644
--- a/cups/http.h
+++ b/cups/http.h
@@ -603,6 +603,10 @@ extern void		httpShutdown(http_t *http) _CUPS_API_2_0;
 extern const char	*httpStateString(http_state_t state) _CUPS_API_2_0;
 extern const char	*httpURIStatusString(http_uri_status_t status) _CUPS_API_2_0;
 
+/* New in CUPS 2.4 */
+extern unsigned		httpIppResponseFlags(http_t *http) _CUPS_API_2_4;
+extern void		httpSetIppResponseFlags(http_t *http, unsigned flags) _CUPS_API_2_4;
+
 /*
  * C++ magic...
  */
diff --git a/cups/ipp-private.h b/cups/ipp-private.h
index b3902f05b..691bc156e 100644
--- a/cups/ipp-private.h
+++ b/cups/ipp-private.h
@@ -121,6 +121,7 @@ struct _ipp_attribute_s			/**** IPP attribute ****/
 
 struct _ipp_s				/**** IPP Request/Response/Notification ****/
 {
+  unsigned              flags;          /* IPP_FLG_* flags */
   ipp_state_t		state;		/* State of request */
   _ipp_request_t	request;	/* Request header */
   ipp_attribute_t	*attrs;		/* Attributes */
diff --git a/cups/ipp-support.c b/cups/ipp-support.c
index b0cbadf8d..c094d5aca 100644
--- a/cups/ipp-support.c
+++ b/cups/ipp-support.c
@@ -2432,7 +2432,6 @@ ippSetPort(int p)			/* I - Port number to use */
   _cupsGlobals()->ipp_port = p;
 }
 
-
 /*
  * 'ippStateString()' - Return the name corresponding to a state value.
  *
diff --git a/cups/ipp.c b/cups/ipp.c
index 59b04aa1d..4794081cc 100644
--- a/cups/ipp.c
+++ b/cups/ipp.c
@@ -2609,9 +2609,17 @@ ippNextAttribute(ipp_t *ipp)		/* I - IPP message */
 /*
  * 'ippNew()' - Allocate a new IPP message.
  */
-
 ipp_t *					/* O - New IPP message */
 ippNew(void)
+{
+    return ippNewFlg(0);
+}
+
+/*
+ * 'ippNew()' - Allocate a new IPP message with flags
+ */
+ipp_t *					/* O - New IPP message */
+ippNewFlg(unsigned flags)		/* I - IPP_FLG_* flags */
 {
   ipp_t			*temp;		/* New IPP message */
   _cups_globals_t	*cg = _cupsGlobals();
@@ -2631,6 +2639,7 @@ ippNew(void)
     if (cg->server_version == 0)
       _cupsSetDefaults();
 
+    temp->flags = flags;
     temp->request.any.version[0] = (ipp_uchar_t)(cg->server_version / 10);
     temp->request.any.version[1] = (ipp_uchar_t)(cg->server_version % 10);
     temp->use                    = 1;
@@ -2654,6 +2663,16 @@ ippNew(void)
 
 ipp_t *					/* O - IPP request message */
 ippNewRequest(ipp_op_t op)		/* I - Operation code */
+{
+    return ippNewRequestFlg(op, 0);
+}
+
+/*
+ *  'ippNewRequestFlg()' - Allocate a new IPP request message with flags.
+ */
+ipp_t *					/* O - IPP request message */
+ippNewRequestFlg(ipp_op_t op,		/* I - Operation code */
+                 unsigned flags)	/* I - IPP_FLG_* flags */
 {
   ipp_t		*request;		/* IPP request message */
   cups_lang_t	*language;		/* Current language localization */
@@ -2668,7 +2687,7 @@ ippNewRequest(ipp_op_t op)		/* I - Operation code */
   * Create a new IPP message...
   */
 
-  if ((request = ippNew()) == NULL)
+  if ((request = ippNewFlg(flags)) == NULL)
     return (NULL);
 
  /*
@@ -2721,6 +2740,17 @@ ippNewRequest(ipp_op_t op)		/* I - Operation code */
 
 ipp_t *					/* O - IPP response message */
 ippNewResponse(ipp_t *request)		/* I - IPP request message */
+{
+    return ippNewResponseFlg(request, 0);
+}
+
+
+/*
+ * 'ippNewResponseFlg()' - Allocate a new IPP response message with flags.
+ */
+ipp_t *					/* O - IPP response message */
+ippNewResponseFlg(ipp_t *request,	/* I - IPP request message */
+                  unsigned flags)	/* I - IPP_FLG_* flags */
 {
   ipp_t			*response;	/* IPP response message */
   ipp_attribute_t	*attr;		/* Current attribute */
@@ -2737,7 +2767,7 @@ ippNewResponse(ipp_t *request)		/* I - IPP request message */
   * Create a new IPP message...
   */
 
-  if ((response = ippNew()) == NULL)
+  if ((response = ippNewFlg(flags)) == NULL)
     return (NULL);
 
  /*
@@ -3040,10 +3070,26 @@ ippReadIO(void       *src,		/* I - Data source */
 
           DEBUG_printf(("2ippReadIO: name length=%d", n));
 
-          if (n && parent)
+          if (n && parent && (ipp->flags & IPP_FLG_ALLOW_NAMED_MEMBERS) == 0)
           {
+	    /*
+	     * According to RFC 8010, attributes within collection must not have name.
+	     * Instead, name must be encoded as value of special dedicated attribute,
+	     * IPP_TAG_MEMBERNAME
+	     *
+	     * Unfortunately, some devices violate this specification and encode collection
+	     * member names as attribute names.
+	     *
+	     * Pantum M7300FDW is known to have this bug in firmware.
+	     *
+	     * Setting IPP_FLG_ALLOW_NAMED_MEMBERS in ippNewFlg, ippNewRequestFlg
+	     * and ippNewResponseFlg allows to violate this rule and treat attribute
+	     * name as member name.
+	     */
+
             _cupsSetError(IPP_STATUS_ERROR_INTERNAL, _("Invalid named IPP attribute in collection."), 1);
             DEBUG_puts("1ippReadIO: bad attribute name in collection.");
+            cups_debug_printf("1ippReadIO: bad attribute name in collection.");
 	    goto rollback;
           }
           else if (n == 0 && tag != IPP_TAG_MEMBERNAME && tag != IPP_TAG_END_COLLECTION)
@@ -3467,7 +3513,7 @@ ippReadIO(void       *src,		/* I - Data source */
 	        * Oh, boy, here comes a collection value, so read it...
 		*/
 
-                value->collection = ippNew();
+                value->collection = ippNewFlg(ipp->flags);
 
                 if (n > 0)
 		{
diff --git a/cups/ipp.h b/cups/ipp.h
index f7ab6f423..a2243f7df 100644
--- a/cups/ipp.h
+++ b/cups/ipp.h
@@ -937,6 +937,28 @@ extern int		ippValidateAttributes(ipp_t *ipp) _CUPS_API_1_7;
 /**** New in CUPS 2.0 ****/
 extern const char	*ippStateString(ipp_state_t state) _CUPS_API_2_0;
 
+/**** New in CUPS 2.4 ****/
+
+/* Flags for ippNewFlg, ippNewRequestFlg and ippNewResponseFlg
+ *
+ * With these flags IPP protocol parser allows some violations
+ * of the protocol specification. These flags intended for testing
+ * and implementing workarounds for firmware bugs
+ *
+ * Use wisely and with care!
+ */
+#  define IPP_FLG_ALLOW_NAMED_MEMBERS	0x01	/* Allow collection member
+						 * names to be encoded as
+						 * attribute name, not using
+						 * IPP_TAG_MEMBERNAME */
+
+#  define IPP_FLG_WITH_HW_WORKAROUNDS		IPP_FLG_ALLOW_NAMED_MEMBERS
+
+extern ipp_t		*ippNewFlg(unsigned flags) _CUPS_PUBLIC;
+extern ipp_t		*ippNewRequestFlg(ipp_op_t op,
+			                  unsigned flags) _CUPS_PUBLIC;
+extern ipp_t		*ippNewResponseFlg(ipp_t *request,
+			                  unsigned flags) _CUPS_PUBLIC;
 
 /*
  * C++ magic...
diff --git a/cups/ppd-cache.c b/cups/ppd-cache.c
index d664812a2..9ca4d7e91 100644
--- a/cups/ppd-cache.c
+++ b/cups/ppd-cache.c
@@ -5316,6 +5316,7 @@ cups_connect(http_t     **http,		/* IO - Current HTTP connection */
   {
     httpClose(*http);
     *http = httpConnect2(host, port, NULL, AF_UNSPEC, encryption, 1, 5000, NULL);
+    httpSetIppResponseFlags(*http, IPP_FLG_WITH_HW_WORKAROUNDS);
   }
 
   return (*http != NULL);
diff --git a/cups/request.c b/cups/request.c
index 5baa512bb..5ad0a3ea0 100644
--- a/cups/request.c
+++ b/cups/request.c
@@ -383,7 +383,7 @@ cupsGetResponse(http_t     *http,	/* I - Connection to server or @code CUPS_HTTP
     * Get the IPP response...
     */
 
-    response = ippNew();
+    response = ippNewFlg(httpIppResponseFlags(http));
 
     while ((state = ippRead(http, response)) != IPP_STATE_DATA)
       if (state == IPP_STATE_ERROR)
diff --git a/scheduler/ipp.c b/scheduler/ipp.c
index e0caa98fe..4f49c5bee 100644
--- a/scheduler/ipp.c
+++ b/scheduler/ipp.c
@@ -5348,6 +5348,8 @@ create_local_bg_thread(
     return (NULL);
   }
 
+  httpSetIppResponseFlags(http, IPP_FLG_WITH_HW_WORKAROUNDS);
+
  /*
   * Query the printer for its capabilities...
   */

Re: Pantum M7300FDW

[Zdenek Dohnal]

Hi Alex!

On 3/15/24 15:27, Alexander Pevzner wrote:
>
> P.S. I observed that my patch to ipp-usb, which enables devices with 
> similar issues, was not accepted by RedHat.
Yes, I tried to remove the functionality based on security concerns that 
were raised here, but completely forgot about that I could (and have to) 
do that via quirks...
>
> Zdenek, if you choose to pursue this route, it may be safer to 
> blacklist this device in your ipp-usb build. Otherwise, ipp-usb may 
> hold the device while still providing eSCL but preventing access by 
> proprietary printing drivers.
Thanks, I totally forgot about this!
>
> In the latest version of ipp-usb, I have transformed this workaround 
> into a quirk with three options: "reject" - the old behavior, "accept" 
> - akin to 0.99.24 behavior, and "sanitize" - ipp-usb rectifies all 
> broken IPP responses. With the "sanitize" option, this device prints 
> over USB using unmodified CUPS.
That's great! To have a way how to configure this behavior is ideal from 
POV - we can ship the safe way as default (IMO sanitize option should 
insert message into the log - admin would like to know whether something 
is not correct, instead of silently workaround issue) and if the user 
has problems, they can switch the quirk anytime.


Zdenek

-- 
Zdenek Dohnal
Senior Software Engineer
Red Hat, BRQ-TPBC

Re: Pantum M7300FDW

[Alexander Pevzner]

Hi Zdenek!

On 3/19/24 16:25, Zdenek Dohnal wrote:
> That's great! To have a way how to configure this behavior is ideal from 
> POV - we can ship the safe way as default (IMO sanitize option should 
> insert message into the log - admin would like to know whether something 
> is not correct, instead of silently workaround issue) and if the user 
> has problems, they can switch the quirk anytime.

It actually writes messages to the log:

   IPP sanitize: 1511 bytes replaced with 1621 -- IPP message was fixed
   IPP sanitize: not needed - message is OK, no need to sanitize

When ipp-usb receives IPP response, it attempts to parse it with and 
without workarounds. If message can be parsed without workarounds, it is 
bypassed as is, byte by byte, without any modifications.

So I believe, this quirk is safe when enabled.

-- 

	Wishes, Alexander Pevzner (pzz@apevzner.com)

Re: Pantum M7300FDW

[Zdenek Dohnal]

On 3/19/24 14:37, Alexander Pevzner wrote:
> Hi Zdenek!
>
> On 3/19/24 16:25, Zdenek Dohnal wrote:
>> That's great! To have a way how to configure this behavior is ideal 
>> from POV - we can ship the safe way as default (IMO sanitize option 
>> should insert message into the log - admin would like to know whether 
>> something is not correct, instead of silently workaround issue) and 
>> if the user has problems, they can switch the quirk anytime.
>
> It actually writes messages to the log:
>
>   IPP sanitize: 1511 bytes replaced with 1621 -- IPP message was fixed
>   IPP sanitize: not needed - message is OK, no need to sanitize

I meant something around - "Printer XY sends invalid IPP response - 
please try to install the latest firmware. If the problem persists, 
report it to your printer manufacturer." . This should give a clear 
message something is problematic and can cause issues, plus hint what to do.


Zdenek

-- 
Zdenek Dohnal
Senior Software Engineer
Red Hat, BRQ-TPBC