Horton

Thread

Commits

  1. Fix compilation warnings with libselinux 3.1 in contrib/sepgsql/

  1. security_context_t marked as deprecated in libselinux 3.1

    Michael Paquier <michael@paquier.xyz> — 2020-08-13T01:27:35Z

    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

  2. Re: security_context_t marked as deprecated in libselinux 3.1

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-08-13T02:50:21Z

    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

  3. Re: security_context_t marked as deprecated in libselinux 3.1

    Michael Paquier <michael@paquier.xyz> — 2020-08-13T05:22:41Z

    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

  4. Re: security_context_t marked as deprecated in libselinux 3.1

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-08-13T05:29:35Z

    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

  5. Re: security_context_t marked as deprecated in libselinux 3.1

    Michael Paquier <michael@paquier.xyz> — 2020-08-13T05:35:28Z

    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

  6. Re: security_context_t marked as deprecated in libselinux 3.1

    Joe Conway <mail@joeconway.com> — 2020-08-13T10:54:41Z

    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

  7. Re: security_context_t marked as deprecated in libselinux 3.1

    Michael Paquier <michael@paquier.xyz> — 2020-08-14T00:10:30Z

    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

  8. Re: security_context_t marked as deprecated in libselinux 3.1

    Michael Paquier <michael@paquier.xyz> — 2020-08-14T00:43:20Z

    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

  9. Re: security_context_t marked as deprecated in libselinux 3.1

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-08-14T00:47:28Z

    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

  10. Re: security_context_t marked as deprecated in libselinux 3.1

    Michael Paquier <michael@paquier.xyz> — 2020-08-14T01:05:26Z

    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

  11. Re: security_context_t marked as deprecated in libselinux 3.1

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-11-03T18:10:28Z

    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

  12. Re: security_context_t marked as deprecated in libselinux 3.1

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-11-03T23:01:20Z

    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

  13. Re: security_context_t marked as deprecated in libselinux 3.1

    Michael Paquier <michael@paquier.xyz> — 2022-11-03T23:49:24Z

    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

  14. Re: security_context_t marked as deprecated in libselinux 3.1

    Michael Paquier <michael@paquier.xyz> — 2022-11-09T00:53:51Z

    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

  15. Re: security_context_t marked as deprecated in libselinux 3.1

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-11-09T08:20:50Z

    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/