SELinuxProject / selinux

This is the upstream repository for the Security Enhanced Linux (SELinux) userland libraries and tools. The software provided by this project complements the SELinux features integrated into the Linux kernel and is used by Linux distributions. All bugs and patches should be submitted to selinux@vger.kernel.org
Other
1.32k stars 356 forks source link

Some though about how SELinux toolling is maintained in github #280

Open kloczek opened 3 years ago

kloczek commented 3 years ago

Current state causes many difficulties on maintaining SELinux packages tooling.

One repo to rule them all

Theoretically all SELinux tools are in separated source subtrees however and all dist tar balls are released as set of many tar balls however:

If you are not familiar with meson I can offer to help on that part. Using meson would allow solve many other issues like transparent LTO or PGO optimisation, easy integration of test units, coverage, generate dist tar balls and many other.

bachradsusi commented 3 years ago

Thanks for you thoughts. Please note that that main discussion point is at selinux@vger.kernel.org mailing list.

Current state causes many difficulties on maintaining SELinux packages tooling.

One repo to rule them all

Theoretically all SELinux tools are in separated source subtrees however and all dist tar balls are released as set of many tar balls however:

* many times single git commit is done across many subdirectories which makes very difficult extracting patches from git after latest release

I use git format-patch... -- <directory>:

$ git show 4a142ac46a11 -- libsepol | grep -E 'diff.*(libselinux|libsepol)'
diff --git a/libsepol/src/Makefile b/libsepol/src/Makefile

$ git show 4a142ac46a11 -- libselinux | grep -E 'diff.*(libselinux|libsepol)'
diff --git a/libselinux/src/load_policy.c b/libselinux/src/load_policy.c

$ git show 4a142ac46a11 | grep -E 'diff.*(libselinux|libsepol)'           
diff --git a/libselinux/src/load_policy.c b/libselinux/src/load_policy.c
diff --git a/libsepol/src/Makefile b/libsepol/src/Makefile
* because  everything is in one repo changes in each subdirectories must wait on that one (master) release and _sometimes that release even does not introduce any changes in some directors_

Could you please share the concrete change which was not propagated to new release?

* despite semi modularisation test suite seems is maintained in assumption that it will be executed in main selinux directory. Example is libsepol test suite:
+ /usr/bin/make -O -j1 V=1 VERBOSE=1 test
/usr/bin/make -C tests test
make[1]: Entering directory '/home/tkloczko/rpmbuild/BUILD/libsepol-3.1/tests'
gcc -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection  -fdata-sections -ffunction-sections -Os -I../include/ -I../../checkpolicy/  -c -o debug.o debug.c
gcc -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection  -fdata-sections -ffunction-sections -Os -I../include/ -I../../checkpolicy/  -c -o helpers.o helpers.c
helpers.c:25:10: fatal error: parse_util.h: No such file or directory
   25 | #include "parse_util.h"
      |          ^~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [<builtin>: helpers.o] Error 1

That parse_util.h is not part of public checkpolicy API and for example in Fedora there is no checkpolicy-devel package

* there are circular dependencies like checkpolicy during build requires libsepol and libsepol requires header files provide by checkpolicy (like in libsepol test suite uses checkpolicy headers)

In fact libsepol doesn't requires checkpolicy to build, the test suite is not part of the build process

all: 
        $(MAKE) -C src 
        $(MAKE) -C utils
* there is no proper separation between public and private API. This why currently is not possible to use only shared libraries

I see this as the public API is declared in libsepol/include, libselinux/include and libsemanage/include, everythin else is internal API.

How to solve above issues all together?

* start maintaining SELinux tooling in separated git modules. Proper separation can be done continuously with extracting one by one each necessary part and remove it from SELinuxProject/selinux

* introduce proper build framework using for example meson with checking build dependencies using

* define properly libraries public interface and use only that API in other modules

* start release each module (currently directories) with own versioning. This would allow smooth fixing critical issues

* divide current test suite and spread across each component with using only other components public API/ABI and test internal API only inside each component

If you are not familiar with meson I can offer to help on that part. Using meson would allow solve many other issues like transparent LTO or PGO optimisation, easy integration of test units, coverage, generate dist tar balls and many other.

You propose quite a big change which needs to be discussed at selinux@vger.kernel.org mailing list. Also feel free to send patches there, see https://github.com/SELinuxProject/selinux/blob/master/CONTRIBUTING.md for instructions.

kloczek commented 3 years ago

In fact libsepol doesn't requires checkpolicy to build, the test suite is not part of the build process

And that is the problem. Whole SELinux is in kind of internal self orthogonal state. On one angle everything is in separated directories but everything is kept in single git repo. Tracing changes against single directory/module is not easies as it could be, On the another angle (of test units) everything in single tree.

Again: if it is not possible to use test units per directory unpackaged from dist tar balls those units cannot be used to automatically confirm in automated build process that some basic functionalities are OK. That step of building, preparing and testing binary packaged resources is really important.

You propose quite a big change which needs to be discussed at selinux@vger.kernel.org mailing list. Also feel free to send patches there, see https://github.com/SELinuxProject/selinux/blob/master/CONTRIBUTING.md for instructions.

This is not one big change. All what needs to be done is for example:

  1. clone/fork existing SELinuxProject/selinux to SELinuxProject/libsepol
  2. in cloned repo remove everything except libsepo
  3. in cloned using git move libsepol/* content one level up
  4. than ONLY after finishing content of the SELinuxProject/libsepol in SELinuxProject/selinux remove in SELinuxProject/selinux/libsepol and add all other changes directories to solidify state that libsepol is now in separated repo. This can be done as single PR which could be tested one directory separation
  5. go to 1. and repeat with next directory.

In above scenario each cloned repo will keep full history of the past changes. At the end SELinuxProject/selinux could be deleted or archived (left in RO state just FTR). In other words all that kind of changes could be done sets of smaller batches of changes. Issue is that because it is about cloning/forking repos steps 1-3 cannot be done be someone like me. I can only submit PRs (and I'm not asking to give me permission to that kind of operations).

bachradsusi commented 3 years ago

Again, this discussion should happen at selinux@vger.kernel.org - you don't have to be subscribed to send an email there, it's common to CC people from outside. This place is not visible for everybody, not properly archived and so on.

What you propose brings a lot of work and testing which has to be done by someone, e.g. 5 steps you mentioned above would break make test in libsepol standalone repo in the way you describe it's broken when you you only libsepol-X.Y.tar.gz.

From my POV the solution for your problem is not to split sources but to merge tarballs together. In fact such tarball is available at https://github.com/SELinuxProject/selinux/releases/tag/20200710 - https://github.com/SELinuxProject/selinux/archive/20200710.tar.gz In the next release this file would be called 3.2.tar.gz but I guess we can generate it as selinux-3.2.tar.gz as part of release.

I can only submit PRs

Thanks. Please follow https://github.com/SELinuxProject/selinux/blob/master/CONTRIBUTING.md If you can't send a patch to mailing list for some reason, I can do it for you.

kloczek commented 3 years ago

Really I don't see any sense to discuss this with anyone else than maintainer. I'm not interested to waste time on talks with anyone else.

If you are not convinced by what I;ve wrote just cloce the ticket.

williamcroberts commented 3 years ago

Really I don't see any sense to discuss this with anyone else than maintainer. I'm not interested to waste time on talks with anyone else.

If you are not convinced by what I;ve wrote just cloce the ticket.

That mailing list is how you reach maintainers. Not all of the maintainers are reachable via Github. This is something that I think warrants discussion, but you need to be willing to bring it to the right venue: the mailing list.

kloczek commented 3 years ago

So please just discuss that between maintainers. With full respect no one else needs to be involved in such discussion (even me).

kloczek commented 3 years ago

Yet another small pebble to whole pile of problems:

I use git format-patch... -- :

It doesn't work or it works only when git command is used to extract patches. Issue is that It generates patch with exactly the same hash as original commit but with extracted changes only in exact directory. Such patch cannot be verified against original git repo (because it has different content) or cannot be downloaded over gitlab/github/cgit HTTP/HTTPS rest interface by for example `wget https://github.com/SELinuxProject/selinux/commit/.patch' (in case of gitlab/github). Issue is that github/gitlab/cgit rest interfaces do not allow to download part of the commit limited to only exact directory. That can be be solved only by converting directories in single git repo into separated repositories.

I'm using in my automated rpm build processes extraction of the cgit/gitlab/githiub patches over rest interface on really massive scale because I want to retest all new changes against whole set of packages by do scratch build after each passible commit and test such scratch package does it interacts still correctly with all other packages.

Using HTTP/HTTPS rest interface is very useful because it allows download single commit in sometimes fraction of second without cloning git repo and extracting exact commit.

Only packages on scale of thousands of other packages (and and few 100k patches tested that way) are SELinux packages because only here some parts which are logically separated entities are maintained in single git repo.

In other words holding all SELinix tooling code in single git repo adds additional overhead on packaging layer because all SELinux packages maintenance needs to be handled in special way. Using single git repo creates obstacle in continuous testing all possible upcoming changes. As you can guess to confirm that everything is OK such continues testing very extensively relies on test suites integrated into each possible package source trees. However in case of SELinux it cannot be used because bad separation of of test inside each SELinux logical bits.

Of course it is yet another possibility to solve all described issues by just stop distributing SELinux source code tooling as separated dist tar balls (which I don't think that would be healthy). That is probably my last argument pointing on proper separation of SELinuxProject/selinux/* directories repo into separated git repos.

kloczek commented 3 years ago

Is it any chance to modularize whole SELinix source code a bit?

IMO maintaining all SELinux stuff as packages is more and more difficult. All because everything is in single SCM tree. Looking only on Fedora spec files is possible to see how complicated whole packaging process must be now and that complexity only grows. As each part of the SELinux tooling is entangled with main tree which releases are not so often more and more patches from git needs to be added. Still some components are using internal API of other components which forces to use static libraries. IMO everything instead improving looks more and more complicated without clear path where everything is going.

If may I suggest IMO it would start from separating libsepol, libsemanage and libselinux. Than when those three components will be isolated other parts could start using exact version of each of those libraries as minimum version.

BTW discussion anything. IMO it would be better to unlock discussion in git repo and move away from mailing list.

fishilico commented 3 years ago

Looking only on Fedora spec files is possible to see how complicated whole packaging process must be now and that complexity only grows.

For information, using such phrasing ("is possible to see how complicated") without giving proper pointers/URL is unrespectful: if I want to take a look at "Fedora spec files", I can find https://src.fedoraproject.org/rpms/libsepol but I am not sure whether you meant libsepol, libselinux or another package. Then I need to choose a branch and try guessing what you mean by "see how complicated whole packaging process must be now" and fail to see the complexity. In the end, I feel like I have wasted my time trying to understand this sentence.

Could you please avoid using such generic phrasing and be more respectful and precise in your messages?

I suggest IMO it would start from separating libsepol, libsemanage and libselinux.

If I may provide some feedback from some experience from another project I participated recently: the TPM 2.0 software stack uses separate repositories on https://github.com/tpm2-software/. It seems nice and clean, but when you start depending on development versions of other dependencies, it becomes quite hard to install properly. And sometimes changing something in a project breaks another one and this breaks the Continuous Integration for some seeks (for example I experienced this in https://github.com/tpm2-software/tpm2-pkcs11/pull/702 ). So IMHO, right now I feel like separating the SELinux projects in several repositories will require much more maintenance effort than currently and I think there are not enough active maintainers to bear such an effort.

kloczek commented 3 years ago

If I may provide some feedback from some experience from another project I participated recently: the TPM 2.0 software stack uses separate repositories on https://github.com/tpm2-software/. It seems nice and clean, but when you start depending on development versions of other dependencies, it becomes quite hard to install properly. And sometimes changing something in a project breaks another one and this breaks the Continuous Integration for some seeks (for example I experienced this in tpm2-software/tpm2-pkcs11#702 ). So IMHO, right now I feel like separating the SELinux projects in several repositories will require much more maintenance effort than currently and I think there are not enough active maintainers to bear such an effort.

Sorry but I don't see that complexity. We are talking about exactly the same commit but released as version in which changes in API/ABI in nore component should trigger series of releases of another packages. Issue is that as long as there is no in SELinux clear separation between components everything is blurred and there is no boundaries. Result is that currently as you can see for example in Fedora it is necessary to apply several patches taken from git. Example:

[tkloczko@barrel SPECS.fedora]$ for i in $(grep https://github.com/SELinuxProject * -l); do echo -n "$i "; grep -c ^Patch  $i; done
checkpolicy.spec 16
libselinux.spec 37
libsemanage.spec 4
libsepol.spec 101
mcstrans.spec 4
policycoreutils.spec 25
secilc.spec 13
setools.spec 2

Those numbers shows number of patches per each SELinux component.

```console [tkloczko@barrel SPECS.fedora]$ for i in $(grep https://github.com/SELinuxProject * -l); do echo "$i:"; grep ^Patch $i; done checkpolicy.spec: Patch0001: 0001-libsepol-checkpolicy-Set-user-roles-using-role-value.patch Patch0002: 0002-checkpolicy-Do-not-automatically-upgrade-when-using-.patch Patch0003: 0003-checkpolicy-silence-Wextra-semi-stmt-warning.patch Patch0004: 0004-checkpolicy-pass-CFLAGS-at-link-stage.patch Patch0005: 0005-checkpolicy-drop-pipe-compile-option.patch Patch0006: 0006-checkpolicy-simplify-assignment.patch Patch0007: 0007-checkpolicy-drop-dead-condition.patch Patch0008: 0008-checkpolicy-use-correct-format-specifier-for-unsigne.patch Patch0009: 0009-checkpolicy-follow-declaration-after-statement.patch Patch0010: 0010-checkpolicy-remove-dead-assignments.patch Patch0011: 0011-checkpolicy-check-before-potential-NULL-dereference.patch Patch0012: 0012-checkpolicy-avoid-potential-use-of-uninitialized-var.patch Patch0013: 0013-checkpolicy-drop-redundant-cast-to-the-same-type.patch Patch0014: 0014-checkpolicy-parse_util-drop-unused-declaration.patch Patch0015: 0015-checkpolicy-test-mark-file-local-functions-static.patch Patch0016: 0016-checkpolicy-mark-read-only-parameters-in-policy-defi.patch libselinux.spec: Patch0001: 0001-libselinux-do-not-duplicate-make-target-when-going-i.patch Patch0002: 0002-libselinux-selinux_check_passwd_access_internal-resp.patch Patch0003: 0003-libselinux-silence-Wstringop-overflow-warning-from-g.patch Patch0004: 0004-libselinux-sidtab_hash-do-not-discard-const-qualifie.patch Patch0005: 0005-libselinux-selinux_file_context_cmp-do-not-discard-c.patch Patch0006: 0006-libselinux-label_common-do-not-discard-const-qualifi.patch Patch0007: 0007-libselinux-Sha1Finalise-do-not-discard-const-qualifi.patch Patch0008: 0008-libselinux-sefcontext_compile-mark-local-variable-st.patch Patch0009: 0009-libselinux-avcstat-use-standard-length-modifier-for-.patch Patch0010: 0010-libselinux-selinux_restorecon-mark-local-variable-st.patch Patch0011: 0011-libselinux-selabel_get_digests_all_partial_matches-f.patch Patch0012: 0012-libselinux-getconlist-free-memory-on-multiple-level-.patch Patch0013: 0013-libselinux-exclude_non_seclabel_mounts-drop-unused-v.patch Patch0014: 0014-libselinux-context_new-drop-dead-assignment.patch Patch0015: 0015-libselinux-label_x-init-drop-dead-assignment.patch Patch0016: 0016-libselinux-label_media-init-drop-dead-assignment.patch Patch0017: 0017-libselinux-setexecfilecon-drop-dead-assignment.patch Patch0018: 0018-libselinux-getdefaultcon-free-memory-on-multiple-sam.patch Patch0019: 0019-libselinux-store_stem-do-not-free-possible-non-heap-.patch Patch0020: 0020-libselinux-matchmediacon-close-file-on-error.patch Patch0021: 0021-libselinux-init_selinux_config-free-resources-on-err.patch Patch0022: 0022-libselinux-label_file-init-do-not-pass-NULL-to-strdu.patch Patch0023: 0023-libselinux-matchpathcon-free-memory-on-realloc-failu.patch Patch0024: 0024-libselinux-label_db-db_init-open-file-with-CLOEXEC-m.patch Patch0025: 0025-libselinux-drop-redundant-casts-to-the-same-type.patch Patch0026: 0026-libselinux-sidtab_sid_stats-unify-parameter-name.patch Patch0027: 0027-libselinux-regex-unify-parameter-names.patch Patch0028: 0028-libselinux-label_file.c-fix-indent.patch Patch0029: 0029-libselinux-avc_destroy-3-closes-status-page.patch Patch0030: 0030-libselinux-make-selinux_status_open-3-reentrant.patch Patch0031: 0031-libselinux-do-not-use-status-page-fallback-mode-inte.patch Patch0032: 0032-libselinux-selinux_status_open-return-1-in-fallback-.patch Patch0033: 0033-libselinux-improve-getcon-3-man-page.patch Patch0034: 0034-libselinux-fix-typo.patch Patch0035: 0035-selinux.8-document-how-mount-flag-nosuid-affects-SEL.patch Patch0036: 0036-libselinux-utils-getseuser.c-fix-build-with-gcc-4.8.patch Patch0037: 0037-libselinux-silence-Wextra-semi-stmt-warning.patch libsemanage.spec: Patch0001: 0001-libsemanage-fix-use-after-free-in-parse_module_store.patch Patch0002: 0002-libsemanage-silence-Wextra-semi-stmt-warning.patch Patch0003: 0003-libsemanage-Fix-RESOURCE_LEAK-and-USE_AFTER_FREE-cov.patch Patch0004: 0004-libsemanage-Fix-USE_AFTER_FREE-CWE-672-in-semanage_d.patch libsepol.spec: Patch0001: 0001-libsepol-Expand-role-attributes-in-constraint-expres.patch Patch0002: 0002-libsepol-Properly-handle-types-associated-to-role-at.patch Patch0003: 0003-libsepol-Remove-unnecessary-copying-of-declarations-.patch Patch0004: 0004-libsepol-Check-kernel-to-CIL-and-Conf-functions-for-.patch Patch0005: 0005-libsepol-cil-make-cil_post_fc_fill_data-static.patch Patch0006: 0006-libsepol-cil-remove-stray-printf.patch Patch0007: 0007-libsepol-cil-replace-printf-with-proper-cil_tree_log.patch Patch0008: 0008-libsepol-cil-fix-NULL-pointer-dereference-in-__cil_i.patch Patch0009: 0009-libsepol-cil-do-not-leak-avrulex_ioctl_table-memory-.patch Patch0010: 0010-libsepol-make-num_-unsigned-int-in-module_to_cil.patch Patch0011: 0011-libsepol-Write-NO_IDENTIFIER-for-empty-constraint-ex.patch Patch0012: 0012-libsepol-Enclose-identifier-lists-in-constraint-expr.patch Patch0013: 0013-libsepol-cil-Allow-lists-in-constraint-expressions.patch Patch0014: 0014-libsepol-Enclose-identifier-lists-in-CIL-constraint-.patch Patch0015: 0015-libsepol-Write-NO_IDENTIFIER-for-empty-CIL-constrain.patch Patch0016: 0016-libsepol-cil-Check-for-duplicate-blocks-optionals-an.patch Patch0017: 0017-libsepol-cil-Fix-out-of-bound-read-of-file-context-p.patch Patch0018: 0018-libsepol-cil-Destroy-classperms-list-when-resetting-.patch Patch0019: 0019-libsepol-cil-Destroy-classperm-list-when-resetting-m.patch Patch0020: 0020-libsepol-cil-cil_reset_classperms_set-should-not-res.patch Patch0021: 0021-libsepol-cil-Set-class-field-to-NULL-when-resetting-.patch Patch0022: 0022-libsepol-cil-More-strict-verification-of-constraint-.patch Patch0023: 0023-libsepol-cil-Exit-with-an-error-if-declaration-name-.patch Patch0024: 0024-libsepol-cil-Allow-permission-expressions-when-using.patch Patch0025: 0025-libsepol-cil-Refactor-helper-function-for-cil_gen_no.patch Patch0026: 0026-libsepol-cil-Create-function-cil_add_decl_to_symtab-.patch Patch0027: 0027-libsepol-cil-Move-check-for-the-shadowing-of-macro-p.patch Patch0028: 0028-libsepol-cil-Reorder-checks-for-invalid-rules-when-b.patch Patch0029: 0029-libsepol-cil-Cleanup-build-AST-helper-functions.patch Patch0030: 0030-libsepol-cil-Create-new-first-child-helper-function-.patch Patch0031: 0031-libsepol-cil-Use-AST-to-track-blocks-and-optionals-w.patch Patch0032: 0032-libsepol-cil-Reorder-checks-for-invalid-rules-when-r.patch Patch0033: 0033-libsepol-cil-Sync-checks-for-invalid-rules-in-boolea.patch Patch0034: 0034-libsepol-cil-Check-for-statements-not-allowed-in-opt.patch Patch0035: 0035-libsepol-cil-Sync-checks-for-invalid-rules-in-macros.patch Patch0036: 0036-libsepol-cil-Do-not-allow-tunable-declarations-in-in.patch Patch0037: 0037-libsepol-cil-Make-invalid-statement-error-messages-c.patch Patch0038: 0038-libsepol-cil-Use-CIL_ERR-for-error-messages-in-cil_c.patch Patch0039: 0039-libsepol-cil-Create-functions-to-write-the-CIL-AST.patch Patch0040: 0040-libsepol-cil-Add-functions-to-make-use-of-cil_write_.patch Patch0041: 0041-libsepol-use-checked-arithmetic-builtin-to-perform-s.patch Patch0042: 0042-libsepol-cil-Properly-reset-an-anonymous-classperm-s.patch Patch0043: 0043-libsepol-cil-Fix-instances-where-an-error-returns-SE.patch Patch0044: 0044-libsepol-cil-Detect-degenerate-inheritance-and-exit-.patch Patch0045: 0045-libsepol-cil-Check-datum-in-ordered-list-for-expecte.patch Patch0046: 0046-libsepol-cil-Return-an-error-if-a-call-argument-fail.patch Patch0047: 0047-libsepol-cil-Check-for-self-referential-loops-in-set.patch Patch0048: 0048-libsepol-cil-Fix-name-resolution-involving-inherited.patch Patch0049: 0049-libsepol-cil-Make-name-resolution-in-macros-work-as-.patch Patch0050: 0050-libsepol-cil-Do-not-add-NULL-node-when-inserting-key.patch Patch0051: 0051-libsepo-cil-Refactor-macro-call-resolution.patch Patch0052: 0052-libsepol-cil-Do-not-resolve-arguments-to-declaration.patch Patch0053: 0053-libsepol-cil-Handle-disabled-optional-blocks-in-earl.patch Patch0054: 0054-libsepol-cil-Destroy-the-permission-nodes-when-exiti.patch Patch0055: 0055-libsepol-cil-Limit-the-number-of-open-parenthesis-al.patch Patch0056: 0056-libsepol-cil-Resolve-anonymous-class-permission-sets.patch Patch0057: 0057-libsepol-cil-Pointers-to-datums-should-be-set-to-NUL.patch Patch0058: 0058-libsepol-cil-Resolve-anonymous-levels-only-once.patch Patch0059: 0059-libsepol-quote-paths-in-CIL-conversion.patch Patch0060: 0060-libsepol-cil-Fix-anonymous-IP-address-call-arguments.patch Patch0061: 0061-libsepol-cil-Account-for-anonymous-category-sets-in-.patch Patch0062: 0062-libsepol-Quote-paths-when-generating-policy.conf-fro.patch Patch0063: 0063-libsepol-fix-typos.patch Patch0064: 0064-libsepol-resolve-missing-prototypes.patch Patch0065: 0065-libsepol-remove-unused-functions.patch Patch0066: 0066-libsepol-avoid-unsigned-integer-overflow.patch Patch0067: 0067-libsepol-follow-declaration-after-statement.patch Patch0068: 0068-libsepol-cil-follow-declaration-after-statement.patch Patch0069: 0069-libsepol-remove-dead-stores.patch Patch0070: 0070-libsepol-mark-read-only-parameters-of-ebitmap-interf.patch Patch0071: 0071-libsepol-mark-read-only-parameters-of-type_set_-inte.patch Patch0072: 0072-libsepol-do-not-allocate-memory-of-size-0.patch Patch0073: 0073-libsepol-remove-dead-stores.patch Patch0074: 0074-libsepol-cil-silence-cast-warning.patch Patch0075: 0075-libsepol-cil-drop-extra-semicolon.patch Patch0076: 0076-libsepol-cil-drop-dead-store.patch Patch0077: 0077-libsepol-cil-drop-unnecessary-casts.patch Patch0078: 0078-libsepol-cil-avoid-using-maybe-uninitialized-variabl.patch Patch0079: 0079-libsepol-drop-repeated-semicolons.patch Patch0080: 0080-libsepol-drop-unnecessary-casts.patch Patch0081: 0081-libsepol-declare-file-local-variable-static.patch Patch0082: 0082-libsepol-declare-read-only-arrays-const.patch Patch0083: 0083-libsepol-cil-Allow-duplicate-optional-blocks-in-most.patch Patch0084: 0084-libsepol-cil-Properly-check-for-loops-in-sets.patch Patch0085: 0085-libsepol-cil-Fix-syntax-checking-of-defaultrange-rul.patch Patch0086: 0086-libsepol-cil-Check-for-empty-list-when-marking-never.patch Patch0087: 0087-libsepol-cil-Reduce-the-initial-symtab-sizes-for-blo.patch Patch0088: 0088-libsepol-cil-Improve-degenerate-inheritance-check.patch Patch0089: 0089-libsepol-cil-Add-function-to-determine-if-a-subtree-.patch Patch0090: 0090-libsepol-cil-Only-reset-AST-if-optional-has-a-declar.patch Patch0091: 0091-libsepol-cil-make-array-cil_sym_sizes-const.patch Patch0092: 0092-libsepol-cil-Provide-option-to-allow-qualified-names.patch Patch0093: 0093-libsepol-cil-do-not-override-previous-results-of-__c.patch Patch0094: 0094-libsepol-silence-Wextra-semi-stmt-warning.patch Patch0095: 0095-libsepol-cil-Improve-checking-for-bad-inheritance-pa.patch Patch0096: 0096-libsepol-avoid-unsigned-integer-overflow.patch Patch0097: 0097-libsepol-ignore-UBSAN-false-positives.patch Patch0098: 0098-libsepol-avoid-implicit-conversions.patch Patch0099: 0099-libsepol-assure-string-NUL-termination-of-ibdev_name.patch Patch0100: 0100-libsepol-cil-Fix-handling-category-sets-in-an-expres.patch Patch0101: 0101-libsepol-cil-do-not-allow-0-in-quoted-strings.patch mcstrans.spec: Patch0001: 0001-mcstrans-silence-Wextra-semi-stmt-warning.patch Patch0002: 0002-mcstrans-Fir-RESOURCE_LEAK-and-USE_AFTER_FREE-coveri.patch Patch0003: 0003-mcstrans-Fix-USER_AFTER_FREE-problem.patch Patch0004: 0004-mcstrans-Do-not-accept-incomplete-contexts.patch policycoreutils.spec: Patch0001: 0001-policycoreutils-setfiles-do-not-create-useless-setfi.patch Patch0002: 0002-fixfiles-do-not-exclude-dev-and-run-in-C-mode.patch Patch0003: 0003-policycoreutils-silence-Wextra-semi-stmt-warning.patch Patch0004: 0004-policycoreutils-free-memory-on-lstat-failure-in-sest.patch Patch0005: 0005-policycoreutils-free-memory-of-allocated-context-in-.patch Patch0006: 0006-policycoreutils-free-memory-of-allocated-context-in-.patch Patch0007: 0007-sandbox-add-reset-to-Xephyr-as-it-works-better-with-.patch Patch0008: 0008-Fix-STANDARD_FILE_CONTEXT-section-in-man-pages.patch Patch0009: 0009-If-there-is-no-executable-we-don-t-want-to-print-a-p.patch Patch0010: 0010-Simplication-of-sepolicy-manpage-web-functionality.-.patch Patch0011: 0011-We-want-to-remove-the-trailing-newline-for-etc-syste.patch Patch0012: 0012-Fix-title-in-manpage.py-to-not-contain-online.patch Patch0013: 0013-Don-t-be-verbose-if-you-are-not-on-a-tty.patch Patch0014: 0014-sepolicy-Drop-old-interface-file_type_is_executable-.patch Patch0015: 0015-sepolicy-Another-small-optimization-for-mcs-types.patch Patch0016: 0016-Move-po-translation-files-into-the-right-sub-directo.patch Patch0017: 0017-Use-correct-gettext-domains-in-python-gui-sandbox.patch Patch0018: 0018-Initial-.pot-files-for-gui-python-sandbox.patch Patch0019: 0019-policycoreutils-setfiles-Improve-description-of-d-sw.patch Patch0020: 0020-sepolicy-generate-Handle-more-reserved-port-types.patch Patch0021: 0021-semodule-utils-Fix-RESOURCE_LEAK-coverity-scan-defec.patch Patch0022: 0022-sandbox-Use-matchbox-window-manager-instead-of-openb.patch Patch0023: 0023-sepolicy-Fix-flake8-warnings-in-Fedora-only-code.patch Patch0024: 0024-Do-not-use-Python-slip.patch Patch0025: 0025-dbus-Use-GLib.MainLoop.patch secilc.spec: Patch0001: 0001-secilc-docs-Lists-are-now-allowed-in-constraint-expr.patch Patch0002: 0002-cil_conditional_statements.md-fix-expr-definition.patch Patch0003: 0003-secilc.c-Don-t-fail-if-input-file-is-empty.patch Patch0004: 0004-secilc-docs-Update-the-CIL-documentation-for-various.patch Patch0005: 0005-secilc-Create-the-new-program-called-secil2tree-to-w.patch Patch0006: 0006-secilc-docs-Document-the-order-that-inherited-rules-.patch Patch0007: 0007-secilc-docs-Relocate-and-reword-macro-call-name-reso.patch Patch0008: 0008-secilc-test-Add-test-for-anonymous-args.patch Patch0009: 0009-secilc-Add-support-for-using-qualified-names-to-seci.patch Patch0010: 0010-libsepol-cil-Add-support-for-using-qualified-names-t.patch Patch0011: 0011-libsepol-cil-Add-support-for-using-qualified-names-t.patch Patch0012: 0012-secilc-fix-memory-leaks-in-secilc.patch Patch0013: 0013-secilc-fix-memory-leaks-in-secilc2conf.patch setools.spec: Patch1002: 1002-Do-not-export-use-setools.InfoFlowAnalysis-and-setoo.patch Patch1003: 1003-Require-networkx-on-package-level.patch ```

As you see ALL those patches are from git. All those patches are taken from single repo. If you are thinking about that by keeping everything in one repo makes all that simpler that may be even true but at the end of the day it delegates more work to packaging layer. Example SuSE on top of latest libsepol 3.2 adds 3 patches (all of them are CVEs!!) and you can find more of that kind of patches in other SELinux components. Because libsepol is used as static library by other SELinux components it is necessary to check impact of such CVEs to consider recompile other components which are using libsepol.a.

As I wrote I can offer some help on such separation. Separation would allow release only one component after critical issue in some place without scratching head what to do with other components.

As I wrote I can offer some help on such separation. If SELinux maintainers want my help all what I would ask would be just try to fork SELinuxProject/selinux repo to SELinuxProject/libsepol and I'll try to submit PR with series of commits to reshape that first part. After that it will be still necessary to tweak a bit SELinuxProject/selinux however that part can be done with really small overhead. At any time it would be possible to delete that work without disturbing what is in SELinuxProject/selinux.

Why cloning? IMO it would be good to keep all past changes in each SELinuxProject/<component> but that is not necessary. If SELinux maintainers want to do that without keeping record of past changes I would just ask to copy SELinuxProject/selinux/libsepol to SELinuxProject/libsepol. With separated properly libsepol it will be IMO clear to see where all that may be heading (I can understand that it is still may not be clear).

bachradsusi commented 3 years ago

Those numbers shows number of patches per each SELinux component.

These numbers shows only patches which were backported from upstream master to Fedora. It's not related to the fact that everything is one repo, it's related to the fact that I wanted to have latest unreleased bits in Rawhide so that it's tested and prepared for 3.3 release.

As you see ALL those patches are from git. All those patches are taken from single repo.

Right, but I don't see this as an issue So

git format-patch -N 3.2 -- libsepol
git format-patch -N 3.2 -- libselinux
...

OTOH with the split of the repo, Fedora would have a problem with policycoreutils package as it's originally based on one directory policycoreutils which was split to several new directories, so instead of one command

git format-patch -N 3.2 -- policycoreutils python gui sandbox dbus semodule-utils restorecond

it would have to clone 7 different repositories.

If you are thinking about that by keeping everything in one repo makes all that simpler that may be even true but at the end of the day it delegates more work to packaging layer. Example SuSE on top of latest libsepol 3.2 adds 3 patches (all of them are CVEs!!) and you can find more of that kind of patches in other SELinux components.

Number of patches used in package is not related to the format of this repo. Fedora libsepol package has complete upstream master backported, while SuSE patched only CVEs. If libsepol was split, Fedora would have same number of patches in the package as it's now.

Because libsepol is used as static library by other SELinux components it is necessary to check impact of such CVEs to consider recompile other components which are using libsepol.a.

You would still need to rebuild everything statically linked.

As I wrote I can offer some help on such separation. If SELinux maintainers want my help all what I would ask would be just try to fork SELinuxProject/selinux repo to SELinuxProject/libsepol and I'll try to submit PR with series of commits to reshape that first part. After that it will be still necessary to tweak a bit SELinuxProject/selinux however that part can be done with really small overhead. At any time it would be possible to delete that work without disturbing what is in SELinuxProject/selinux.

Why cloning? IMO it would be good to keep all past changes in each SELinuxProject/<component> but that is not necessary. If SELinux maintainers want to do that without keeping record of past changes I would just ask to copy SELinuxProject/selinux/libsepol to SELinuxProject/libsepol. With separated properly libsepol it will be IMO clear to see where all that may be heading (I can understand that it is still may not be clear).

I don't think you need to use SELinuxProject for this. Feel free to start in your own namespace, and when you think it's ready, send patches for review to selinux@vger.kernel.org. Also I'd like to ask again to move this discussion to the selinux@vger.kernel.org list as well.

kloczek commented 1 year ago

Almost two years later and looks like none of the progress has been made on better isolation/separation of rhe SELinux exacr components. Currently:

Curently looks like usimg SELinus tooling outside monorepo soes not make to much sense. Other problems are related to build fra,ework consisting from set of custom/mand made Makefile files.

Is it any long term to reshape/reorganize that? 🤔