security_context_t marked as deprecated in libselinux 3.1
Hi all, Per the following commit in upstream SELinux, security_context_t has been marked as deprecated, generating complains with -Wdeprecated-declarations: https://github.com/SELinuxProject/selinux/commit/7a124ca2758136f49cc38efc26fb1a2d385ecfd9 This can be seen with Debian GID when building contrib/selinux/, as it we have libselinux 3.1 there. Per the upstream repo, security_context_t maps to char * in include/selinux/selinux.h, so we can get rid easily of the warnings with the attached that replaces the references to security_context_t. Funnily, our code already mixes both definitions, see for example sepgsql_set_client_label, so this clarifies things. Any thoughts? -- Michael diff --git a/contrib/sepgsql/label.c b/contrib/sepgsql/label.c index 32e405530b..b00b91df5a 100644 --- a/contrib/sepgsql/label.c +++ b/contrib/sepgsql/label.c @@ -120,7 +120,7 @@ sepgsql_set_client_label(const char *new_label) tcontext = client_label_peer; else { - if (security_check_context_raw((security_context_t) new_label) < 0) + if (security_check_context_raw(new_label) < 0) ereport(ERROR, (errcode(ERRCODE_INVALID_NAME), errmsg("SELinux: invalid security label: \"%s\"", @@ -453,9 +453,9 @@ sepgsql_get_label(Oid classId, Oid objectId, int32 subId) object.objectSubId = subId; label = GetSecurityLabel(&object, SEPGSQL_LABEL_TAG); - if (!label || security_check_context_raw((security_context_t) label)) + if (!label || security_check_context_raw(label)) { - security_context_t unlabeled; + char *unlabeled; if (security_get_initial_context_raw("unlabeled", &unlabeled) < 0) ereport(ERROR, @@ -487,7 +487,7 @@ sepgsql_object_relabel(const ObjectAddress *object, const char *seclabel) * context of selinux. */ if (seclabel && - security_check_context_raw((security_context_t) seclabel) < 0) + security_check_context_raw(seclabel) < 0) ereport(ERROR, (errcode(ERRCODE_INVALID_NAME), errmsg("SELinux: invalid security label: \"%s\"", seclabel))); @@ -725,7 +725,7 @@ exec_object_restorecon(struct selabel_handle *sehnd, Oid catalogId) char *objname; int objtype = 1234; ObjectAddress object; - security_context_t context; + char *context; /* * The way to determine object name depends on object classes. So, any diff --git a/contrib/sepgsql/selinux.c b/contrib/sepgsql/selinux.c index 9fdc810f2e..2695e88f23 100644 --- a/contrib/sepgsql/selinux.c +++ b/contrib/sepgsql/selinux.c @@ -768,8 +768,8 @@ sepgsql_compute_avd(const char *scontext, * Ask SELinux what is allowed set of permissions on a pair of the * security contexts and the given object class. */ - if (security_compute_av_flags_raw((security_context_t) scontext, - (security_context_t) tcontext, + if (security_compute_av_flags_raw(scontext, + tcontext, tclass_ex, 0, &avd_ex) < 0) ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR), @@ -838,7 +838,7 @@ sepgsql_compute_create(const char *scontext, uint16 tclass, const char *objname) { - security_context_t ncontext; + char *ncontext; security_class_t tclass_ex; const char *tclass_name; char *result; @@ -853,8 +853,8 @@ sepgsql_compute_create(const char *scontext, * Ask SELinux what is the default context for the given object class on a * pair of security contexts */ - if (security_compute_create_name_raw((security_context_t) scontext, - (security_context_t) tcontext, + if (security_compute_create_name_raw(scontext, + tcontext, tclass_ex, objname, &ncontext) < 0) diff --git a/contrib/sepgsql/uavc.c b/contrib/sepgsql/uavc.c index 639a52c556..97189b7c46 100644 --- a/contrib/sepgsql/uavc.c +++ b/contrib/sepgsql/uavc.c @@ -171,7 +171,7 @@ sepgsql_avc_unlabeled(void) { if (!avc_unlabeled) { - security_context_t unlabeled; + char *unlabeled; if (security_get_initial_context_raw("unlabeled", &unlabeled) < 0) ereport(ERROR, @@ -216,7 +216,7 @@ sepgsql_avc_compute(const char *scontext, const char *tcontext, uint16 tclass) * policy is reloaded, validation status shall be kept, so we also cache * whether the supplied security context was valid, or not. */ - if (security_check_context_raw((security_context_t) tcontext) != 0) + if (security_check_context_raw(tcontext) != 0) ucontext = sepgsql_avc_unlabeled(); /* signature.asc Description: PGP signature
Re: security_context_t marked as deprecated in libselinux 3.1
On 2020-Aug-14, Michael Paquier wrote: > On Thu, Aug 13, 2020 at 08:47:28PM -0400, Tom Lane wrote: > > Should we back-patch that? Usually I figure that people might want > > to build back PG branches on newer platforms at some point, so that > > it's useful to apply portability fixes across-the-board. On the > > other hand, since it's only a compiler warning, maybe it's not worth > > the trouble. > > Not sure that's worth the trouble as long as people don't complain > about it directly, and it does not prevent the contrib module to > work. FWIW I just had a CI job fail the "warnings" test because of lacking this patch in the back branches :-) What do you think about back-patching this to at least 11? I would say 10, but since that one is going to end soon, it might not be worth much effort. OTOH maybe we want to backpatch all the way back to 9.2 given the no-warnings policy we recently acquired. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ "I must say, I am absolutely impressed with what pgsql's implementation of VALUES allows me to do. It's kind of ridiculous how much "work" goes away in my code. Too bad I can't do this at work (Oracle 8/9)." (Tom Allison) http://archives.postgresql.org/pgsql-general/2007-06/msg00016.php
Re: security_context_t marked as deprecated in libselinux 3.1
Alvaro Herrera writes: > On 2020-Aug-14, Michael Paquier wrote: >> On Thu, Aug 13, 2020 at 08:47:28PM -0400, Tom Lane wrote: >>> Should we back-patch that? Usually I figure that people might want >>> to build back PG branches on newer platforms at some point, so that >>> it's useful to apply portability fixes across-the-board. On the >>> other hand, since it's only a compiler warning, maybe it's not worth >>> the trouble. >> Not sure that's worth the trouble as long as people don't complain >> about it directly, and it does not prevent the contrib module to >> work. > FWIW I just had a CI job fail the "warnings" test because of lacking > this patch in the back branches :-) What do you think about > back-patching this to at least 11? No objection to back-patching from me. > I would say 10, but since that one > is going to end soon, it might not be worth much effort. OTOH maybe we > want to backpatch all the way back to 9.2 given the no-warnings policy > we recently acquired. I'm not sure that no-warnings policy extends to stuff as far off the beaten path as sepgsql. However, I won't stand in the way if you want to do that. One point though: if you want to touch v10, I'd suggest waiting till after next week's releases. Unlikely as it is that this'd break anything, I don't think we should risk it in the branch's last release. regards, tom lane
Re: security_context_t marked as deprecated in libselinux 3.1
On Thu, Nov 03, 2022 at 07:01:20PM -0400, Tom Lane wrote: > Alvaro Herrera writes: >> FWIW I just had a CI job fail the "warnings" test because of lacking >> this patch in the back branches :-) What do you think about >> back-patching this to at least 11? > > No objection to back-patching from me. Fine by me. >> I would say 10, but since that one >> is going to end soon, it might not be worth much effort. OTOH maybe we >> want to backpatch all the way back to 9.2 given the no-warnings policy >> we recently acquired. > > I'm not sure that no-warnings policy extends to stuff as far off the > beaten path as sepgsql. However, I won't stand in the way if you > want to do that. One point though: if you want to touch v10, I'd > suggest waiting till after next week's releases. Unlikely as it > is that this'd break anything, I don't think we should risk it > in the branch's last release. In line of ad96696, seems like that it would make sense to do the same here even if the bar is lower. sepgsql has not changed in years, so I suspect few conflicts. Alvaro, if you want to take care of that, that's fine by me. I could do it, but not before next week. Agreed to wait after the next minor release. -- Michael signature.asc Description: PGP signature
Re: security_context_t marked as deprecated in libselinux 3.1
On Fri, Nov 04, 2022 at 08:49:24AM +0900, Michael Paquier wrote: > In line of ad96696, seems like that it would make sense to do the same > here even if the bar is lower. sepgsql has not changed in years, so I > suspect few conflicts. Alvaro, if you want to take care of that, > that's fine by me. I could do it, but not before next week. I got to look at that, now that the minor releases have been tagged, and the change has no conflicts down to 9.3. 9.2 needed a slight tweak, though, but it seemed fine as well. (Tested the build on all branches.) So done all the way down, sticking with the new no-warning policy for all the buildable branches. -- Michael signature.asc Description: PGP signature
Re: security_context_t marked as deprecated in libselinux 3.1
On 2022-Nov-09, Michael Paquier wrote: > I got to look at that, now that the minor releases have been tagged, > and the change has no conflicts down to 9.3. 9.2 needed a slight > tweak, though, but it seemed fine as well. (Tested the build on all > branches.) So done all the way down, sticking with the new no-warning > policy for all the buildable branches. Thank you :-) -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
Re: security_context_t marked as deprecated in libselinux 3.1
Michael Paquier writes: > Per the following commit in upstream SELinux, security_context_t has > been marked as deprecated, generating complains with > -Wdeprecated-declarations: > https://github.com/SELinuxProject/selinux/commit/7a124ca2758136f49cc38efc26fb1a2d385ecfd9 Huh. Apparently it's been considered legacy for a good while, too. > This can be seen with Debian GID when building contrib/selinux/, as it > we have libselinux 3.1 there. Per the upstream repo, > security_context_t maps to char * in include/selinux/selinux.h, so we > can get rid easily of the warnings with the attached that replaces > the references to security_context_t. Ummm ... aren't you going to get some cast-away-const warnings now? Or are all of the called functions declared as taking "const char *" not just "char *"? regards, tom lane
Re: security_context_t marked as deprecated in libselinux 3.1
On Wed, Aug 12, 2020 at 10:50:21PM -0400, Tom Lane wrote: > Ummm ... aren't you going to get some cast-away-const warnings now? > Or are all of the called functions declared as taking "const char *" > not just "char *"? Let me see.. The function signatures we use have been visibly changed in 9eb9c932, which comes down to a point between 2.2.2 and 2.3, and there are two of them we care about, both use now "const char *": - security_check_context_raw() - security_compute_create_name_raw() We claim in the docs that the minimum version of libselinux supported is 2.1.10 (7a86fe1a from march 2012). Then, the only buildfarm animal I know of testing selinux is rhinoceros, that uses CentOS 7.1, and this visibly already bundles libselinux 2.5 that was released in 2016 (2b69984), per the RPM list here: http://mirror.centos.org/centos/7/ Joe, what's the version of libselinux used in rhinoceros? 2.5? Based on this information, what if we increased the minimum support to 2.3 then? That's a release from 2014, and maintaining such legacy code does not seem much worth the effort IMO. -- Michael signature.asc Description: PGP signature
Re: security_context_t marked as deprecated in libselinux 3.1
Michael Paquier writes: > On Wed, Aug 12, 2020 at 10:50:21PM -0400, Tom Lane wrote: >> Ummm ... aren't you going to get some cast-away-const warnings now? > Let me see.. The function signatures we use have been visibly changed > in 9eb9c932, which comes down to a point between 2.2.2 and 2.3, and > there are two of them we care about, both use now "const char *": > - security_check_context_raw() > - security_compute_create_name_raw() OK, it's all good then. > Based on this information, what if we increased the minimum support to > 2.3 then? That's a release from 2014, and maintaining such legacy > code does not seem much worth the effort IMO. Well, "you get a compiler warning" isn't a reason to consider the version unsupported. There are probably going to be a few other warnings you get when building on an ancient platform --- as long as it works, I think we're fine. So based on this, no objection, and I think no need to change our statement about what's supported. regards, tom lane
Re: security_context_t marked as deprecated in libselinux 3.1
On Thu, Aug 13, 2020 at 01:29:35AM -0400, Tom Lane wrote: > Well, "you get a compiler warning" isn't a reason to consider the version > unsupported. There are probably going to be a few other warnings you get > when building on an ancient platform --- as long as it works, I think > we're fine. So based on this, no objection, and I think no need to > change our statement about what's supported. Okay, thanks for confirming. Let's do so then, I'll just wait a bit to see if there are more comments from others. -- Michael signature.asc Description: PGP signature
Re: security_context_t marked as deprecated in libselinux 3.1
On 8/13/20 1:22 AM, Michael Paquier wrote: > On Wed, Aug 12, 2020 at 10:50:21PM -0400, Tom Lane wrote: >> Ummm ... aren't you going to get some cast-away-const warnings now? >> Or are all of the called functions declared as taking "const char *" >> not just "char *"? > > Let me see.. The function signatures we use have been visibly changed > in 9eb9c932, which comes down to a point between 2.2.2 and 2.3, and > there are two of them we care about, both use now "const char *": > - security_check_context_raw() > - security_compute_create_name_raw() > We claim in the docs that the minimum version of libselinux supported > is 2.1.10 (7a86fe1a from march 2012). > > Then, the only buildfarm animal I know of testing selinux is > rhinoceros, that uses CentOS 7.1, and this visibly already bundles > libselinux 2.5 that was released in 2016 (2b69984), per the RPM list > here: > http://mirror.centos.org/centos/7/ > Joe, what's the version of libselinux used in rhinoceros? 2.5? rpm -q libselinux libselinux-2.5-15.el7.x86_64 Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development signature.asc Description: OpenPGP digital signature
Re: security_context_t marked as deprecated in libselinux 3.1
On Thu, Aug 13, 2020 at 06:54:41AM -0400, Joe Conway wrote: > On 8/13/20 1:22 AM, Michael Paquier wrote: >> Joe, what's the version of libselinux used in rhinoceros? 2.5? > > rpm -q libselinux > libselinux-2.5-15.el7.x86_64 Thanks! -- Michael signature.asc Description: PGP signature
Re: security_context_t marked as deprecated in libselinux 3.1
On Thu, Aug 13, 2020 at 02:35:28PM +0900, Michael Paquier wrote: > Okay, thanks for confirming. Let's do so then, I'll just wait a bit > to see if there are more comments from others. Applied on HEAD then. Thanks for the discussion! -- Michael signature.asc Description: PGP signature
Re: security_context_t marked as deprecated in libselinux 3.1
Michael Paquier writes: > Applied on HEAD then. Thanks for the discussion! Should we back-patch that? Usually I figure that people might want to build back PG branches on newer platforms at some point, so that it's useful to apply portability fixes across-the-board. On the other hand, since it's only a compiler warning, maybe it's not worth the trouble. regards, tom lane
Re: security_context_t marked as deprecated in libselinux 3.1
On Thu, Aug 13, 2020 at 08:47:28PM -0400, Tom Lane wrote: > Should we back-patch that? Usually I figure that people might want > to build back PG branches on newer platforms at some point, so that > it's useful to apply portability fixes across-the-board. On the > other hand, since it's only a compiler warning, maybe it's not worth > the trouble. Not sure that's worth the trouble as long as people don't complain about it directly, and it does not prevent the contrib module to work. -- Michael signature.asc Description: PGP signature