Description of problem: lprm currently does not support MLS Version-Release number of selected component (if applicable): 1.2.4-2 How reproducible: Everytime Steps to Reproduce: 1. Enqueue a job in CUPS in MLS enforcing 2. lprm that job from another role 3. Note that the job was removed Actual results: The job can be removed from another role Expected results: Only the role that enqueued the job, and sysadm_r should be able to remove jobs Additional info: Please see attached patch as an updated cups-lspp.patch which, along with policy updates that are being sent to the maintainer, resolves this issue. *** The attached patch also obsoletes lspp-access so that needs to be removed from the spec file.
Created attachment 137462 [details] Updated cups-lspp.patch to address lprm issue
The earlier attached patch does handle roles correctly, but does not take into consideration the job's level. This needs to be addressed before the issue can be considered resolved.
Created attachment 137523 [details] Updated cups-lspp.patch which sets the mls levels correctly
Thanks for the updated patch. I took a read through it to check for any problems. This bit looks like it might leak some context_t objects on error. + tmpcon = context_new(spoolcon); + clicon = context_new(con->scon); + if (context_range_set(tmpcon, (context_range_get(clicon))) == -1) + { + cupsdSendError(con, HTTP_SERVER_ERROR); + return (cupsdCloseClient(con)); + } + if (setfilecon(con->filename, context_str(tmpcon)) == -1) + { + cupsdSendError(con, HTTP_SERVER_ERROR); + return (cupsdCloseClient(con)); + } + cupsdLogMessage(CUPSD_LOG_DEBUG2, "cupsdReadClient: %s set to %s", + con->filename, context_str(tmpcon)); + context_free(tmpcon); + context_free(clicon); Similarly in ipp.c with printercon which never gets freed as far as I can tell, and then later with jobcon and tmpcon. There are several places like this in the patch. There are also some getfilecon() calls without a matching freecon(). This addition seems to be stray: + int remote_job; /* Remote print job? */ There seems to be disagreement between different parts of the code about whether to strip 'file:/' or 'file://' from the URI.
Created attachment 137583 [details] Updated patch to address the issues raised by Tim
Created attachment 137595 [details] An else was mistakenly removed from the last patch The previous patch removed an else from printers.c which was needed when attempting to perform a getfilecon() on a file that was not currently on the system. Such as a FileDevice printer where the file does not exist.
Thanks! Applied in 1.2.4-5.
The previous patch did not check to make sure there was level information before setting the level of the spool files. Without that check all non-MLS connections would error, and not be allowed to print. I'm in the process of testing the updated patch now.
Created attachment 137672 [details] Ensure that the security context of a job or connection is always set The following patch ensures that when not running in an MLS mode the security context is set. Also it ensures that there is a valid security context before completing any actions which operate on it.
Matt, I'm going to have to pull this patch. It's crashing cupsd now because context_new("UNKNOWN SL") == 0 and there's no check on the result before it's used.
Program received signal SIGSEGV, Segmentation fault. [Switching to Thread -1208510240 (LWP 12431)] 0x001a2d67 in context_range_get () from /lib/libselinux.so.1 (gdb) up #1 0x00bad996 in cupsdReadClient (con=0x9da6cd8) at client.c:1873 1873 if (context_range_set(tmpcon, (context_range_get(clicon))) == -1) (gdb) info locals line = "POST /printers/deskjet HTTP/1.1", '\0' <repeats 2125 times>, "\n\000\000\000\200[��\030��\000\000\000\000\000�_�����\000\227_��\226[���c�����\000\227_��\226[��\001\004\000\000\000\000hoopoe.elk.\000lk.", '\0' <repeats 1010 times>, "hoopoe.elk.\000lk.", '\0' <repeats 650 times>, "d��\000\204b���g��\003\000\000\000xc��\204b��\f\000\000\000\204b�����\000\f\000\000\000yb��\200c��\223��\000yb�"... operation = "POST\000���!\203�\0008L�\t\234\030�\000\000\000\000\000\000\000\000\000\000�Y\035�\177�\000�����\t�\0008L�\t\000�Y\035�����0j\000�]�\t" version = "HTTP/1.1\000\237.\0006\001\000\0006\001\000\000t���W�\035\000\b�.\000\000�Y\035x����0j\000@M�\t���9\000�Y\035�\177�\000\000�Y\035" locale = "\000\000\000\000\000\020\000\000P�.\000@\231.\000�\237.\000 �.\000HY�\tH���0�!\000 �.\000��\033\000\220���PY�\t����!\203�\000��\035" major = 1 minor = 1 status = <value optimized out> ipp_state = <value optimized out> bytes = <value optimized out> filename = 0x0 buf = "P��\000\000��\000��\005\000x\234_\000�\177�\000PY�\t�\223�\000\210����n\201\000\000���\000roo�<`\000�<`\000�=`\000)f�\000\b\000\000\000PY�\t \000\000\000\000\004\000\000�r�\000�\027�\t\000\000\000\000ܽl\000x���t���p���l���HY�\t\000\000\000\000\b\000\000\000�Y�\t\234Y�\tdS�\t/var/run/cups/certs/0\000�\t\030S�\t,S�\t\004S�\tEn\237vY��\216^�\233�vyQG�Fʵ", '\0' <repeats 44 times>, "20"... ---Type <return> to continue, or q <return> to quit--- filestats = {st_dev = 0, __pad1 = 0, __st_ino = 0, st_mode = 0, st_nlink = 0, st_uid = 0, st_gid = 0, st_rdev = 0, __pad2 = 0, st_size = 0, st_blksize = 0, st_blocks = 0, st_atim = {tv_sec = 0, tv_nsec = 0}, st_mtim = {tv_sec = 0, tv_nsec = 0}, st_ctim = {tv_sec = 0, tv_nsec = 0}, st_ino = 0} type = <value optimized out> p = <value optimized out> spoolcon = 0x9da6c60 "\230l�\t:object_r:print_spool_t" clicon = (context_t) 0x0 tmpcon = (context_t) 0x9d8b678 request_id = 1
(In reply to comment #10) > Matt, I'm going to have to pull this patch. It's crashing cupsd now because > context_new("UNKNOWN SL") == 0 and there's no check on the result before it's used. Was that with patch id=137595 that you pulled into 1.2.4-5? This is the bug I was referencing when I re-opened the bug and sent patch id=137672. Please let me know if you are seeing this issue with the updated patch.
I see that bug with id=137672 as well. + clicon = context_new(con->scon); + tmpcon = context_new(spoolcon); + freecon(spoolcon); + if (context_range_set(tmpcon, (context_range_get(clicon))) == -1) clicon is 0x0 at this point, and con->scon was "UNKNOWN SL".
I can add the additional checks, but just before that section con->scon should have been compared to UNKNOWN_SL, and that code wouldn't have been executed.
Created attachment 137751 [details] Check that context_new returned non-null before proceeding The attached patch ensures that context_new returned a valid result before operating on the context_t. It also cleans up a compiler warning in client.c that was a result of a change in CUPS proper, and updates the UNLABELED export check to compare job->scon to UNKNOWN_SL in addition to null instead of just null.
I just re-tested this. The package I built which should have had the id=137672 patch in.. didn't somehow. :-( Thanks for id=137751; I'll check that in to CVS. I'm afraid it's too late now for Fedora Core 6 (bad timing :-( ), so we'll do a Fedora Core update to add in the LSPP patch shortly after FC6 release.
Fixed package is cups-1.2.4-8.
..and now cups-1.2.4-9 has made it into the current Fedora Core 6 tree.