temurin-build
temurin-build copied to clipboard
OpenJ9 linux always configure --with-openssl=fetched
OpenJ9 get_source.sh will fetch the latest OpenSSL version during the build. We should compile against that version rather than 1.0.2r on the system.
Related AdoptOpenJDK/openjdk-infrastructure#2084
Signed-off-by: Adam Brousseau [email protected]
Looking for reviews from @pshipton and @sxa
If we do this will it still run against systems with 1.0.2 as a default e.g. RHEL7?
Yes. The version used for compile shouldn't affect the version used at runtime.
Is it worth the risk of changing it?
Questionable. In theory it shouldn't matter. The OpenJ9 builds fetch and compile against the latest, and that is my preference as doing this will discover a problem if one is ever introduced, which should never happen.
Is there any reason to download and use openssl when all of our machines are configured with the playbooks which will ensure a suitable version is available on platforms where we are not bundling it? It introduces extra risk if the openssl source download fails.
Note the older versions contain known security vulnerabilities if that matters for the maintenance of the machines.
Doing the check then setting it to the /usr/local directory OR fetched runs the risk of behavior changing based on whether it is present. If, for whatever reason, the installation of openssl into /usr/local failed that failure will be hidden and it'll just use a differnet one, which is always risky for ensuring consistency
In theory the version used for compile doesn't matter.
I should note that we need to ensure the correct and latest version (1.1.1k atm) is bundled for Windows and Mac.
As Peter says, it should not matter which version of openssl we use at compile time.
Only one source file (currently) needs to include any openssl header files, the others are redundant. With a little work, I expect we can remove those includes as well in which case no version of openssl would be required at build time.
I think Win and Mac do the right thing https://github.com/AdoptOpenJDK/openjdk-build/blob/041c33c6a56ff3474133714c6d0ef6d7d9bce233/build-farm/platform-specific-configurations/mac.sh#L51
Running ./configure with arguments 'bash ./configure --verbose --with-vendor-name=AdoptOpenJDK --with-vendor-url=https://adoptopenjdk.net/ --with-vendor-bug-url=https://github.com/AdoptOpenJDK/openjdk-support/issues --with-vendor-vm-bug-url=https://github.com/eclipse/openj9/issues --with-vendor-version-string=AdoptOpenJDK --with-version-opt=202103241343 --without-version-pre --with-version-build=7 --with-boot-jdk=/Library/Java/JavaVirtualMachines/adoptopenjdk-10.jdk/Contents/Home --with-macosx-codesign-identity="Developer ID Application: London Jamocha Community CIC" --with-debug-level=release --with-native-debug-symbols=external --enable-ccache --with-freemarker-jar=/Users/jenkins/workspace/build-scripts/jobs/jdk11u/jdk11u-mac-x64-openj9/workspace/./build//freemarker-2.3.31/freemarker.jar --with-jvm-variants=server --with-cacerts-file=/Users/jenkins/workspace/build-scripts/jobs/jdk11u/jdk11u-mac-x64-openj9/sbin/../security/cacerts --disable-ccache --disable-warnings-as-errors --with-openssl=fetched --enable-openssl-bundling --with-sysroot=/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/ --enable-dtrace=auto --with-cmake --with-freetype=bundled'
https://github.com/AdoptOpenJDK/openjdk-build/blob/041c33c6a56ff3474133714c6d0ef6d7d9bce233/build-farm/platform-specific-configurations/windows.sh#L148
Running ./configure with arguments 'bash ./configure --verbose --with-vendor-name=AdoptOpenJDK --with-vendor-url=https://adoptopenjdk.net/ --with-vendor-bug-url=https://github.com/AdoptOpenJDK/openjdk-support/issues --with-vendor-vm-bug-url=https://github.com/eclipse/openj9/issues --with-vendor-version-string=AdoptOpenJDK --with-version-opt=202103251815 --without-version-pre --with-version-build=7 --with-boot-jdk=/cygdrive/c/openjdk/jdk-10 --with-debug-level=release --with-native-debug-symbols=external --with-jvm-variants=server --with-cacerts-file=/cygdrive/e/jenkins/tmp/sbin/../security/cacerts --disable-warnings-as-errors --with-freemarker-jar=/cygdrive/c/openjdk/freemarker.jar --with-openssl=/cygdrive/c/openjdk/OpenSSL-1.1.1j-x86_64-VS2017 --enable-openssl-bundling --enable-cuda --with-cuda=C:/PROGRA~2/NVIDIA~2/CUDA/v9.0 --with-toolchain-version=2017 '
Note the older versions contain known security vulnerabilities if that matters for the maintenance of the machines.
We don't change the default on the machine, so any system apps will always be using the one supplied with the OS, not the one we store in /usr/local/openssl-xxx
purely for the purposes of building OpenJ9 against.
I should note that we need to ensure the correct and latest version (1.1.1k atm) is bundled for Windows and Mac.
Yep I'm finalising that under https://github.com/AdoptOpenJDK/openjdk-build/pull/2553 which is separate from this PR
I didn't consider the JITServer use of openssl in my answers. I'll ask @mpirvu to comment on that.
Note there is another playbook for openssl 1.1.1b for jitserver. https://github.com/AdoptOpenJDK/openjdk-infrastructure/blob/5ffd313d178b3ca4bbbf67d98d1f34f3c7c2ab03/ansible/playbooks/AdoptOpenJDK_Unix_Playbook/roles/OpenSSL/tasks/main.yml
Same answer for JITServer, it should not matter which version of OpenSSL is present at compile time. With respect to different variants of 1.1.x, as long as the API for various routines remains the same, as it should, we should be fine.
Can we have the jitserver use the version that we pull in get_source.sh? That way we could also remove the second playbook.
Can we have the jitserver use the version that we pull in get_source.sh? That way we could also remove the second playbook.
yes, that should work
As Peter says, it should not matter which version of openssl we use at compile time.
Only one source file (currently) needs to include any openssl header files, the others are redundant. With a little work, I expect we can remove those includes as well in which case no version of openssl would be required at build time.
When I wrote the above, I forgot about jdk/crypto/jniprovider/NativeCrypto.c
in the extension repos, but, again, compiles against any version should be fine.
Any concerns about reproducing older builds with this change?
Any concerns about reproducing older builds with this change?
From my perspective, it shouldn't make any difference.
run tests
Failure on GH Action
Compiling 224 properties into resource bundles for jdk.localedata
engines/e_afalg.c:31:4: warning: #warning "AFALG ENGINE requires Kernel Headers >= 4.1.0" [-Wcpp]
# warning "AFALG ENGINE requires Kernel Headers >= 4.1.0"
^~~~~~~
engines/e_afalg.c:32:4: warning: #warning "Skipping Compilation of AFALG engine" [-Wcpp]
# warning "Skipping Compilation of AFALG engine"
^~~~~~~
Compiling 3180 files for java.base
test/rsa_test.c: In function 'test_rsa_oaep':
test/rsa_test.c:349:5: warning: 'key' may be used uninitialized in this function [-Wmaybe-uninitialized]
RSA_free(key);
^~~~~~~~~~~~~
ar: creating apps/libapps.a
ar: creating libssl.a
ar: creating test/libtestutil.a
/bin/sh: CCACHE_COMPRESS=1: command not found
Makefile:6863: recipe for target 'fuzz/asn1parse-test' failed
gmake[5]: *** [fuzz/asn1parse-test] Error 127
gmake[4]: *** [all] Error 2
Makefile:174: recipe for target 'all' failed
I still don't see a compelling reason for changing this. The comments above seem to suggest there's no technical reason to use a later version on platforms where it is not bundled and it only has the potential to introduce instability by downloading things from third party websites during the build process.
Questions:
- What is the disadvantage of keeping it as-is?
- @keithc-ca suggests it might be fairly easy to remove this requirement anyway - is that something that there is a desire for?
- For @mpirvu: Does JITServer still require 1.1.1 to build against?
- The playbook role that @AdamBrousseau referred to earlier is not run on any of the AdoptOpenJDK buils systems (and given some of the things in there relating to adjusting default environments I am disinclined to do so) yet we have jitserver enabled on some platforms already e.g. pLinux JDK11 has
-enable-jitserver
and--with-openssl=system
Does JITServer still require 1.1.1 to build against?
JITServer does not need SSL library at build time. At runtime, given the options to force message encryption, JITServer looks for a library named:
// Library names for OpenSSL 1.1.1, 1.1.0, 1.0.2 and symbolic links
static const char * const libNames[] =
{
"libssl.so.1.1", // 1.1.x library name
"libssl.so.1.0.0", // 1.0.x library name
"libssl.so.10", // 1.0.x library name on RHEL
"libssl.so" // general symlink library name
};
It should work with any 1.1.x library version.
Does JITServer still require 1.1.1 to build against?
JITServer does not need SSL library at build time.
Correction: it does need the header files. The question is whether the 1.0.2 header files will suffice.
has the potential to introduce instability by downloading things from third party websites during the build process
Note that we have been cloning the latest openssl code every build as long as this req has been around.
The question is whether the 1.0.2 header files will suffice.
Yes, 1.0.2 headers will suffice. When we implemented SSL support for JITServer we tested the following scenarios:
building against 1.1.1, loaded with 1.0.2g
building against 1.0.2g, loaded with 1.1.1
building against 1.1.1, loaded with 1.1.1
What is the disadvantage of keeping it as-is?
Is there is ever an API compatibility problem with a new version of openssl it won't be noticed in the Adopt builds. However it will be in the OpenJ9 builds since they build against the latest.
has the potential to introduce instability by downloading things from third party websites during the build process
Atm it comes from github, as does the source for the other repos. If it doesn't work, probably nothing works so you can't build anyway.
GH Action needs updating.
Compiling 224 properties into resource bundles for jdk.localedata
engines/e_afalg.c:31:4: warning: #warning "AFALG ENGINE requires Kernel Headers >= 4.1.0" [-Wcpp]
# warning "AFALG ENGINE requires Kernel Headers >= 4.1.0"
^~~~~~~
engines/e_afalg.c:32:4: warning: #warning "Skipping Compilation of AFALG engine" [-Wcpp]
# warning "Skipping Compilation of AFALG engine"
^~~~~~~
Compiling 3180 files for java.base
test/rsa_test.c: In function 'test_rsa_oaep':
test/rsa_test.c:349:5: warning: 'key' may be used uninitialized in this function [-Wmaybe-uninitialized]
RSA_free(key);
^~~~~~~~~~~~~
ar: creating apps/libapps.a
ar: creating libssl.a
ar: creating test/libtestutil.a
/bin/sh: CCACHE_COMPRESS=1: command not found
Makefile:6863: recipe for target 'fuzz/asn1parse-test' failed
Is there is ever an API compatibility problem with a new version of openssl it won't be noticed in the Adopt builds. However it will be in the OpenJ9 builds since they build against the latest.
We frequently test build infrastructure changes on multiple OSs to validate the playbooks including ones which have 1.1.1 as a default so it likely wouldn't be completely missed, although that will be subject to whichever update policy the distribution provides). Also since we are pulling latest 1.1.1 on Windows, changes are we'd notice if the incompatibility such as you describe were to show up.
Other than for developers pulling (potentially unecessarily?) the latest, I'm still unclear on what circumstances where it beneficial though? Since it would only affect people building it, I would argue that's less of a concern than potential incompatibility with older versions. If we were to build against the latest headers, would we be confident that it still work out of the box on distributions like RHEL7 which have OpenSSL 1.0.2? Surely there is a risk that if there is a breaking change in a later 1.1.1 then using the latest one would mask the fact it won't execute on supported platforms if OpenJ9 were to use of new functionality without additional work since we're relying on the system SSL libraries on most platforms and that would be a customer visible break. Unless you plan to prereq 1.1.1 then it seems a risky strategy not to build against the earlier supported version.
Atm it comes from github, as does the source for the other repos. If it doesn't work, probably nothing works so you can't build anyway.
Fair point - I had assumed it was using the tarball from the website :-)
GH Action needs updating.
@karianna Can you advise - is that something this PR author needs to be responsible for?
GH Action needs updating.
@karianna Can you advise - is that something this PR author needs to be responsible for?
They're the best placed in all likeliness, they can ask on Slack for extra help.
If we were to build against the latest headers, would we be confident that it still work out of the box on distributions like RHEL7 which have OpenSSL 1.0.2?
Yes, it's designed to work this way.
Surely there is a risk that if there is a breaking change in a later 1.1.1 then using the latest one would mask the fact it won't execute on supported platforms if OpenJ9 were to use of new functionality without additional work since we're relying on the system SSL libraries on most platforms and that would be a customer visible break.
I don't believe so. Perhaps if there was new work done and it wasn't properly implemented or tested then Adopt building against 1.0.2 may catch that fact, but perhaps not. While building against 1.0.2 is supported today for no particular reason, it's not a benefit or something we need to keep working. If there is related new work done and any reason to do so, I can see support for building against 1.0.2 being dropped. It's more important that 1.0.2 is supported at runtime, which is confirmed by runtime testing, not by what happens at build time.
Unless you plan to prereq 1.1.1 then it seems a risky strategy not to build against the earlier supported version.
1.1.1 is actually a prereq for some features, and 1.1.1 can be used at runtime even if 1.0.2 is used to compile (and vice versa).
In summary, basically it doesn't matter what you compile against. You can take this change, or not. The benefits are very nebulous.
Is this not redundant now that #2917 is merged?
I wanted to take another look at this because we (Semeru) still use "system" on p/z linux.