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

lpd "reserve=rfc1179" actually uses ports 512-731, not 721-731 #743

Closed
bmasonrh opened this issue Jun 26, 2023 · 9 comments · Fixed by #746
Closed

lpd "reserve=rfc1179" actually uses ports 512-731, not 721-731 #743

bmasonrh opened this issue Jun 26, 2023 · 9 comments · Fixed by #746
Labels
bug Something isn't working priority-medium

Comments

@bmasonrh
Copy link
Contributor

While creating PR #741, I noticed that the use of cups_rresvport() will cause ports from 731-512 to be used instead of just 721-731. Since cups_rresvport() starts with the value of lport passed in and decrements lport to 512, if all ports from 731-721 are unavailable, cups_rresvport() will start binding to ports between 720-512.

One possible fix would be to add a parameter to cups_rresvport() to provide a lower bound for port reservation. This would make it incompatible with the glibc function that it was intended to replace, but I don't know if that's important now.

Another possible fix would be to change lpd_queue() to check the value of lport returned by cups_rresvport() and, if *lport < 721 && reserve == RESERVE_RFC117, close the fd and re-set lport to 731.

I'm happy to file a PR if this is something that should be addressed.

@bmasonrh
Copy link
Contributor Author

Excerpt from error_log generated after running:

lpadmin -p lpdtest -v 'lpd://192.168.122.1/lpd?reserve=RFC1179' -E

and quickly sending a couple hundred jobs to lpdtest.

error_log_excerpt.txt

@bmasonrh bmasonrh changed the title lpd "reserve=rfc1179" will actually use ports 512-731, not 721-731 lpd "reserve=rfc1179" actually uses ports 512-731, not 721-731 Jun 26, 2023
@zdohnal
Copy link
Member

zdohnal commented Jun 27, 2023

I would go for adding the parameter. The function is static for the backend, so no one else should use it, so having it 1on1 with original BSD resvport() is not needed. @michaelrsweet WDYT?

@michaelrsweet I see several users set LPD for accessing a Windows print server and we don't plan to bring LPD into future printing architecture (no support in PAPPL), so what is an expected path for such users which used LPD? Upgrade the Windows server and setup IPP+OAuth2? Because it looks to me they use LPD in case they don't want currently to setup IPP+Kerberos.

@zdohnal zdohnal added bug Something isn't working priority-medium labels Jun 27, 2023
@michaelrsweet
Copy link
Member

@zdohnal WRT cups_rresvport, go ahead and add a new parameter.

WRT LPD support in PAPPL, go ahead and file an enhancement request. But I'm inclined to support SMB over LPD to support legacy Windows servers since that also allows authentication to actually work.

@zdohnal
Copy link
Member

zdohnal commented Jun 28, 2023

@michaelrsweet I'm not sure whether SMB over LPD support in PAPPL would be something which Samba will use. I talked with Andreas Schneider, a member of Samba upstream, and they are considering creating a new Samba backend for IPP Everywhere, which will act as a printer application in our terminology.

I don't know much about what possibilities old Windows print servers offer - do you know of some other way such servers offer without Samba/OAuth2 in place and we have it supported in PAPPL? Or can I file RFE for plain LPD? (I saw some old MS servers with old IPP 1.1, but users had it together with Kerberos, so I'm not sure whether MS server enforces Kerberos with IPP...)

@zdohnal
Copy link
Member

zdohnal commented Jun 28, 2023

@bmasonrh so ack for a new parameter :) if you have time for implementing it, feel free to send PR. Otherwise I will get to it in the future.

@michaelrsweet
Copy link
Member

@zdohnal Like I said, file a RFE for adding plain LPD support. But longer term we'll need something that Windows supports out-of-the-box which means SMB for older systems and IPP for newer systems. From the Samba perspective we'd be calling out to the existing smbprint client program to talk to another Windows system, but never (intentionally) sending a print job to a Samba-based printer - what would be the point when the same system is already running CUPS?

@zdohnal
Copy link
Member

zdohnal commented Jun 28, 2023

@zdohnal Like I said, file a RFE for adding plain LPD support. But longer term we'll need something that Windows supports out-of-the-box which means SMB for older systems and IPP for newer systems.

Ok, let's stick with long term solutions - SMB and IPP.

From the Samba perspective we'd be calling out to the existing smbprint client program to talk to another Windows system, but never (intentionally) sending a print job to a Samba-based printer - what would be the point when the same system is already running CUPS?

I'm not sure if I understand (again, still sometimes my English fails...):

Where are we going to call smbprint from? (from PAPPL based printer app?/CUPS?)

Never sending a print job to a Samba-based printer? (sending a job from CUPS/PAPPL based printerapp/client app? sending it to Samba-based printer on our machine or the original printer on Windows which is shared by Samba?)

The same system is running CUPS? (here I would say you mean our machine, since Windows does not run CUPS...)

Probably my note about Samba upstream plans created confusion or I did miss something important - I thought CUPS 3.0 will talk with a printer application which provides Samba support, so all communication will go via this application (I don't know where smbprint program would take place in this...). And this I communicated with Samba - that they need a daemon/service which would put Samba printers on mDNS and enables IPP 2.0 protocol. Did I miss the meaning of that part of comment entirely? :D

Andreas' idea was that they can reuse their CUPS backend for Samba and uplift it to make it a printer application, so I'm not sure whether they would use PAPPL at all (thus whether it is worth to spend time to implementing Samba support to PAPPL, if Samba is not going to use it).

@michaelrsweet
Copy link
Member

@zdohnal

Where are we going to call smbprint from? (from PAPPL based printer app?/CUPS?)

PAPPL would execute smbprint to send a print job to a Windows server - the printer is hosted by the Windows system and the PAPPL-based printer application is sending it a print job (acting as the IPP Everywhere gateway/proxy).

There is no point in implementing the other way - CUPS already handles incoming IPP print requests and Samba already handles passing SMB print jobs through to CUPS.

@bmasonrh
Copy link
Contributor Author

@bmasonrh so ack for a new parameter :) if you have time for implementing it, feel free to send PR. Otherwise I will get to it in the future.

#746

zdohnal added a commit that referenced this issue Sep 13, 2023
In backend/lpd.c, if "reserve=rfc1179" is used in the Device URI, ports from 512-731 will be used instead of only 721=731 as per RFC 1179.

cups_rresvport() starts with the value of lport passed in to the function and decrements lport to 512 until an open port is found. Thus, if all ports from 731-721 are unavailable, cups_rresvport() will start binding to ports between 720-512.

This patch resolves this issue by adding a parameter to cups_rresvport() that defines the minimum port number that should be used and makes the appropriate changes in lpd_queue().

Resolves #743
zdohnal added a commit that referenced this issue Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority-medium
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants