I forgot another application of C plugins: they can be used to "virtualize" an application from the file-system, i.e. redirect open, unlink, rename, etc. to other files, although it is not its main purpose.

I understand the CAML_GETENV part and it looks fine to me. The other part is complex, I have not reviewed it. Maybe it would make sense to split the PR in two? I wouldn't mind merging the GETENV part in 4.04+dev.

I am still working on the CAML_CPLUGINS part anyway, it seems not to work correctly because asmrun files are not compiled in -fPIC by default...

Ok, the new version works both with bytecode and native code.

@gasche As it is a runtime thing, I am planning to ask @damiendoligez for both, since he is the author of most the recommendations of LafoSec :-)

I think it would still be nicer to split the two separate features in two separate commits.

these seem like quite distinct features -- the getenv one is quite useful for Mirage/Xen since we can supply an alternative macro that supplies the runtime configuration information from XenStore (right now we have to define a custom getenv stub).

(right now we have to define a custom getenv stub).

Wouldn't you do this anyway (for other getenvs that may lurk in e.g. C library code) ?

Wouldn't you do this anyway (for other getenvs that may lurk in e.g. C library code)

Preference is to remove getenv and fix up the C libraries where possible. This may be impractical, but slightly less so now that the OCaml runtime can be configured to not trigger a zillion dynamic getenv calls.

@lefessan If you're going to split the plugins part off, could I suggest you also make a third pull request which does something like adding a "configure" option for PIC, which does the right thing (i.e. C compiler flags etc + the asmrun/Makefile change that is required)? (We also need this for the gdb plugin.)

As a proof-of-concept, here is an experimental set of plugins:

https://github.com/OCamlPro/ocp-cplugins

For example, the ocp-show-build -o build.log -- make followed by grep ocamlopt.opt build.log was particularly useful to understand how to translate the multiple calls to ocamlbuild in dose into ocp-build format...

@mshinwell I am not an expert of configure scripts (and even less if it is confirmed that we are moving to autoconf), so I will leave the task of adding -fPIC correctly to a better expert. The current behavior is enough for the current purpose (but would clearly benefit from -fPIC for more uses).

I would prefer if --getenv were instead a compile-time option. This would require that it be stored in the bytecode files generated by ocamlc and in the startup of programs generated by ocamlopt.

@drbo what would be the semantics of linking together some modules compiled with --no-getenv and some with getenv enabled? I'm not sure it is easy to implement a per-module semantics (do compiled C stubs have some form of private per-module register?).

@drbo, @lefessan The no-getenv option could also be a runtime variant, so you could choose it at link time.

From what I understood at the core-dev meeting, there was not a strong interest in this patch, but neither any opposition. Since it is interesting for us, I will rebase it, add access to a few more variables/functions (especially hooks), and merge it in the next weeks.

I am not fond of runtime variants. OPAM provides the possibility to have multiple switches, I find it better to provide a compiler with --no-getenv enabled than to increase the number of variants.

With N different configure-time flags (no-getenv, fp, afl instrumentation, ...), will we have 2^N OPAM switches? Can switches be created "lazily" on the user machine (based on some configure-time options)?

I wouldn't actually create an ocaml-no-getenv switch, but an ocaml-secure switch, with all the configure arguments for maximum security enabled.

Can switches be created "lazily" on the user machine (based on some configure-time options)?

Not sure I understand your question, but a possible answer would be to modify the configure script to read an OCAML_CONFIGURE env variable, that you could be used to create variants from the standard switch with specific arguments. I will leave that as an exercise for the reader (I have already added too many env variables to OCaml for one lifetime).

I was thinking about supporting that in OPAM. You could do opam switch 4.03/blabla --configure "--no-get-env --fp". This would create an internal switch based on upstream 4.03, but with extra configure options.

It's probably worth submitting an issue on opam. Maybe it is already possible to do it. Instead of opam switch 4.03/blabla --configure "--no-get-env --fp", I would probably set a variable, that would then be available to all the packages in the switch (especially as compilers are now standard packages in 2.0): opam switch 4.03/blabla --set "configure_args=--no-get-env --fp"

Oh, but then you would need also a way to "unsplit" the variables in the command line:

build: [
   "./configure" unsplit(configure_args}
   make "world.opt"
]

@AltGr

I don't understand what's wrong with using runtime variants as @damiendoligez suggests -- the "no getenv" feature is purely constrained to affecting libasmrun.a and doesn't touch the OCaml compilation units. We already have all the compiler CLI options required to support it present, so it doesn't add any complexity to the build systems.

This would save the (much more heavyweight) option of a new compiler flag for features that absolutely require them since they affect OCaml compilation units (like AFL fuzzing, or flambda).

I don't understand what's wrong with using runtime variants as @damiendoligez suggests

Because, for security reasons, you want all your toolchain to be unaffected by environment variables. Suppose you have a completely scripted build system, that cannot be modified, but you can change env variables before starting it. Then, an engineer can set an env. variable to connect to the OCaml compiler during the build, and inject some code in the generated object, that will be linked in the executable at the end. That's the kind of security holes that ANSSI (French equivalent of NSA) is trying to prevent.

I don't see how you can prevent that with runtime variants, unless you actually set OCAMLPARAM=runtime-variant=n which defeats the whole purpose of the patch...

PS: a good complement would be to disable OCAMLPARAM, OCAMLLIB and other env variables on the OCaml side when --no-getenv is specified.

Because, for security reasons, you want all your toolchain to be unaffected by environment variables

A configure option to disable the installation of the standard runtime library (and debug version) would work here, wouldn't it? Presumably any such locked down OCaml distribution would have its own distribution system, and this could choose which runtime variants to install as a matter of local security policy.

What if the attack happens during the compilation of OCaml itself ? Code could be injected inside stdlib for example, while the standard runtime variant is still being used. If we add an option to configure to change that, I don't see the benefit over the --no-getenv option. Or maybe, we should add --secure option, that will trigger the no-getenv behavior, but also other features for security.

What if the attack happens during the compilation of OCaml itself ?

I'm afraid I don't understand your threat model. Presumably you would build your toolchain in a deterministic environment (e.g. a locked down Docker container with tight seccomp allowances, and a trusted kernel), and provide that as a binary toolchain for untrusted code to be compiled.

Or maybe, we should add --secure option, that will trigger the no-getenv behavior, but also other features for security.

One person's --secure is another person's --broken (if they actually want to be able to dynamically change parameters, as most developers will want to).

as a binary toolchain for untrusted code to be compiled.

The threat model is not to compile untrusted code, but untrusted engineers who would want to inject their untrusted code in a trusted code. In such a setting, everything should be static... but most developers won't want to work in such an environment !

Or maybe, we should add --secure option, that will trigger the no-getenv behavior, but also other features for security.

+1 to that.-not-getenv sounds like it's only a tiny part of the solution.

As for merging soon, lack of interest is not enough, you need at least a code review. You also need approval from another core developer, unless the patch is really small (which it isn't in its current state).

I have one question about the patch: why don't you #define the getenv identifier directly instead of changing the calls in the source ? That would make it more robust to future additions.
Same question for the other syscalls.

Finally, I really think this should be two separate PRs, as the tracing stuff has nothing to do with security.

+1 to that.-not-getenv sounds like it's only a tiny part of the solution.
Finally, I really think this should be two separate PRs, as the tracing stuff has nothing to do with security.

The rationale was that the addition of C plugins was creating a security hole (well, not bigger than the current debugger support), and that the --no-getenv argument would provide a way to disable it. Since everybody here seems to think it's not the case, I will drop the --no-getenv part of the patch and wait for a bigger fix for security.

As for merging soon, lack of interest is not enough, you need at least a code review. You also need approval from another core developer, unless the patch is really small (which it isn't in its current state).

I will see if Pierre can do it.

I have one question about the patch: why don't you #define the getenv identifier directly instead of changing the calls in the source ? That would make it more robust to future additions.
Same question for the other syscalls.

I think it's really a bad practice to #define the names of existing functions: it makes code hard to debug because other developers don't understand why a call, to a C function they know, does not behave as usual. Moreover, you also want to control when the macro should be used, and when it shouldn't.

I think it's really a bad practice to #define the names of existing functions: it makes code hard to debug because other developers don't understand why a call, to a C function they know, does not behave as usual. Moreover, you also want to control when the macro should be used, and when it shouldn't.

Good arguments, but for security you want to make sure the macros are always used, right?

Here, the idea was to prevent getenv calls done by the runtime, without the OCaml part asking for them, i.e. Sys.getenv would still work, even if --no-getenv would be set.

Replaced by #668