Created attachment 1905562 [details] fix as a file Created attachment 1905562 [details] fix as a file Hello Seems that cups-lspp.patch has broken getting and setting of file contexts. A file descriptor is opened, then it tries to get/set SELinux context of file by its path while the file descriptor has not been closed yet. Also, original CUPS code before the added check for (con->file > 0) seems to not exit from the function if the descriptor does not exist, such check seems to be needed, otherwise CUPS returns authentication failor by this code from the patch when trying to open the printing dialog in LibreOffice. Here is a fix: diff --git a/scheduler/client.c b/scheduler/client.c index 37626e3f5..4e0f4e97d 100644 --- a/scheduler/client.c +++ b/scheduler/client.c @@ -2007,9 +2007,9 @@ cupsdReadClient(cupsd_client_t *con) /* I - Client to read from */ } #ifdef WITH_LSPP - if (strncmp(con->scon, UNKNOWN_SL, strlen(UNKNOWN_SL)) != 0) + if ( (con->file > 0) && (strncmp(con->scon, UNKNOWN_SL, strlen(UNKNOWN_SL)) != 0) ) { - if (getfilecon(con->filename, &spoolcon) == -1) + if (fgetfilecon(con->file, &spoolcon) == -1) { cupsdSendError(con, HTTP_STATUS_SERVER_ERROR, CUPSD_AUTH_NONE); cupsdCloseClient(con); diff --git a/scheduler/job.c b/scheduler/job.c index 4c9486082..7cdf017a9 100644 --- a/scheduler/job.c +++ b/scheduler/job.c @@ -2388,7 +2388,7 @@ cupsdSaveJob(cupsd_job_t *job) /* I - Job */ #ifdef WITH_LSPP if (job->scon && strncmp(job->scon, UNKNOWN_SL, strlen(UNKNOWN_SL)) != 0) { - if (getfilecon(filename, &spoolcon) == -1) + if (fgetfilecon(fp, &spoolcon) == -1) { cupsdLogJob(job, CUPSD_LOG_ERROR, "Unable to get context of job control file \"%s\" - %s.", Signed-off-by: Mikhail Novosyolov <m.novosyolov>
Check for $uname being "Linux" in *.m4 in that patch is always false (but I checked not on Fedora), I removed that check.
Created attachment 1905569 [details] patch v2 One more check for valid file descriptor is needed Patch v2: diff --git a/scheduler/client.c b/scheduler/client.c index 37626e3f5..4e0f4e97d 100644 --- a/scheduler/client.c +++ b/scheduler/client.c @@ -2007,9 +2007,9 @@ cupsdReadClient(cupsd_client_t *con) /* I - Client to read from */ } #ifdef WITH_LSPP - if (strncmp(con->scon, UNKNOWN_SL, strlen(UNKNOWN_SL)) != 0) + if ( (con->file > 0) && (strncmp(con->scon, UNKNOWN_SL, strlen(UNKNOWN_SL)) != 0) ) { - if (getfilecon(con->filename, &spoolcon) == -1) + if (fgetfilecon(con->file, &spoolcon) == -1) { cupsdSendError(con, HTTP_STATUS_SERVER_ERROR, CUPSD_AUTH_NONE); cupsdCloseClient(con); diff --git a/scheduler/job.c b/scheduler/job.c index 4c9486082..c96329804 100644 --- a/scheduler/job.c +++ b/scheduler/job.c @@ -2386,9 +2386,9 @@ cupsdSaveJob(cupsd_job_t *job) /* I - Job */ fchown(cupsFileNumber(fp), RunUser, Group); #ifdef WITH_LSPP - if (job->scon && strncmp(job->scon, UNKNOWN_SL, strlen(UNKNOWN_SL)) != 0) + if ((fp > 0) && job->scon && strncmp(job->scon, UNKNOWN_SL, strlen(UNKNOWN_SL)) != 0) { - if (getfilecon(filename, &spoolcon) == -1) + if (fgetfilecon(fp, &spoolcon) == -1) { cupsdLogJob(job, CUPSD_LOG_ERROR, "Unable to get context of job control file \"%s\" - %s.",
Created attachment 1905611 [details] patch v3 Sorry, I hurried up and made some mistakes. Patch v3: diff --git a/scheduler/client.c b/scheduler/client.c index 37626e3f5..4e0f4e97d 100644 --- a/scheduler/client.c +++ b/scheduler/client.c @@ -2007,9 +2007,9 @@ cupsdReadClient(cupsd_client_t *con) /* I - Client to read from */ } #ifdef WITH_LSPP - if (strncmp(con->scon, UNKNOWN_SL, strlen(UNKNOWN_SL)) != 0) + if ( (con->file > 0) && (strncmp(con->scon, UNKNOWN_SL, strlen(UNKNOWN_SL)) != 0) ) { - if (getfilecon(con->filename, &spoolcon) == -1) + if (fgetfilecon(con->file, &spoolcon) == -1) { cupsdSendError(con, HTTP_STATUS_SERVER_ERROR, CUPSD_AUTH_NONE); cupsdCloseClient(con); diff --git a/scheduler/job.c b/scheduler/job.c index 4c9486082..6cec07c1f 100644 --- a/scheduler/job.c +++ b/scheduler/job.c @@ -2355,6 +2355,7 @@ cupsdSaveJob(cupsd_job_t *job) /* I - Job */ { char filename[1024]; /* Job control filename */ cups_file_t *fp; /* Job file */ + int jfd; /* Job file descriptor */ #ifdef WITH_LSPP security_context_t spoolcon; /* context of the job control file */ context_t jobcon; /* contex_t container for job->scon */ @@ -2383,12 +2384,13 @@ cupsdSaveJob(cupsd_job_t *job) /* I - Job */ if ((fp = cupsdCreateConfFile(filename, ConfigFilePerm & 0600)) == NULL) return; + jfd = cupsFileNumber(fp); fchown(cupsFileNumber(fp), RunUser, Group); #ifdef WITH_LSPP - if (job->scon && strncmp(job->scon, UNKNOWN_SL, strlen(UNKNOWN_SL)) != 0) + if ((jfd > 0) && job->scon && strncmp(job->scon, UNKNOWN_SL, strlen(UNKNOWN_SL)) != 0) { - if (getfilecon(filename, &spoolcon) == -1) + if (fgetfilecon(jfd, &spoolcon) == -1) { cupsdLogJob(job, CUPSD_LOG_ERROR, "Unable to get context of job control file \"%s\" - %s.", @@ -2441,7 +2443,7 @@ cupsdSaveJob(cupsd_job_t *job) /* I - Job */ } free(jobrange_copy); } - if (setfilecon(filename, context_str(tmpcon)) == -1) + if (fsetfilecon(jfd, context_str(tmpcon)) == -1) { cupsdLogJob(job, CUPSD_LOG_ERROR, "Unable to set context of job control file \"%s\" - %s.",
Also I suggest to log SELinux contexts at standard log level: diff --git a/scheduler/ipp.c b/scheduler/ipp.c index 64b190780..bf38942e3 100644 --- a/scheduler/ipp.c +++ b/scheduler/ipp.c @@ -3919,7 +3919,7 @@ check_context(cupsd_client_t *con, /* I - Client connection */ "Error while determining SELinux enforcement"); return -1; } - cupsdLogJob(job, CUPSD_LOG_DEBUG, + cupsdLogJob(job, CUPSD_LOG_INFO, "check_context: client context %s job context %s", con->scon, job->scon); diff --git a/scheduler/job.c b/scheduler/job.c index 4c9486082..61c6b1669 100644 --- a/scheduler/job.c +++ b/scheduler/job.c @@ -2450,7 +2452,7 @@ cupsdSaveJob(cupsd_job_t *job) /* I - Job */ context_free(jobcon); return; } - cupsdLogJob(job, CUPSD_LOG_DEBUG2, "New spool file context=%s", + cupsdLogJob(job, CUPSD_LOG_INFO, "New spool file context=%s", context_str(tmpcon)); context_free(tmpcon); context_free(jobcon); @@ -5236,7 +5238,7 @@ start_job(cupsd_job_t *job, /* I - Job ID */ return ; } printercon = context_new(devcon); - cupsdLogJob(job, CUPSD_LOG_DEBUG, + cupsdLogJob(job, CUPSD_LOG_INFO, "Printer context %s client context %s", context_str(printercon), job->scon); context_free(printercon);
Hi Mikhail, thank you for reporting the issue and the patch! It would be great if you told me what is now broken (meaning where I can see the problem) and how to reproduce the problem, so I can try to write automatic test which will be run in Fedora CI, so we could catch any possible breakages in the future. Thank you in advance!
(In reply to Zdenek Dohnal from comment #5) > Hi Mikhail, > > thank you for reporting the issue and the patch! > > It would be great if you told me what is now broken (meaning where I can see > the problem) and how to reproduce the problem, so I can try to write > automatic test which will be run in Fedora CI, so we could catch any > possible breakages in the future. > > Thank you in advance! That's an interesting question. I have not found any docs about how to use features introduced by this patch. Maybe Red Hat has some internal docs about how it is recommened to be used. I have not tried to run this on Fedora, I am running this on ROSA Linux, where this patch is borrowed from Fedora. Based on its code, the setup on Fedora/RHEL must be the following: 1) enable SELinux MLS, but boot in permissive (enforcing=0 in kernel cmdline or set permissive in /etc/selinux/config) 2) make is_lspp_config() from the patch return 1 (true), _probably_ by adding Classification mls into /etc/cups/cupsd.conf 3) add PerPageLabels No into /etc/cups/cups-files.conf (I do not know how labels are intended to work) 4) create a new Linux user, e.g. "user" 5) probably add it into the "lp" group (or somehow else allow printing to him) 6) assign possible SELinux MLS contexts to him: # semanage login -m -s user_u -r s0-s3:c1,c3 user 7) now somehow login under this user with MLS context set, probably pam_selinux will set some context, check it by running: $ id -Z 8) set "LogLevel debug" in /etc/cups/cupsd.conf, systemctl restart cups 9) add a printer or: # dnf install boomaga It will try to add a virtual printer. 10) see /var/log/cups/error_log My version of this patch is here: https://abf.io/import/cups/blob/rosa2019.05/cups-2.2.10-lspp.patch It has the following differences against the one in Fedora/RHEL: 1) is_lspp_config() returns true without editing cupsd.conf 2) PerPageLabels VALUE(TRUE) => VALUE(FALSE) 3) code getting client UID, AUID etc. by client_pid_to_auid() is disabled (#if 0) because it works extrememly unstably: often returns an error probably bacuase PID already does not exists, or, when installing boomaga package, it thinks that client PID is 0 (why?!), but /proc/0/loginuid does not exist and it makes the connection to CUPS fail. So I offed it and log only the SELinux contexts. 4) CUPSD_LOG_DEBUG is changed to CUPSD_LOG_INFO in some places.
ALT Linux also edits this patch: https://git.altlinux.org/gears/c/cups.git?p=cups.git;a=tree;f=patches;h=02240f0e8a911e7de6f1dd1398eea63ed9715e3a;hb=HEAD
> 3) code getting client UID, AUID etc. by client_pid_to_auid() is disabled (#if 0) Restored it. Simply disabling that code breaks other code. In general this does work. But in some cases fails strangely...
cups-lspp.patch also breaks opening http://localhost:631: it tries to read /proc/0/loginuid, which does not exist, and fails the connection with error "Unable to determine client auid for client pid=0". A workaround is the following: --- a/scheduler/client.c +++ b/scheduler/client.c @@ -287,12 +287,9 @@ */ if ((con->auid = client_pid_to_auid(cr.pid)) == -1) { - httpClose(con->http); - cupsdLogClient(con, CUPSD_LOG_ERROR, + cupsdLogClient(con, CUPSD_LOG_INFO, "Unable to determine client auid for client pid=%d", cr.pid); - free(con); - return; } cupsdLogClient(con, CUPSD_LOG_INFO, "peer's pid=%d, uid=%d, gid=%d, auid=%d", @@ -308,10 +308,8 @@ */ if (getpeercon(httpGetFd(con->http), &con->scon)) { - httpClose(con->http); + cupsdSetString(&con->scon, UNKNOWN_SL); cupsdLogClient(con, CUPSD_LOG_ERROR, "getpeercon() failed"); - free(con); - return; } cupsdLogClient(con, CUPSD_LOG_INFO, "client context=%s", con->scon); ALT Linux does this (https://git.altlinux.org/gears/c/cups.git?p=cups.git;a=blob;f=patches/ALT-lspp-context-via-tcp.patch;h=880311d5143a13d01cbc67b0f08c548354345b57;hb=HEAD) With my patch v3 (in comment#4) and this LSPP patch becomes usable for auditing (did not try labelling).
This bug appears to have been reported against 'rawhide' during the Fedora Linux 38 development cycle. Changing version to 38.
This bug appears to have been reported against 'rawhide' during the Fedora Linux 39 development cycle. Changing version to 39.
To be honest I'm not sure how you compiled CUPS with this patch - selinux/flask.h is for 3 years removed from libselinux, so CUPS fails to build from source once we build with WITH_LSPP - I will need to rewrite the patch to the current libselinux before I apply your patch.
(In reply to Zdenek Dohnal from comment #12) > To be honest I'm not sure how you compiled CUPS with this patch - > selinux/flask.h is for 3 years removed from libselinux, so CUPS fails to > build from source once we build with WITH_LSPP - I will need to rewrite the > patch to the current libselinux before I apply your patch. libselinux-2.9 was used
This bug appears to have been reported against 'rawhide' during the Fedora Linux 40 development cycle. Changing version to 40.