Thread: 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
Attachment
Michael Paquier <michael@paquier.xyz> 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
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
Attachment
Michael Paquier <michael@paquier.xyz> 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
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
Attachment
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
Attachment
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
Attachment
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
Attachment
Michael Paquier <michael@paquier.xyz> 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
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
Attachment
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
Alvaro Herrera <alvherre@alvh.no-ip.org> 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
On Thu, Nov 03, 2022 at 07:01:20PM -0400, Tom Lane wrote: > Alvaro Herrera <alvherre@alvh.no-ip.org> 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
Attachment
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
Attachment
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/