Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix driverless printing on Pantum M7300FDW #826

Closed
wants to merge 1 commit into from

Conversation

alexpevzner
Copy link
Member

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 know to have this bug in firmware.

So if we have not seen IPP_TAG_MEMBERNAME while decoding the current message, we allow sender to violate this rule and threat attribute name as member name.

It fixes driverless printing on Pantum M7300FDW.

Please note, this change breaks bad_collection test in the testipp.c, because now it decodes. So I've slightly modified that test, adding IPP_TAG_MEMBERNAME to the first attribute in the collection - it disactivates the workaround, making sure that bad_collection will not be decoded, as expected.

I've also added to the test a real-life example of Pantum M7300FDW response to the Get-Printer-Attributes request which now decodes.

The similar patches already added to https://github.com/OpenPrinting/goipp and https://github.com/OpenPrinting/ipp-usb so with all this stuff taken together, the device becomes fully-functional in the driverless mode (printing/scanning, network/USB, things just work).

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.

So if we have not seen IPP_TAG_MEMBERNAME while decoding the current
message, we allow sender to violate this rule and treat attribute
name as member name.

It fixes driverless printing on Pantum M7300FDW.
@alexpevzner
Copy link
Member Author

@michaelrsweet, @tillkamppeter, please review when time allows.

@zdohnal, JFYI.

@michaelrsweet
Copy link
Member

NO.

We cannot support broken printers like this without exposing CUPS to known security attacks. The code enforces the proper syntax for a reason, and we will not break CUPS for the sake of this printer.

@michaelrsweet michaelrsweet self-assigned this Nov 15, 2023
@michaelrsweet michaelrsweet added the wontfix This will not be worked on label Nov 15, 2023
@alexpevzner
Copy link
Member Author

Hi @michaelrsweet,

Frankly speaking, I'm a little concerned about how quickly and abruptly this decision was made. Without discussion, without anything...

Let's be practical. Pantum printers become popular across the world. It is not some kind of exotic, unknown and rarely used hardware.

Yes, this is a broken printer firmware. However, we already has a common practice to support broken firmware where possible. CUPS quirks for USB devices, ipp-usb, sane-airscan contain a lot of kludges to support broken hardware. It doesn't make us, engineers, happy, but it makes our users happy.

After all, this is consistent with the Postel's principle to "be conservative in what you send, be liberal in what you accept".

And honestly speaking, I don't see any security risk introduced here. I understand that syntax is enforced purposely and would be glad to somehow limit the applicability of this relaxation only to these broken Pantum devices, but at this place there is no indication what device we are speaking with.

Also, I'd like to receive some feedback from distro maintainers. @zdohnal, @tillkamppeter, WDYT?

@alexpevzner alexpevzner reopened this Nov 15, 2023
@michaelrsweet
Copy link
Member

We have already fixed multiple security vulnerabilities in this code, specifically to do with invalid IPP messages from printers. It causes erratic behaviour, crashes, and disclosure of information. So no matter how “popular” a printer might be, we are not allowing obviously bad data to open up security holes in CUPS. If it is actually so popular, the vendor can fix their printer firmware!

@OpenPrinting OpenPrinting locked as resolved and limited conversation to collaborators Nov 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants