[PATCH 1/4] libsepol: Get rid of the old and duplicated symbols

[PATCH 1/4] libsepol: Get rid of the old and duplicated symbols

[Petr Lautrbach]

Versioned duplicate symbols cause problems for LTO. These symbols were
introduced during the CIL integration several releases ago and were only
consumed by other SELinux userspace components.

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
---
 libsepol/cil/src/cil.c       | 84 ------------------------------------
 libsepol/src/libsepol.map.in |  5 ---
 2 files changed, 89 deletions(-)

diff --git a/libsepol/cil/src/cil.c b/libsepol/cil/src/cil.c
index a3c6a2934c72..95bdb5e5854c 100644
--- a/libsepol/cil/src/cil.c
+++ b/libsepol/cil/src/cil.c
@@ -51,27 +51,6 @@
 #include "cil_policy.h"
 #include "cil_strpool.h"
 
-#if !defined(SHARED) || defined(ANDROID) || defined(__APPLE__)
-    #define DISABLE_SYMVER 1
-#endif
-
-#ifndef DISABLE_SYMVER
-asm(".symver cil_build_policydb_pdb,        cil_build_policydb@LIBSEPOL_1.0");
-asm(".symver cil_build_policydb_create_pdb, cil_build_policydb@@LIBSEPOL_1.1");
-
-asm(".symver cil_compile_pdb,   cil_compile@LIBSEPOL_1.0");
-asm(".symver cil_compile_nopdb, cil_compile@@LIBSEPOL_1.1");
-
-asm(".symver cil_userprefixes_to_string_pdb,   cil_userprefixes_to_string@LIBSEPOL_1.0");
-asm(".symver cil_userprefixes_to_string_nopdb, cil_userprefixes_to_string@@LIBSEPOL_1.1");
-
-asm(".symver cil_selinuxusers_to_string_pdb,   cil_selinuxusers_to_string@LIBSEPOL_1.0");
-asm(".symver cil_selinuxusers_to_string_nopdb, cil_selinuxusers_to_string@@LIBSEPOL_1.1");
-
-asm(".symver cil_filecons_to_string_pdb,   cil_filecons_to_string@LIBSEPOL_1.0");
-asm(".symver cil_filecons_to_string_nopdb, cil_filecons_to_string@@LIBSEPOL_1.1");
-#endif
-
 int cil_sym_sizes[CIL_SYM_ARRAY_NUM][CIL_SYM_NUM] = {
 	{64, 64, 64, 1 << 13, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64},
 	{64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64},
@@ -549,11 +528,7 @@ exit:
 	return rc;
 }
 
-#ifdef DISABLE_SYMVER
 int cil_compile(struct cil_db *db)
-#else
-int cil_compile_nopdb(struct cil_db *db)
-#endif
 {
 	int rc = SEPOL_ERR;
 
@@ -597,33 +572,7 @@ exit:
 	return rc;
 }
 
-#ifndef DISABLE_SYMVER
-int cil_compile_pdb(struct cil_db *db, __attribute__((unused)) sepol_policydb_t *sepol_db)
-{
-	return cil_compile_nopdb(db);
-}
-
-int cil_build_policydb_pdb(cil_db_t *db, sepol_policydb_t *sepol_db)
-{
-	int rc;
-
-	cil_log(CIL_INFO, "Building policy binary\n");
-	rc = cil_binary_create_allocated_pdb(db, sepol_db);
-	if (rc != SEPOL_OK) {
-		cil_log(CIL_ERR, "Failed to generate binary\n");
-		goto exit;
-	}
-
-exit:
-	return rc;
-}
-#endif
-
-#ifdef DISABLE_SYMVER
 int cil_build_policydb(cil_db_t *db, sepol_policydb_t **sepol_db)
-#else
-int cil_build_policydb_create_pdb(cil_db_t *db, sepol_policydb_t **sepol_db)
-#endif
 {
 	int rc;
 
@@ -1371,11 +1320,7 @@ const char * cil_node_to_string(struct cil_tree_node *node)
 	return "<unknown>";
 }
 
-#ifdef DISABLE_SYMVER
 int cil_userprefixes_to_string(struct cil_db *db, char **out, size_t *size)
-#else
-int cil_userprefixes_to_string_nopdb(struct cil_db *db, char **out, size_t *size)
-#endif
 {
 	int rc = SEPOL_ERR;
 	size_t str_len = 0;
@@ -1420,13 +1365,6 @@ exit:
 
 }
 
-#ifndef DISABLE_SYMVER
-int cil_userprefixes_to_string_pdb(struct cil_db *db, __attribute__((unused)) sepol_policydb_t *sepol_db, char **out, size_t *size)
-{
-	return cil_userprefixes_to_string_nopdb(db, out, size);
-}
-#endif
-
 static int cil_cats_to_ebitmap(struct cil_cats *cats, struct ebitmap* cats_ebitmap)
 {
 	int rc = SEPOL_ERR;
@@ -1614,11 +1552,7 @@ static int __cil_level_to_string(struct cil_level *lvl, char *out)
 	return str_tmp - out;
 }
 
-#ifdef DISABLE_SYMVER
 int cil_selinuxusers_to_string(struct cil_db *db, char **out, size_t *size)
-#else
-int cil_selinuxusers_to_string_nopdb(struct cil_db *db, char **out, size_t *size)
-#endif
 {
 	size_t str_len = 0;
 	int buf_pos = 0;
@@ -1675,18 +1609,7 @@ int cil_selinuxusers_to_string_nopdb(struct cil_db *db, char **out, size_t *size
 	return SEPOL_OK;
 }
 
-#ifndef DISABLE_SYMVER
-int cil_selinuxusers_to_string_pdb(struct cil_db *db, __attribute__((unused)) sepol_policydb_t *sepol_db, char **out, size_t *size)
-{
-	return cil_selinuxusers_to_string_nopdb(db, out, size);
-}
-#endif
-
-#ifdef DISABLE_SYMVER
 int cil_filecons_to_string(struct cil_db *db, char **out, size_t *size)
-#else
-int cil_filecons_to_string_nopdb(struct cil_db *db, char **out, size_t *size)
-#endif
 {
 	uint32_t i = 0;
 	int buf_pos = 0;
@@ -1804,13 +1727,6 @@ int cil_filecons_to_string_nopdb(struct cil_db *db, char **out, size_t *size)
 	return SEPOL_OK;
 }
 
-#ifndef DISABLE_SYMVER
-int cil_filecons_to_string_pdb(struct cil_db *db, __attribute__((unused)) sepol_policydb_t *sepol_db, char **out, size_t *size)
-{
-	return cil_filecons_to_string_nopdb(db, out, size);
-}
-#endif
-
 void cil_set_disable_dontaudit(struct cil_db *db, int disable_dontaudit)
 {
 	db->disable_dontaudit = disable_dontaudit;
diff --git a/libsepol/src/libsepol.map.in b/libsepol/src/libsepol.map.in
index f08c2a861693..98da9789b71b 100644
--- a/libsepol/src/libsepol.map.in
+++ b/libsepol/src/libsepol.map.in
@@ -1,19 +1,14 @@
 LIBSEPOL_1.0 {
   global:
 	cil_add_file;
-	cil_build_policydb;
-	cil_compile;
 	cil_db_destroy;
 	cil_db_init;
-	cil_filecons_to_string;
-	cil_selinuxusers_to_string;
 	cil_set_disable_dontaudit;
 	cil_set_disable_neverallow;
 	cil_set_handle_unknown;
 	cil_set_log_handler;
 	cil_set_log_level;
 	cil_set_preserve_tunables;
-	cil_userprefixes_to_string;
 	expand_module_avrules;
 	sepol_bool_clone;
 	sepol_bool_compare;
-- 
2.28.0

[PATCH 2/4] libsepol: Bump libsepol.so version

[Petr Lautrbach]

It's due to the previous ABI incompatible change.

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
---
 libselinux/src/load_policy.c | 2 +-
 libsepol/src/Makefile        | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libselinux/src/load_policy.c b/libselinux/src/load_policy.c
index 2aea826f863e..0034fa53d6e6 100644
--- a/libselinux/src/load_policy.c
+++ b/libselinux/src/load_policy.c
@@ -76,7 +76,7 @@ int selinux_mkload_policy(int preservebools __attribute__((unused)))
 #ifdef SHARED
 	char *errormsg = NULL;
 	void *libsepolh = NULL;
-	libsepolh = dlopen("libsepol.so.1", RTLD_NOW);
+	libsepolh = dlopen("libsepol.so.2", RTLD_NOW);
 	if (libsepolh) {
 		usesepol = 1;
 		dlerror();
diff --git a/libsepol/src/Makefile b/libsepol/src/Makefile
index 8d466f56ed0e..dc8b1773d974 100644
--- a/libsepol/src/Makefile
+++ b/libsepol/src/Makefile
@@ -7,7 +7,7 @@ RANLIB ?= ranlib
 CILDIR ?= ../cil
 
 VERSION = $(shell cat ../VERSION)
-LIBVERSION = 1
+LIBVERSION = 2
 
 LEX = flex
 CIL_GENERATED = $(CILDIR)/src/cil_lexer.c
-- 
2.28.0

[PATCH 3/4] libsemanage: Remove legacy and duplicate symbols

[Petr Lautrbach]

Versioned duplicate symbols cause problems for LTO. These symbols were
introduced during the CIL integration several releases ago and were only
consumed by other SELinux userspace components.

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
---
 libsemanage/include/semanage/modules.h |   2 +-
 libsemanage/src/libsemanage.map        |   5 --
 libsemanage/src/modules.c              | 100 +------------------------
 libsemanage/src/modules.h              |   9 +--
 libsemanage/src/semanageswig_python.i  |   2 -
 5 files changed, 4 insertions(+), 114 deletions(-)

diff --git a/libsemanage/include/semanage/modules.h b/libsemanage/include/semanage/modules.h
index ac4039314857..b51f61f033d5 100644
--- a/libsemanage/include/semanage/modules.h
+++ b/libsemanage/include/semanage/modules.h
@@ -33,7 +33,7 @@ typedef struct semanage_module_key semanage_module_key_t;
  */
 
 extern int semanage_module_install(semanage_handle_t *,
-				   char *module_data, size_t data_len, char *name, char *ext_lang);
+				   char *module_data, size_t data_len, const char *name, const char *ext_lang);
 extern int semanage_module_install_file(semanage_handle_t *,
 					const char *module_name);
 extern int semanage_module_remove(semanage_handle_t *, char *module_name);
diff --git a/libsemanage/src/libsemanage.map b/libsemanage/src/libsemanage.map
index 1375a8ca0ea7..4bec06aaae27 100644
--- a/libsemanage/src/libsemanage.map
+++ b/libsemanage/src/libsemanage.map
@@ -167,18 +167,13 @@ LIBSEMANAGE_1.0 {
     semanage_mls_enabled;
     semanage_module_disable;
     semanage_module_enable;
-    semanage_module_get_enabled;
     semanage_module_get_name;
     semanage_module_get_version;
     semanage_module_info_datum_destroy;
-    semanage_module_install;
-    semanage_module_install_base;
-    semanage_module_install_base_file;
     semanage_module_install_file;
     semanage_module_list;
     semanage_module_list_nth;
     semanage_module_remove;
-    semanage_module_upgrade;
     semanage_module_upgrade_file;
     semanage_msg_get_channel;
     semanage_msg_get_fname;
diff --git a/libsemanage/src/modules.c b/libsemanage/src/modules.c
index 6d3eb60ae462..8b36801038df 100644
--- a/libsemanage/src/modules.c
+++ b/libsemanage/src/modules.c
@@ -42,70 +42,7 @@
 #include "modules.h"
 #include "debug.h"
 
-asm(".symver semanage_module_get_enabled_1_1,semanage_module_get_enabled@@LIBSEMANAGE_1.1");
-asm(".symver semanage_module_get_enabled_1_0,semanage_module_get_enabled@LIBSEMANAGE_1.0");
-asm(".symver semanage_module_install_pp,semanage_module_install@LIBSEMANAGE_1.0");
-asm(".symver semanage_module_install_hll,semanage_module_install@@LIBSEMANAGE_1.1");
-
-/* Takes a module stored in 'module_data' and parses its headers.
- * Sets reference variables 'module_name' to module's name and
- * 'version' to module's version. The caller is responsible for
- * free()ing 'module_name' and 'version'; they will be
- * set to NULL upon entering this function.  Returns 0 on success, -1
- * if out of memory, or -2 if data did not represent a module.
- */
-static int parse_module_headers(semanage_handle_t * sh, char *module_data,
-				size_t data_len, char **module_name, char **version)
-{
-	struct sepol_policy_file *pf;
-	int file_type;
-	*version = NULL;
-
-	if (sepol_policy_file_create(&pf)) {
-		ERR(sh, "Out of memory!");
-		return -1;
-	}
-	sepol_policy_file_set_mem(pf, module_data, data_len);
-	sepol_policy_file_set_handle(pf, sh->sepolh);
-	if (module_data == NULL ||
-	    data_len == 0 ||
-	    sepol_module_package_info(pf, &file_type, module_name, version) == -1) {
-		sepol_policy_file_free(pf);
-		ERR(sh, "Could not parse module data.");
-		return -2;
-	}
-	sepol_policy_file_free(pf);
-	if (file_type != SEPOL_POLICY_MOD) {
-		ERR(sh, "Data did not represent a pp module. Please upgrade to the latest version of libsemanage to support hll modules.");
-		return -2;
-	}
-
-	return 0;
-}
-
-/* This function is used to preserve ABI compatibility with
- * versions of semodule using LIBSEMANAGE_1.0
- */
-int semanage_module_install_pp(semanage_handle_t * sh,
-			    char *module_data, size_t data_len)
-{
-	char *name = NULL;
-	char *version = NULL;
-	int status;
-
-	if ((status = parse_module_headers(sh, module_data, data_len, &name, &version)) != 0) {
-		goto cleanup;
-	}
-
-	status = semanage_module_install_hll(sh, module_data, data_len, name, "pp");
-
-cleanup:
-	free(name);
-	free(version);
-	return status;
-}
-
-int semanage_module_install_hll(semanage_handle_t * sh,
+int semanage_module_install(semanage_handle_t * sh,
 			    char *module_data, size_t data_len, const char *name, const char *ext_lang)
 {
 	if (sh->funcs->install == NULL) {
@@ -160,16 +97,6 @@ int semanage_module_extract(semanage_handle_t * sh,
 	return sh->funcs->extract(sh, modkey, extract_cil, mapped_data, data_len, modinfo);
 }
 
-/* Legacy function that remains to preserve ABI
- * compatibility. Please use semanage_module_install instead.
- */
-int semanage_module_upgrade(semanage_handle_t * sh,
-			    char *module_data, size_t data_len)
-{
-	return semanage_module_install_pp(sh, module_data, data_len);
-	
-}
-
 /* Legacy function that remains to preserve ABI
  * compatibility. Please use semanage_module_install_file instead.
  */
@@ -179,24 +106,6 @@ int semanage_module_upgrade_file(semanage_handle_t * sh,
 	return semanage_module_install_file(sh, module_name);
 }
 
-/* Legacy function that remains to preserve ABI
- * compatibility. Please use semanage_module_install instead.
- */
-int semanage_module_install_base(semanage_handle_t * sh,
-				 char *module_data, size_t data_len)
-{
-	return semanage_module_install_pp(sh, module_data, data_len);
-}
-
-/* Legacy function that remains to preserve ABI
- * compatibility. Please use semanage_module_install_file instead.
- */
-int semanage_module_install_base_file(semanage_handle_t * sh,
-				 const char *module_name)
-{
-	return semanage_module_install_file(sh, module_name);
-}
-
 int semanage_module_remove(semanage_handle_t * sh, char *module_name)
 {
 	if (sh->funcs->remove == NULL) {
@@ -780,7 +689,7 @@ int semanage_module_key_set_priority(semanage_handle_t *sh,
 }
 
 
-int semanage_module_get_enabled_1_1(semanage_handle_t *sh,
+int semanage_module_get_enabled(semanage_handle_t *sh,
 				const semanage_module_key_t *modkey,
 				int *enabled)
 {
@@ -800,11 +709,6 @@ int semanage_module_get_enabled_1_1(semanage_handle_t *sh,
 	return sh->funcs->get_enabled(sh, modkey, enabled);
 }
 
-int semanage_module_get_enabled_1_0(semanage_module_info_t *modinfo)
-{
-	return modinfo->enabled;
-}
-
 int semanage_module_set_enabled(semanage_handle_t *sh,
 				const semanage_module_key_t *modkey,
 				int enabled)
diff --git a/libsemanage/src/modules.h b/libsemanage/src/modules.h
index 2d3576fb15df..64d4a157f5ca 100644
--- a/libsemanage/src/modules.h
+++ b/libsemanage/src/modules.h
@@ -26,16 +26,9 @@
 
 #include "semanage/modules.h"
 
-int semanage_module_install_pp(semanage_handle_t * sh,
-			    char *module_data, size_t data_len);
-int semanage_module_install_hll(semanage_handle_t * sh,
-			    char *module_data, size_t data_len, const char *name, const char *ext_lang);
-int semanage_module_upgrade(semanage_handle_t * sh,
-			    char *module_data, size_t data_len);
+
 int semanage_module_upgrade_file(semanage_handle_t * sh,
 				 const char *module_name);
-int semanage_module_install_base(semanage_handle_t * sh,
-				 char *module_data, size_t data_len);
 int semanage_module_install_base_file(semanage_handle_t * sh,
 				 const char *module_name);
 
diff --git a/libsemanage/src/semanageswig_python.i b/libsemanage/src/semanageswig_python.i
index 8dd79fc24213..5f0113966962 100644
--- a/libsemanage/src/semanageswig_python.i
+++ b/libsemanage/src/semanageswig_python.i
@@ -30,8 +30,6 @@
 %}
 
 %include "stdint.i"
-%ignore semanage_module_install_pp;
-%ignore semanage_module_install_hll;
 
 %wrapper %{
 
-- 
2.28.0

[PATCH 4/4] libsemanage: Bump libsemanage.so version

[Petr Lautrbach]

It's due to the previous ABI incompatible change

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
---
 libsemanage/src/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libsemanage/src/Makefile b/libsemanage/src/Makefile
index a0eb3747d74b..ab6cae51f5c3 100644
--- a/libsemanage/src/Makefile
+++ b/libsemanage/src/Makefile
@@ -32,7 +32,7 @@ YACC = bison
 YFLAGS = -d
 
 VERSION = $(shell cat ../VERSION)
-LIBVERSION = 1
+LIBVERSION = 2
 
 LIBA=libsemanage.a
 TARGET=libsemanage.so
-- 
2.28.0

Re: [PATCH 4/4] libsemanage: Bump libsemanage.so version

[Stephen Smalley]

On Wed, Sep 30, 2020 at 10:51 AM Petr Lautrbach <plautrba@redhat.com> wrote:
>
> It's due to the previous ABI incompatible change
>
> Signed-off-by: Petr Lautrbach <plautrba@redhat.com>

My only real question is what are the implications for distros for
this change?  Would Fedora end up having to carry both so versions for
a time?  Or can you cleanly switch from the old to the new without
disruption?

Re: [PATCH 4/4] libsemanage: Bump libsemanage.so version

[Petr Lautrbach]

[-- Attachment #1: Type: text/plain, Size: 1834 bytes --]

On Wed, Sep 30, 2020 at 11:22:21AM -0400, Stephen Smalley wrote:
> On Wed, Sep 30, 2020 at 10:51 AM Petr Lautrbach <plautrba@redhat.com> wrote:
> >
> > It's due to the previous ABI incompatible change
> >
> > Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
> 
> My only real question is what are the implications for distros for
> this change?  Would Fedora end up having to carry both so versions for
> a time?  Or can you cleanly switch from the old to the new without
> disruption?
> 

Fedora and other distribution will need to temporary ship something like libsepol-compat and
libsemanage-compat with libsepol.so.1 resp libsemanage.so.1 in order not to
break buildroots. Also all packages which require so.1, see bellow, will have to
be rebuilt against so.2

# dnf repoquery --whatrequires libsepol.'so.1()(64bit)'
libselinux-utils-0:3.1-3.fc34.x86_64
libsemanage-0:3.1-2.fc33.x86_64
libsepol-devel-0:3.1-3.fc33.x86_64
parted-0:3.3-6.fc34.x86_64
policycoreutils-0:3.1-4.fc33.x86_64
python3-setools-0:4.3.0-5.fc33.x86_64
secilc-0:3.1-2.fc33.x86_64

# dnf -C repoquery --whatrequires 'libsemanage.so.1()(64bit)'
libsemanage-devel-0:3.1-2.fc33.x86_64
policycoreutils-0:3.1-4.fc33.x86_64
python3-libsemanage-0:3.1-2.fc33.x86_64
shadow-utils-2:4.8.1-4.fc33.x86_64
sssd-common-0:2.3.1-4.fc33.x86_64
sssd-ipa-0:2.3.1-4.fc33.x86_64

I've experienced with this, builds are available in
https://copr.fedorainfracloud.org/coprs/plautrba/selinux-fedora/

E.g. for libsemanage, I've added

+%set_build_flags
+CFLAGS="$CFLAGS -fno-semantic-interposition"
+sed -i 's/LIBVERSION = 2/LIBVERSION = 1/' src/Makefile
+%make_build
+cp src/libsemanage.so.1 ${RPM_BUILD_ROOT}/%{_libdir}/libsemanage.so.1

to the spec file in order to get libsemanage.so.1 which is shipped by
libsemanage.so.1

Petr

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/4] libsemanage: Bump libsemanage.so version

[Stephen Smalley]

On Wed, Sep 30, 2020 at 11:56 AM Petr Lautrbach <plautrba@redhat.com> wrote:
>
> On Wed, Sep 30, 2020 at 11:22:21AM -0400, Stephen Smalley wrote:
> > On Wed, Sep 30, 2020 at 10:51 AM Petr Lautrbach <plautrba@redhat.com> wrote:
> > >
> > > It's due to the previous ABI incompatible change
> > >
> > > Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
> >
> > My only real question is what are the implications for distros for
> > this change?  Would Fedora end up having to carry both so versions for
> > a time?  Or can you cleanly switch from the old to the new without
> > disruption?
> >
>
> Fedora and other distribution will need to temporary ship something like libsepol-compat and
> libsemanage-compat with libsepol.so.1 resp libsemanage.so.1 in order not to
> break buildroots. Also all packages which require so.1, see bellow, will have to
> be rebuilt against so.2
>
> # dnf repoquery --whatrequires libsepol.'so.1()(64bit)'
> libselinux-utils-0:3.1-3.fc34.x86_64
> libsemanage-0:3.1-2.fc33.x86_64
> libsepol-devel-0:3.1-3.fc33.x86_64
> parted-0:3.3-6.fc34.x86_64
> policycoreutils-0:3.1-4.fc33.x86_64
> python3-setools-0:4.3.0-5.fc33.x86_64
> secilc-0:3.1-2.fc33.x86_64
>
> # dnf -C repoquery --whatrequires 'libsemanage.so.1()(64bit)'
> libsemanage-devel-0:3.1-2.fc33.x86_64
> policycoreutils-0:3.1-4.fc33.x86_64
> python3-libsemanage-0:3.1-2.fc33.x86_64
> shadow-utils-2:4.8.1-4.fc33.x86_64
> sssd-common-0:2.3.1-4.fc33.x86_64
> sssd-ipa-0:2.3.1-4.fc33.x86_64
>
> I've experienced with this, builds are available in
> https://copr.fedorainfracloud.org/coprs/plautrba/selinux-fedora/
>
> E.g. for libsemanage, I've added
>
> +%set_build_flags
> +CFLAGS="$CFLAGS -fno-semantic-interposition"
> +sed -i 's/LIBVERSION = 2/LIBVERSION = 1/' src/Makefile
> +%make_build
> +cp src/libsemanage.so.1 ${RPM_BUILD_ROOT}/%{_libdir}/libsemanage.so.1
>
> to the spec file in order to get libsemanage.so.1 which is shipped by
> libsemanage.so.1

The parted dependency looks suspect; seems to be an incorrect linking
with libsepol despite not directly calling any sepol functions.
Aside from that, if we have to bump the so version and deal with
compat packages anyway, should we go ahead and purge all of the other
deprecated functions in libsepol and libsemanage (grep -ri deprecated
libsepol libsemanage)?

Re: [PATCH 4/4] libsemanage: Bump libsemanage.so version

[Petr Lautrbach]

[-- Attachment #1: Type: text/plain, Size: 3067 bytes --]

On Thu, Oct 01, 2020 at 10:18:35AM -0400, Stephen Smalley wrote:
> On Wed, Sep 30, 2020 at 11:56 AM Petr Lautrbach <plautrba@redhat.com> wrote:
> >
> > On Wed, Sep 30, 2020 at 11:22:21AM -0400, Stephen Smalley wrote:
> > > On Wed, Sep 30, 2020 at 10:51 AM Petr Lautrbach <plautrba@redhat.com> wrote:
> > > >
> > > > It's due to the previous ABI incompatible change
> > > >
> > > > Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
> > >
> > > My only real question is what are the implications for distros for
> > > this change?  Would Fedora end up having to carry both so versions for
> > > a time?  Or can you cleanly switch from the old to the new without
> > > disruption?
> > >
> >
> > Fedora and other distribution will need to temporary ship something like libsepol-compat and
> > libsemanage-compat with libsepol.so.1 resp libsemanage.so.1 in order not to
> > break buildroots. Also all packages which require so.1, see bellow, will have to
> > be rebuilt against so.2
> >
> > # dnf repoquery --whatrequires libsepol.'so.1()(64bit)'
> > libselinux-utils-0:3.1-3.fc34.x86_64
> > libsemanage-0:3.1-2.fc33.x86_64
> > libsepol-devel-0:3.1-3.fc33.x86_64
> > parted-0:3.3-6.fc34.x86_64
> > policycoreutils-0:3.1-4.fc33.x86_64
> > python3-setools-0:4.3.0-5.fc33.x86_64
> > secilc-0:3.1-2.fc33.x86_64
> >
> > # dnf -C repoquery --whatrequires 'libsemanage.so.1()(64bit)'
> > libsemanage-devel-0:3.1-2.fc33.x86_64
> > policycoreutils-0:3.1-4.fc33.x86_64
> > python3-libsemanage-0:3.1-2.fc33.x86_64
> > shadow-utils-2:4.8.1-4.fc33.x86_64
> > sssd-common-0:2.3.1-4.fc33.x86_64
> > sssd-ipa-0:2.3.1-4.fc33.x86_64
> >
> > I've experienced with this, builds are available in
> > https://copr.fedorainfracloud.org/coprs/plautrba/selinux-fedora/
> >
> > E.g. for libsemanage, I've added
> >
> > +%set_build_flags
> > +CFLAGS="$CFLAGS -fno-semantic-interposition"
> > +sed -i 's/LIBVERSION = 2/LIBVERSION = 1/' src/Makefile
> > +%make_build
> > +cp src/libsemanage.so.1 ${RPM_BUILD_ROOT}/%{_libdir}/libsemanage.so.1
> >
> > to the spec file in order to get libsemanage.so.1 which is shipped by
> > libsemanage.so.1
> 
> The parted dependency looks suspect; seems to be an incorrect linking
> with libsepol despite not directly calling any sepol functions.
> Aside from that, if we have to bump the so version and deal with
> compat packages anyway, should we go ahead and purge all of the other
> deprecated functions in libsepol and libsemanage (grep -ri deprecated
> libsepol libsemanage)?
> 

I'd like to ship compat only for short time until all dependent components are
rebuilt.

Purging deprecated functions could have much bigger impact than this patchset as it affects API. With
this change, it's generally enough to rebuild a component. If we drop functions
and change API, different software could stop work. There are only few packages
using libsepol and libsemanage directly, but there might be much bigger group of
python or ruby scripts using deprecated symbols like matchpathcon*()

Petr

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/4] libsemanage: Bump libsemanage.so version

[Stephen Smalley]

On Thu, Oct 1, 2020 at 12:56 PM Petr Lautrbach <plautrba@redhat.com> wrote:
>
> On Thu, Oct 01, 2020 at 10:18:35AM -0400, Stephen Smalley wrote:
> > On Wed, Sep 30, 2020 at 11:56 AM Petr Lautrbach <plautrba@redhat.com> wrote:
> > >
> > > On Wed, Sep 30, 2020 at 11:22:21AM -0400, Stephen Smalley wrote:
> > > > On Wed, Sep 30, 2020 at 10:51 AM Petr Lautrbach <plautrba@redhat.com> wrote:
> > > > >
> > > > > It's due to the previous ABI incompatible change
> > > > >
> > > > > Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
> > > >
> > > > My only real question is what are the implications for distros for
> > > > this change?  Would Fedora end up having to carry both so versions for
> > > > a time?  Or can you cleanly switch from the old to the new without
> > > > disruption?
> > > >
> > >
> > > Fedora and other distribution will need to temporary ship something like libsepol-compat and
> > > libsemanage-compat with libsepol.so.1 resp libsemanage.so.1 in order not to
> > > break buildroots. Also all packages which require so.1, see bellow, will have to
> > > be rebuilt against so.2
> > >
> > > # dnf repoquery --whatrequires libsepol.'so.1()(64bit)'
> > > libselinux-utils-0:3.1-3.fc34.x86_64
> > > libsemanage-0:3.1-2.fc33.x86_64
> > > libsepol-devel-0:3.1-3.fc33.x86_64
> > > parted-0:3.3-6.fc34.x86_64
> > > policycoreutils-0:3.1-4.fc33.x86_64
> > > python3-setools-0:4.3.0-5.fc33.x86_64
> > > secilc-0:3.1-2.fc33.x86_64
> > >
> > > # dnf -C repoquery --whatrequires 'libsemanage.so.1()(64bit)'
> > > libsemanage-devel-0:3.1-2.fc33.x86_64
> > > policycoreutils-0:3.1-4.fc33.x86_64
> > > python3-libsemanage-0:3.1-2.fc33.x86_64
> > > shadow-utils-2:4.8.1-4.fc33.x86_64
> > > sssd-common-0:2.3.1-4.fc33.x86_64
> > > sssd-ipa-0:2.3.1-4.fc33.x86_64
> > >
> > > I've experienced with this, builds are available in
> > > https://copr.fedorainfracloud.org/coprs/plautrba/selinux-fedora/
> > >
> > > E.g. for libsemanage, I've added
> > >
> > > +%set_build_flags
> > > +CFLAGS="$CFLAGS -fno-semantic-interposition"
> > > +sed -i 's/LIBVERSION = 2/LIBVERSION = 1/' src/Makefile
> > > +%make_build
> > > +cp src/libsemanage.so.1 ${RPM_BUILD_ROOT}/%{_libdir}/libsemanage.so.1
> > >
> > > to the spec file in order to get libsemanage.so.1 which is shipped by
> > > libsemanage.so.1
> >
> > The parted dependency looks suspect; seems to be an incorrect linking
> > with libsepol despite not directly calling any sepol functions.
> > Aside from that, if we have to bump the so version and deal with
> > compat packages anyway, should we go ahead and purge all of the other
> > deprecated functions in libsepol and libsemanage (grep -ri deprecated
> > libsepol libsemanage)?
> >
>
> I'd like to ship compat only for short time until all dependent components are
> rebuilt.
>
> Purging deprecated functions could have much bigger impact than this patchset as it affects API. With
> this change, it's generally enough to rebuild a component. If we drop functions
> and change API, different software could stop work. There are only few packages
> using libsepol and libsemanage directly, but there might be much bigger group of
> python or ruby scripts using deprecated symbols like matchpathcon*()

Yes, I just meant libsepol and libsemanage deprecated functions not
libselinux (so not matchpathcon) since you are already bumping the so
version.  But it's fine if you don't want to do it at the same time.

Re: [PATCH 4/4] libsemanage: Bump libsemanage.so version

[Petr Lautrbach]

[-- Attachment #1: Type: text/plain, Size: 3801 bytes --]

On Thu, Oct 01, 2020 at 01:08:27PM -0400, Stephen Smalley wrote:
> On Thu, Oct 1, 2020 at 12:56 PM Petr Lautrbach <plautrba@redhat.com> wrote:
> >
> > On Thu, Oct 01, 2020 at 10:18:35AM -0400, Stephen Smalley wrote:
> > > On Wed, Sep 30, 2020 at 11:56 AM Petr Lautrbach <plautrba@redhat.com> wrote:
> > > >
> > > > On Wed, Sep 30, 2020 at 11:22:21AM -0400, Stephen Smalley wrote:
> > > > > On Wed, Sep 30, 2020 at 10:51 AM Petr Lautrbach <plautrba@redhat.com> wrote:
> > > > > >
> > > > > > It's due to the previous ABI incompatible change
> > > > > >
> > > > > > Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
> > > > >
> > > > > My only real question is what are the implications for distros for
> > > > > this change?  Would Fedora end up having to carry both so versions for
> > > > > a time?  Or can you cleanly switch from the old to the new without
> > > > > disruption?
> > > > >
> > > >
> > > > Fedora and other distribution will need to temporary ship something like libsepol-compat and
> > > > libsemanage-compat with libsepol.so.1 resp libsemanage.so.1 in order not to
> > > > break buildroots. Also all packages which require so.1, see bellow, will have to
> > > > be rebuilt against so.2
> > > >
> > > > # dnf repoquery --whatrequires libsepol.'so.1()(64bit)'
> > > > libselinux-utils-0:3.1-3.fc34.x86_64
> > > > libsemanage-0:3.1-2.fc33.x86_64
> > > > libsepol-devel-0:3.1-3.fc33.x86_64
> > > > parted-0:3.3-6.fc34.x86_64
> > > > policycoreutils-0:3.1-4.fc33.x86_64
> > > > python3-setools-0:4.3.0-5.fc33.x86_64
> > > > secilc-0:3.1-2.fc33.x86_64
> > > >
> > > > # dnf -C repoquery --whatrequires 'libsemanage.so.1()(64bit)'
> > > > libsemanage-devel-0:3.1-2.fc33.x86_64
> > > > policycoreutils-0:3.1-4.fc33.x86_64
> > > > python3-libsemanage-0:3.1-2.fc33.x86_64
> > > > shadow-utils-2:4.8.1-4.fc33.x86_64
> > > > sssd-common-0:2.3.1-4.fc33.x86_64
> > > > sssd-ipa-0:2.3.1-4.fc33.x86_64
> > > >
> > > > I've experienced with this, builds are available in
> > > > https://copr.fedorainfracloud.org/coprs/plautrba/selinux-fedora/
> > > >
> > > > E.g. for libsemanage, I've added
> > > >
> > > > +%set_build_flags
> > > > +CFLAGS="$CFLAGS -fno-semantic-interposition"
> > > > +sed -i 's/LIBVERSION = 2/LIBVERSION = 1/' src/Makefile
> > > > +%make_build
> > > > +cp src/libsemanage.so.1 ${RPM_BUILD_ROOT}/%{_libdir}/libsemanage.so.1
> > > >
> > > > to the spec file in order to get libsemanage.so.1 which is shipped by
> > > > libsemanage.so.1
> > >
> > > The parted dependency looks suspect; seems to be an incorrect linking
> > > with libsepol despite not directly calling any sepol functions.
> > > Aside from that, if we have to bump the so version and deal with
> > > compat packages anyway, should we go ahead and purge all of the other
> > > deprecated functions in libsepol and libsemanage (grep -ri deprecated
> > > libsepol libsemanage)?
> > >
> >
> > I'd like to ship compat only for short time until all dependent components are
> > rebuilt.
> >
> > Purging deprecated functions could have much bigger impact than this patchset as it affects API. With
> > this change, it's generally enough to rebuild a component. If we drop functions
> > and change API, different software could stop work. There are only few packages
> > using libsepol and libsemanage directly, but there might be much bigger group of
> > python or ruby scripts using deprecated symbols like matchpathcon*()
> 
> Yes, I just meant libsepol and libsemanage deprecated functions not
> libselinux (so not matchpathcon) since you are already bumping the so
> version.  But it's fine if you don't want to do it at the same time.
> 

I see, I missed that point, sorry. It seems to be reasonable, but I'll check it
again tomorrow.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/4] libsemanage: Bump libsemanage.so version

[Nicolas Iooss]

On Thu, Oct 1, 2020 at 7:48 PM Petr Lautrbach <plautrba@redhat.com> wrote:
>
> On Thu, Oct 01, 2020 at 01:08:27PM -0400, Stephen Smalley wrote:
> > On Thu, Oct 1, 2020 at 12:56 PM Petr Lautrbach <plautrba@redhat.com> wrote:
> > >
> > > On Thu, Oct 01, 2020 at 10:18:35AM -0400, Stephen Smalley wrote:
> > > > On Wed, Sep 30, 2020 at 11:56 AM Petr Lautrbach <plautrba@redhat.com> wrote:
> > > > >
> > > > > On Wed, Sep 30, 2020 at 11:22:21AM -0400, Stephen Smalley wrote:
> > > > > > On Wed, Sep 30, 2020 at 10:51 AM Petr Lautrbach <plautrba@redhat.com> wrote:
> > > > > > >
> > > > > > > It's due to the previous ABI incompatible change
> > > > > > >
> > > > > > > Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
> > > > > >
> > > > > > My only real question is what are the implications for distros for
> > > > > > this change?  Would Fedora end up having to carry both so versions for
> > > > > > a time?  Or can you cleanly switch from the old to the new without
> > > > > > disruption?
> > > > > >
> > > > >
> > > > > Fedora and other distribution will need to temporary ship something like libsepol-compat and
> > > > > libsemanage-compat with libsepol.so.1 resp libsemanage.so.1 in order not to
> > > > > break buildroots. Also all packages which require so.1, see bellow, will have to
> > > > > be rebuilt against so.2
> > > > >
> > > > > # dnf repoquery --whatrequires libsepol.'so.1()(64bit)'
> > > > > libselinux-utils-0:3.1-3.fc34.x86_64
> > > > > libsemanage-0:3.1-2.fc33.x86_64
> > > > > libsepol-devel-0:3.1-3.fc33.x86_64
> > > > > parted-0:3.3-6.fc34.x86_64
> > > > > policycoreutils-0:3.1-4.fc33.x86_64
> > > > > python3-setools-0:4.3.0-5.fc33.x86_64
> > > > > secilc-0:3.1-2.fc33.x86_64
> > > > >
> > > > > # dnf -C repoquery --whatrequires 'libsemanage.so.1()(64bit)'
> > > > > libsemanage-devel-0:3.1-2.fc33.x86_64
> > > > > policycoreutils-0:3.1-4.fc33.x86_64
> > > > > python3-libsemanage-0:3.1-2.fc33.x86_64
> > > > > shadow-utils-2:4.8.1-4.fc33.x86_64
> > > > > sssd-common-0:2.3.1-4.fc33.x86_64
> > > > > sssd-ipa-0:2.3.1-4.fc33.x86_64
> > > > >
> > > > > I've experienced with this, builds are available in
> > > > > https://copr.fedorainfracloud.org/coprs/plautrba/selinux-fedora/
> > > > >
> > > > > E.g. for libsemanage, I've added
> > > > >
> > > > > +%set_build_flags
> > > > > +CFLAGS="$CFLAGS -fno-semantic-interposition"
> > > > > +sed -i 's/LIBVERSION = 2/LIBVERSION = 1/' src/Makefile
> > > > > +%make_build
> > > > > +cp src/libsemanage.so.1 ${RPM_BUILD_ROOT}/%{_libdir}/libsemanage.so.1
> > > > >
> > > > > to the spec file in order to get libsemanage.so.1 which is shipped by
> > > > > libsemanage.so.1
> > > >
> > > > The parted dependency looks suspect; seems to be an incorrect linking
> > > > with libsepol despite not directly calling any sepol functions.
> > > > Aside from that, if we have to bump the so version and deal with
> > > > compat packages anyway, should we go ahead and purge all of the other
> > > > deprecated functions in libsepol and libsemanage (grep -ri deprecated
> > > > libsepol libsemanage)?
> > > >
> > >
> > > I'd like to ship compat only for short time until all dependent components are
> > > rebuilt.
> > >
> > > Purging deprecated functions could have much bigger impact than this patchset as it affects API. With
> > > this change, it's generally enough to rebuild a component. If we drop functions
> > > and change API, different software could stop work. There are only few packages
> > > using libsepol and libsemanage directly, but there might be much bigger group of
> > > python or ruby scripts using deprecated symbols like matchpathcon*()
> >
> > Yes, I just meant libsepol and libsemanage deprecated functions not
> > libselinux (so not matchpathcon) since you are already bumping the so
> > version.  But it's fine if you don't want to do it at the same time.
> >
>
> I see, I missed that point, sorry. It seems to be reasonable, but I'll check it
> again tomorrow.

Hello,
I have another question: why is bumping the .so version needed? As we
are not changing the ABI of a "linked symbol" (thanks to using
versioned symbols, with .map files), if we do not bump the .so
version, programs that were built with libsepol.so from years ago will
just stop working due to missing symbols, with an error message which
will be quite clear (and this only if they were using deprecated
symbols). In my humble opinion, bumping the .so version is most
required when the calling convention of a non-versioned symbol
changes, but this leads to unexpected execution paths.

Nevertheless I did not spend time to search for a document that would
explain why bumping the .so version would be recommended when removing
symbols. If you know one, could you please add a reference to it in
the commit description ("Following guidelines from https://...) and/or
to some documentation?

>From a "distro maintainer point of view" (for Arch Linux), having just
spent a considerable amount of time due to breaking changes in the
last release of PAM, I am not eager to spend time dealing with finding
clever ways to smoothly upgrade the system if there is no
easy&straightforward way to do this. Introducing a transition package
for each library which is bumped is acceptable to me, but if the
release after the next one bumps the version again, introducing
another set of transition packages will begin to be quite painful. In
short: I agree to remove the deprecated functions in order to "bump
the .so version only once", as suggested.

Cheers,
Nicolas

Re: [PATCH 4/4] libsemanage: Bump libsemanage.so version

[Stephen Smalley]

On Fri, Oct 2, 2020 at 2:53 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> Hello,
> I have another question: why is bumping the .so version needed? As we
> are not changing the ABI of a "linked symbol" (thanks to using
> versioned symbols, with .map files), if we do not bump the .so
> version, programs that were built with libsepol.so from years ago will
> just stop working due to missing symbols, with an error message which
> will be quite clear (and this only if they were using deprecated
> symbols). In my humble opinion, bumping the .so version is most
> required when the calling convention of a non-versioned symbol
> changes, but this leads to unexpected execution paths.
>
> Nevertheless I did not spend time to search for a document that would
> explain why bumping the .so version would be recommended when removing
> symbols. If you know one, could you please add a reference to it in
> the commit description ("Following guidelines from https://...) and/or
> to some documentation?
>
> >From a "distro maintainer point of view" (for Arch Linux), having just
> spent a considerable amount of time due to breaking changes in the
> last release of PAM, I am not eager to spend time dealing with finding
> clever ways to smoothly upgrade the system if there is no
> easy&straightforward way to do this. Introducing a transition package
> for each library which is bumped is acceptable to me, but if the
> release after the next one bumps the version again, introducing
> another set of transition packages will begin to be quite painful. In
> short: I agree to remove the deprecated functions in order to "bump
> the .so version only once", as suggested.

My original understanding of library ABI compat requirements came from
Ulrich Drepper's paper,
https://www.akkadia.org/drepper/dsohowto.pdf

Looks like Debian's policy is here:
https://www.debian.org/doc/debian-policy/ch-sharedlibs.html#run-time-shared-libraries

IIUC, if we follow the rules laid out by both, removing an old symbol
version entirely is incompatible and requires a SONAME change.  That
said, commit c3f9492d7ff05bdc8581817655ad05bc1e1174b8 ("selinux:
Remove legacy local boolean and user code") was technically an
incompatible change; it left the symbols in place but made them always
fail or ignore no-longer-used parameters, which isn't truly
compatible, and we didn't change the SONAMEs then.

I'd personally be ok with not changing the SONAME as long as these
interfaces were only ever used by the selinux userspace code itself or
only by really ancient code that is no longer in use by any supported
distribution but I don't think that flies with e.g. the Debian policy.

Re: [PATCH 4/4] libsemanage: Bump libsemanage.so version

[Nicolas Iooss]

On Fri, Oct 2, 2020 at 4:50 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Fri, Oct 2, 2020 at 2:53 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
> >
> > Hello,
> > I have another question: why is bumping the .so version needed? As we
> > are not changing the ABI of a "linked symbol" (thanks to using
> > versioned symbols, with .map files), if we do not bump the .so
> > version, programs that were built with libsepol.so from years ago will
> > just stop working due to missing symbols, with an error message which
> > will be quite clear (and this only if they were using deprecated
> > symbols). In my humble opinion, bumping the .so version is most
> > required when the calling convention of a non-versioned symbol
> > changes, but this leads to unexpected execution paths.
> >
> > Nevertheless I did not spend time to search for a document that would
> > explain why bumping the .so version would be recommended when removing
> > symbols. If you know one, could you please add a reference to it in
> > the commit description ("Following guidelines from https://...) and/or
> > to some documentation?
> >
> > >From a "distro maintainer point of view" (for Arch Linux), having just
> > spent a considerable amount of time due to breaking changes in the
> > last release of PAM, I am not eager to spend time dealing with finding
> > clever ways to smoothly upgrade the system if there is no
> > easy&straightforward way to do this. Introducing a transition package
> > for each library which is bumped is acceptable to me, but if the
> > release after the next one bumps the version again, introducing
> > another set of transition packages will begin to be quite painful. In
> > short: I agree to remove the deprecated functions in order to "bump
> > the .so version only once", as suggested.
>
> My original understanding of library ABI compat requirements came from
> Ulrich Drepper's paper,
> https://www.akkadia.org/drepper/dsohowto.pdf
>
> Looks like Debian's policy is here:
> https://www.debian.org/doc/debian-policy/ch-sharedlibs.html#run-time-shared-libraries
>
> IIUC, if we follow the rules laid out by both, removing an old symbol
> version entirely is incompatible and requires a SONAME change.  That
> said, commit c3f9492d7ff05bdc8581817655ad05bc1e1174b8 ("selinux:
> Remove legacy local boolean and user code") was technically an
> incompatible change; it left the symbols in place but made them always
> fail or ignore no-longer-used parameters, which isn't truly
> compatible, and we didn't change the SONAMEs then.
>
> I'd personally be ok with not changing the SONAME as long as these
> interfaces were only ever used by the selinux userspace code itself or
> only by really ancient code that is no longer in use by any supported
> distribution but I don't think that flies with e.g. the Debian policy.

Thanks for the details. Debian policy makes sense and I agree with
bumping the version in the SONAME.

Nicolas

Re: [PATCH 4/4] libsemanage: Bump libsemanage.so version

[Petr Lautrbach]

[-- Attachment #1: Type: text/plain, Size: 4196 bytes --]

Bcc: 
Subject: Re: [PATCH 4/4] libsemanage: Bump libsemanage.so version
Reply-To: 
In-Reply-To: <CAJfZ7=m5bwek=R_6gObMGdAKXNYSUwZU-rWv7EPAhw8SQU_vfg@mail.gmail.com>

On Fri, Oct 02, 2020 at 05:41:53PM +0200, Nicolas Iooss wrote:
> On Fri, Oct 2, 2020 at 4:50 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > On Fri, Oct 2, 2020 at 2:53 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
> > >
> > > Hello,
> > > I have another question: why is bumping the .so version needed? As we
> > > are not changing the ABI of a "linked symbol" (thanks to using
> > > versioned symbols, with .map files), if we do not bump the .so
> > > version, programs that were built with libsepol.so from years ago will
> > > just stop working due to missing symbols, with an error message which
> > > will be quite clear (and this only if they were using deprecated
> > > symbols). In my humble opinion, bumping the .so version is most
> > > required when the calling convention of a non-versioned symbol
> > > changes, but this leads to unexpected execution paths.
> > >
> > > Nevertheless I did not spend time to search for a document that would
> > > explain why bumping the .so version would be recommended when removing
> > > symbols. If you know one, could you please add a reference to it in
> > > the commit description ("Following guidelines from https://...) and/or
> > > to some documentation?
> > >
> > > >From a "distro maintainer point of view" (for Arch Linux), having just
> > > spent a considerable amount of time due to breaking changes in the
> > > last release of PAM, I am not eager to spend time dealing with finding
> > > clever ways to smoothly upgrade the system if there is no
> > > easy&straightforward way to do this. Introducing a transition package
> > > for each library which is bumped is acceptable to me, but if the
> > > release after the next one bumps the version again, introducing
> > > another set of transition packages will begin to be quite painful. In
> > > short: I agree to remove the deprecated functions in order to "bump
> > > the .so version only once", as suggested.
> >
> > My original understanding of library ABI compat requirements came from
> > Ulrich Drepper's paper,
> > https://www.akkadia.org/drepper/dsohowto.pdf
> >
> > Looks like Debian's policy is here:
> > https://www.debian.org/doc/debian-policy/ch-sharedlibs.html#run-time-shared-libraries
> >
> > IIUC, if we follow the rules laid out by both, removing an old symbol
> > version entirely is incompatible and requires a SONAME change.  That
> > said, commit c3f9492d7ff05bdc8581817655ad05bc1e1174b8 ("selinux:
> > Remove legacy local boolean and user code") was technically an
> > incompatible change; it left the symbols in place but made them always
> > fail or ignore no-longer-used parameters, which isn't truly
> > compatible, and we didn't change the SONAMEs then.
> >
> > I'd personally be ok with not changing the SONAME as long as these
> > interfaces were only ever used by the selinux userspace code itself or
> > only by really ancient code that is no longer in use by any supported
> > distribution but I don't think that flies with e.g. the Debian policy.
> 
> Thanks for the details. Debian policy makes sense and I agree with
> bumping the version in the SONAME.
> 
> Nicolas
> 

Thanks, I'll prepare another patchset with improved commit messages.

In the mean time I'm looking into removing deprecated symbols from libsepol as it's
supposed to be required only by selinux components. So far I've found that
deprecated sepol_check_context() is used in chkcon utility. As Fedora doesn't ship
this tool I incline to remove it from libsepol as well.

And there's also

/* Deprecated */
struct sepol_handle sepol_compat_handle = {
	.msg_callback = sepol_msg_default_handler,
	.msg_callback_arg = NULL,
};

void sepol_debug(int on)
{
	sepol_compat_handle.msg_callback = (on) ?
	    sepol_msg_default_handler : NULL;
}

/* End deprecated */

which is used on few places internally.


Later I'll check whether sssd uses any on deprecated libsemanage symbol and
decide what to do.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/4] libsemanage: Bump libsemanage.so version

[Stephen Smalley]

On Wed, Oct 7, 2020 at 5:50 AM Petr Lautrbach <plautrba@redhat.com> wrote:
> Thanks, I'll prepare another patchset with improved commit messages.
>
> In the mean time I'm looking into removing deprecated symbols from libsepol as it's
> supposed to be required only by selinux components. So far I've found that
> deprecated sepol_check_context() is used in chkcon utility. As Fedora doesn't ship
> this tool I incline to remove it from libsepol as well.

Looks like it is also used by setfiles and sefcontext_compile at
least.  Might have been copied into external packages as well, e.g.
busybox.  So that one might need to stay.  The reason it was
deprecated was because it predated the introduction of the use of
sepol handles and relied on global state set previously via
sepol_set_policydb_from_file(), so I think the idea was to convert
over to using sepol_context_check() instead (but this requires a much
longer sequence of calls, ala h = sepol_handle_create();
sepol_policy_file_create(&pf); sepol_policy_file_set_fp(pf, fp);
sepol_policy_file_set_handle(pf, h); sepol_policydb_create(&policydb);
sepol_policydb_read(policydb, pf); sepol_context_from_string(h,
string, &ctx); sepol_context_check(h, policydb, ctx);).  Probably not
worth the trouble now.

> And there's also
>
> /* Deprecated */
> struct sepol_handle sepol_compat_handle = {
>         .msg_callback = sepol_msg_default_handler,
>         .msg_callback_arg = NULL,
> };
>
> void sepol_debug(int on)
> {
>         sepol_compat_handle.msg_callback = (on) ?
>             sepol_msg_default_handler : NULL;
> }
>
> /* End deprecated */
>
> which is used on few places internally.
>
>
> Later I'll check whether sssd uses any on deprecated libsemanage symbol and
> decide what to do.

The main ones in libsepol that I was referencing were the ones in
libsepol/src/deprecated_funcs.c.

Re: [PATCH 4/4] libsemanage: Bump libsemanage.so version

[Dominick Grift]

Stephen Smalley <stephen.smalley.work@gmail.com> writes:

> On Wed, Oct 7, 2020 at 5:50 AM Petr Lautrbach <plautrba@redhat.com> wrote:
>> Thanks, I'll prepare another patchset with improved commit messages.
>>
>> In the mean time I'm looking into removing deprecated symbols from libsepol as it's
>> supposed to be required only by selinux components. So far I've found that
>> deprecated sepol_check_context() is used in chkcon utility. As Fedora doesn't ship
>> this tool I incline to remove it from libsepol as well.
>
> Looks like it is also used by setfiles and sefcontext_compile at
> least.  Might have been copied into external packages as well, e.g.
> busybox.  So that one might need to stay.  The reason it was
> deprecated was because it predated the introduction of the use of
> sepol handles and relied on global state set previously via
> sepol_set_policydb_from_file(), so I think the idea was to convert
> over to using sepol_context_check() instead (but this requires a much
> longer sequence of calls, ala h = sepol_handle_create();
> sepol_policy_file_create(&pf); sepol_policy_file_set_fp(pf, fp);
> sepol_policy_file_set_handle(pf, h); sepol_policydb_create(&policydb);
> sepol_policydb_read(policydb, pf); sepol_context_from_string(h,
> string, &ctx); sepol_context_check(h, policydb, ctx);).  Probably not
> worth the trouble now.

BTW Busybox urgently could use some modernization. It might not even
work fully anymore with recent kernels. For example its a user space
object manager (for the passwd class) and it does not use
selinux_check_access()

Theres also just generally a lot old stuff that has been improved since

It still works on Linux 5.4 (I use it with OpenWrt) but compiling it
floods the screen with "deprecation notices". Most notably for
selinux_context_t

If and once OpenWrt moves to Linux 5.9 (new LTS) some functionality may
no longer work

>
>> And there's also
>>
>> /* Deprecated */
>> struct sepol_handle sepol_compat_handle = {
>>         .msg_callback = sepol_msg_default_handler,
>>         .msg_callback_arg = NULL,
>> };
>>
>> void sepol_debug(int on)
>> {
>>         sepol_compat_handle.msg_callback = (on) ?
>>             sepol_msg_default_handler : NULL;
>> }
>>
>> /* End deprecated */
>>
>> which is used on few places internally.
>>
>>
>> Later I'll check whether sssd uses any on deprecated libsemanage symbol and
>> decide what to do.
>
> The main ones in libsepol that I was referencing were the ones in
> libsepol/src/deprecated_funcs.c.

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift