FIPS module in build with -z defs

[kroeckx]

It seems that when you configure with no-shared, it's still creating a fips.so file.

[kroeckx]

There is also a p_test.so and legacy.so

[mattcaswell]

Well, its a design decision that the fips module will only ever be in a .so form. And I'm also not sure it follows that just because you want the static libraries for libcrypto/libssl that you don't want to use fips. You can of course independently turn off building of the fips module with "no-fips". The legacy library is another issue - I suppose you may want the option for that to be static and just part of libcrypto - although that's not possible right at the moment.

[kroeckx]

So the issue is not that we generate the shared library itself. It seems that when making the shared library, but that we pass -z defs, and that that works with gcc but not with clang. We don't pass it for the other shared libraries.

[kroeckx]

So it seems that gcc properly links in the asan library, clang doesn't.

[kroeckx]

It seems we can fix this with adding -lasan

[paulidale]

I think it would be sensible to allow any provider (except FIPS) to be statically linked into libcrypto.

[levitte]

Alternative fix in #8749

[kroeckx]

This issue is not fixed. OSS-Fuzz doesn't use the enable-asan option and things like that, it just adds it to the CFLAGS.

[levitte]

Can OSS-Fuzz be changed to use the enable-asan option?

[kroeckx]

No. But they use the enable-fuzz-libfuzzer option.

[levitte]

So let me see if I get this right, they configure in a way that makes it harder for us to detect that they enable asan, when there's an option for them to use that makes everyone happy (I assume), and now you're hinting that we should kinda piggy-back enable-asan on a different, unrelated option?

This is screwed up...

[levitte]

I went looking, and yeah, I get how that's hard to fix. Meh...

[levitte]

#8778 is my attempt to fix this

[kroeckx]

Please note that this is a general framework to do fuzzing. They set the correct cflags so that the compiler instruments the code for fuzzing. They also have different builds, with asan, with ubsan, and so just set additional cflags. We're not being told that it's msan they want to do, just that they want to fuzz and some cflags. It currently already has code that does: if [[ $CFLAGS = *sanitize=memory* ]] then CONFIGURE_FLAGS="no-asm" fi You can turn that into enable-msan if you really want to, but I don't see the point. If you want we can add some additional config arument that disables the -z defs. Note that there are also other projects fuzz projects that want to link against openssl, they have the same problem that they can't link openssl, and they do not use something like enable-fuzz-libfuzzer. The only way to detect it there is that it has some -fsanitize=foo as part of the config call. So I think there are only 2 solutions: - Detect the sanitizer based on the cflags, and use that to disable -z defs - Add a new config option to disable -z defs

[levitte]

You can turn that into enable-msan if you really want to, but I don't see the point. If you want we can add some additional config arument that disables the -z defs.

That's exactly the purpose with my translating -fsanitize=memory to enable-msan. Note this:

openssl/Configurations/shared-info.pl

Lines 35 to 38 in ad7e17d

	dso_ldflags =>
	$disabled{asan} && $disabled{msan} && $disabled{ubsan}
	? '-z defs'
	: '',

In other words, the addition of -z defs is only made if msan hasn't been enabled.

[kroeckx]

This issue is still not solved.

[levitte]

Please show me an actual configuration so I have a chance to try and reproduce.

[kroeckx]

Here is a link to the latest log: https://oss-fuzz-build-logs.storage.googleapis.com/log-989a654e-97eb-4596-8232-8be66963441f.txt

You can find the recent logs on https://oss-fuzz-build-logs.storage.googleapis.com/index.html

[levitte]

Ah, yeah I see what happens, the processing order in Configure isn't quite right, the shared-info.pl data is processed too early, before the state of asan, msan and ubsan has been determined.

I'll fix when I'm more awake

[levitte]

I hope that #8846 fixed things well enough... I forgot to place a "Fixes #8735" in that one, unfortunately.

[kroeckx]

Just in case it wasn't clear, this is still a problem.

[levitte]

Okie, I see step 21 in today's, and wow that's a load of sanitizer checks I can't even find the docs for!

It seems like the sanest is really to not use -z defs if -fsanitize is present, period.

[levitte]

I hope #8892 finally fixes this mess

[kroeckx]

It doesn't.

[kroeckx]

Never mind, that's still a log that didn't have that commit.