Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

`secure_getenv` names #1331

Closed
wants to merge 1 commit into from
Closed

`secure_getenv` names #1331

wants to merge 1 commit into from

Conversation

@xclerc
Copy link
Contributor

xclerc commented Sep 13, 2017

In glibc 2.17, __secure_getenv was renamed to secure_getenv.

While the configure script correctly determines which name
should be used on a particular system, we ran into a problem
when migrating from CentOS 6 to CentOS 7. The problem
occurs when the compiler is built on a CentOS 6 system, and
then used on a CentOS 7: the linker fails as it cannot resolve
the symbol for the function.

@hhugo suggested to add a switch to the configure script
in order to request the use of the fallback implementation for
secure_getenv (based on either issetugid or the combination
of geteuid, getuid, getegid, and getgid).

The switch is currently named -use-secure-getenv-fallback, but
comments are welcome; maybe something along the lines of
emulate-secure-getenv would be better?

…ntation for secure_getenv.
@shindere

This comment has been minimized.

Copy link
Contributor

shindere commented Sep 13, 2017

@bobot

This comment has been minimized.

Copy link
Contributor

bobot commented Sep 13, 2017

@xclerc it seems that was available during the switch. Do you know when they stopped proposing the previous symbol?

Edit: fix the link

@xclerc

This comment has been minimized.

Copy link
Contributor Author

xclerc commented Sep 13, 2017

@shindere do you mean when the compiler is run or when the compiled program is run?
(anyway, it seems at odds with the usual way to deal with this kind of problems)

@bobot I am afraid I do not see the link with the referenced page...

@xavierleroy

This comment has been minimized.

Copy link
Contributor

xavierleroy commented Sep 13, 2017

OCaml doesn't support the scenario "I compile on Linux distro X and run on Linux distro Y". And I doubt there are many Linux distributions that actually give this kind of guarantees, even RedHat/CentOS. So, no, we're not going to add configure flags to work around this specific instance of this problem. Your best bet is to complete your migration to CentOS 7 everywhere. Your second best bet is to patch config/s.h after it's been generated by configure.

@mshinwell

This comment has been minimized.

Copy link
Contributor

mshinwell commented Sep 14, 2017

I'd like to discuss this one a bit further, if I may.

As far as I understand it, glibc is designed so that executables built on older minor versions will run on new minor versions unmodified, by using symbol versioning. (Such a minor-to-minor transition exists in the case of CentOS 6 to CentOS 7, for example.) I think therefore the expectation of users is that, if they build the OCaml compilers and runtime on an older minor version of glibc than that on which they are running, they will run. (For example, ocamlopt.opt depends only on glibc, in terms of dynamic libraries.)

As such I don't think the question is really about whether OCaml explicitly supports this kind of transition; it's about ensuring that we don't break reasonable expectations. Unfortunately we are breaking them at the moment: the __secure_getenv symbol prior to glibc 2.17 appears to have been an internal function. I think that, if we are going to have the compilers or runtime link against such internal symbols, then there needs to be an option to disable such uses (and to use a fallback implementation). If the configure argument for the secure-getenv functionality is too specific, perhaps a more generic one could be implemented, asking the build to avoid any non-public APIs.

@mshinwell mshinwell reopened this Sep 14, 2017
@bobot

This comment has been minimized.

Copy link
Contributor

bobot commented Sep 14, 2017

Unfortunately we are breaking them at the moment: the __secure_getenv symbol prior to glibc 2.17 appears to have been an internal function.

However it seems that the libc still seems to export the symbol for backward compatibility (cf. the (fixed) link I posted) and for example on my debian (objdump -T /lib/x86_64-linux-gnu/libc.so.6):

0000000000039ef0  w   DF .text	000000000000001b  GLIBC_PRIVATE __libc_secure_getenv
0000000000039ef0  w   DF .text	000000000000001b (GLIBC_2.2.5) __secure_getenv
0000000000039ef0  w   DF .text	000000000000001b  GLIBC_2.17  secure_getenv

Is the problem particular to CentOS?

@xclerc

This comment has been minimized.

Copy link
Contributor Author

xclerc commented Sep 14, 2017

It is exported, but (as stated in your reference) as a weak reference.
Unless I am mistaken, this means that it can be used at run time but
not at compile time (hence ABI compatibility but not API compatibility).

@xavierleroy

This comment has been minimized.

Copy link
Contributor

xavierleroy commented Sep 14, 2017

@bobot: watch out for symbol versioning... The GLIBC_2.2.5 tag causes __secure_getenv to be there but not accessible to us.

@xavierleroy

This comment has been minimized.

Copy link
Contributor

xavierleroy commented Sep 14, 2017

@mshinwell : glibc has a binary backward compatibility policy based on symbol versioning. That doesn't mean OCaml should bend over backward to provide the same kind of compatibility. Feels like a terrible burden for a dubious benefit.

@mshinwell

This comment has been minimized.

Copy link
Contributor

mshinwell commented Sep 14, 2017

I suppose another solution might be to not use __secure_getenv at all (and always use the fallback implementation, which I think does the same thing).

@xavierleroy

This comment has been minimized.

Copy link
Contributor

xavierleroy commented Sep 14, 2017

The use of __secure_getenv was introduced to better support... old versions of Redhat/CentOS that don't have secure_getenv ! Apparently it is more secure than the fallback implementation, because it takes into account the history of the process (did it start as setuid and dropped permissions later?) and some Linux-y capability stuff. So, no, we are not going to weaken security just to ease one OS upgrade at one user, even if the user is Jane Street.

@mshinwell

This comment has been minimized.

Copy link
Contributor

mshinwell commented Sep 14, 2017

Ah, I thought that version was before the fancy stuff was added, but looking at the source code maybe not...

@damiendoligez

This comment has been minimized.

Copy link
Member

damiendoligez commented Sep 21, 2017

I'm going to side with Xavier on this one, and I'll lay the blame on glibc for removing a symbol while pretending to provide backward compatibility for binaries.

We might be interested in another PR for a flag that says "don't use non-public APIs", but note that __secure_getenv is documented in the LSB specification (version 1.1.0) so I find it hard to tell that it's not a public API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.