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

Update to openssl 3.0.10 #14900

Merged
merged 2 commits into from
Aug 24, 2023
Merged

Update to openssl 3.0.10 #14900

merged 2 commits into from
Aug 24, 2023

Conversation

pshipton
Copy link
Member

@pshipton pshipton commented Apr 11, 2022

Update the notices file with the new license for v3. Updated the build instructions.

https://www.openssl.org/source/apache-license-2.0.txt

@pshipton
Copy link
Member Author

jenkins compile amac jdk11 depends ibmruntimes/openj9-openjdk-jdk11#500

@pshipton
Copy link
Member Author

jenkins compile zlinux,win,xmac,aix jdk11 depends ibmruntimes/openj9-openjdk-jdk11#500

@pshipton
Copy link
Member Author

pshipton commented Apr 12, 2022

It doesn't build on Windows

21:08:51  	"C:/cygwin64/bin/perl.exe" util/mkbuildinf.pl ""cl" /Zi /Fdossl_static.pdb /Gs0 /GF /Gy /MD /W3 /wd4090 /nologo /O2 -D"L_ENDIAN" -D"OPENSSL_PIC"" "VC-WIN64A" > crypto/buildinf.h
21:08:51  Can't open perl script "util": Is a directory

@pshipton
Copy link
Member Author

pshipton commented Apr 12, 2022

My rebuild for amac is https://openj9-jenkins.osuosl.org/job/Build_JDK11_aarch64_mac_Personal/57
Strangely it got the following.

18:58:12  WARNING: /Users/jenkins/workspace/Build_JDK11_aarch64_mac_Personal/build/macosx-aarch64-normal-server-release/jdk/bin/jmod is loading libcrypto in an unsafe way
18:58:12  JVMDUMP039I Processing dump event "abort", detail "" at 2022/04/12 08:58:12 - please wait.

The build doesn't yet support loading libcrypto.3.dylib (as ibmruntimes/openj9-openjdk-jdk11#499 is not yet merged) so it should be trying to load the old library libcrypto.1.1.dylib, which doesn't exist in the build. Perhaps it found some other library on the machine which is causing this error. I don't think this matters for the changes themselves. openssl3 was built, and copied into the JVM build. If it wasn't copied in, there would have been an error as seen in https://openj9-jenkins.osuosl.org/job/Build_JDK11_aarch64_mac_Personal/54

13:36:18  /Users/jenkins/workspace/Build_JDK11_aarch64_mac_Personal/closed/custom/copy/Copy-java.base.gmk:206: *** Cannot bundle OpenSSL - none of libcrypto.1.1.dylib libcrypto.1.0.0.dylib are present in /Users/jenkins/workspace/Build_JDK11_aarch64_mac_Personal/openssl.  Stop.

@pshipton
Copy link
Member Author

@keithc-ca any ideas for fixing #14900 (comment) ?

@pshipton pshipton marked this pull request as draft April 12, 2022 13:14
@keithc-ca
Copy link
Contributor

@keithc-ca any ideas for fixing #14900 (comment) ?

I'll have a look.

@pshipton
Copy link
Member Author

Windows is the only platform that uses a different compiler on jdk8 vs jdk11.

jenkins compile win,win32 jdk8 depends ibmruntimes/openj9-openjdk-jdk8#559

@pshipton
Copy link
Member Author

Windows and AIX are the only platforms that use a different compiler on jdk17 vs jdk11.

jenkins compile win,aix jdk17 depends ibmruntimes/openj9-openjdk-jdk17#88

@pshipton
Copy link
Member Author

All the Windows versions have the same basic problem #14900 (comment). If we can fix that, we'll be able to see if anything else is uncovered when using the different compiler versions.

@keithc-ca
Copy link
Contributor

Perhaps some different 'special setup' is required for openssl 3 on Windows; see https://github.com/ibmruntimes/openj9-openjdk-jdk11/blob/openj9/closed/openssl.gmk#L30.

@pshipton
Copy link
Member Author

Maybe we need a different version of perl.

@pshipton
Copy link
Member Author

perl seems fine, it's the cygwin perl. It doesn't have a problem with forward slashes, even when I call it via nmake.

This makefile works

all:
	perl util/test.pl

and so does

all:
	C:\cygwin64\bin\perl.exe util/test.pl

@pshipton
Copy link
Member Author

pshipton commented Apr 12, 2022

The following makefile doesn't appear to do anything, I suspect it recognized C: and then stops.

all:
	C:/cygwin64/bin/perl.exe util/test.pl

@pshipton
Copy link
Member Author

Running c:/cygwin64/bin/perl.exe util/test.pl from cygwin bash works fine.

@keithc-ca
Copy link
Contributor

In my local build, the command echoed by nmake is:

"C:/cygwin/bin/perl.exe" util/mkbuildinf.pl ""cl" /Zi /Fdossl_static.pdb /Gs0 /GF /Gy /MD /W3 /wd4090 /nologo /O2 -D"L_ENDIAN" -D"OPENSSL_PIC"" "VC-WIN64A" > crypto/buildinf.h

Notice numerous nested or unbalanced double-quotes. One avenue I think worth exploring is telling nmake to use a shell other than cmd.

@pshipton
Copy link
Member Author

pshipton commented Apr 12, 2022

That looks very similar to the command in #14900 (comment) except for the cygwin / cygwin64 directory.

Did you also get Can't open perl script "util": Is a directory?

If I have a util/mkbuildinf.pl file, and run that command from bash or cmd, it "works" in that it runs my util/mkbuildinf.pl.

@keithc-ca
Copy link
Contributor

Yes.

Can't open perl script "util": Is a directory

@keithc-ca
Copy link
Contributor

For the record, other uses of perl fail similarly when invoked by nmake:

C:\space\openssl>nmake libcrypto.rc

Microsoft (R) Program Maintenance Utility Version 14.16.27042.0
Copyright (C) Microsoft Corporation.  All rights reserved.

        "C:/cygwin/bin/perl.exe" util/mkrc.pl libcrypto > libcrypto.rc
Can't open perl script "util": Is a directory
NMAKE : fatal error U1077: '"C:/cygwin/bin/perl.exe"' : return code '0xff'
Stop.

Removing the double quotes fixes it.

https://github.com/openssl/openssl/blob/master/NOTES-PERL.md#perl-on-windows recommends Strawberry Perl; I'll give that a spin to see if it helps.

@keithc-ca
Copy link
Contributor

No dice:

C:\space\openssl>nmake libcrypto.rc

Microsoft (R) Program Maintenance Utility Version 14.16.27042.0
Copyright (C) Microsoft Corporation.  All rights reserved.

        "C:\Strawberry\perl\bin\perl.exe" util/mkrc.pl libcrypto > libcrypto.rc
Can't open perl script "util": Permission denied
NMAKE : fatal error U1077: '"C:\Strawberry\perl\bin\perl.exe"' : return code '0xd'
Stop.

@keithc-ca
Copy link
Contributor

It's behaving as if nmake is splitting util/mkrc.pl so perl sees
I'm going to see if using the configuration Cgwin-x86_64 fairs better.

@keithc-ca
Copy link
Contributor

Using the Cgwin-x86_64 configuration succeded with jdk11 and openssl 3.0.2.
We'll need to verify that this strategy also works for openssl 1.x.

@keithc-ca
Copy link
Contributor

As the description says, this depends on ibmruntimes/openj9-openjdk-jdk11#500, but we'll also need similar changes for other versions, and we'll need all versions to support using openssl 3.x on all platforms (not just Linux) before it would be safe to merge this.

@pshipton
Copy link
Member Author

Also, openssl 3 doesn't build on some of the linux platforms, there is an (internal) issue open to update all the machines with the required perl modules. Then we can see if there is another problem.

@keithc-ca
Copy link
Contributor

My point is that we can't change --openssl-version globally for all platforms until the other platforms are ready.

@keithc-ca
Copy link
Contributor

A new tag for openssl-3.0.3 appeared today.

@pshipton pshipton changed the title Update to openssl 3.0.2 Update to openssl 3.0.3 May 3, 2022
@paulcheeseman
Copy link

@pshipton

We need to ensure that the fix for CVE-2023-2975 is included:

https://www.openssl.org/news/secadv/20230714.txt

OpenSSL have not yet shipped a release containing this fix due to the low severity, but the patch is available:

https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=00e2f5eea29994d19293ec4e8c8775ba73678598

@keithc-ca
Copy link
Contributor

OpenSSL versions 1.1.1 and 1.0.2 are not affected by this issue.

We don't yet bundle version 3.* so there's nothing OpenJ9 can do to help avoid CVE-2023-2975. OpenJ9 will use 3.* when available, but it will be end users that will have to upgrade an installation of openssl 3.* to avoid that issue.

@pshipton
Copy link
Member Author

pshipton commented Jul 20, 2023

When we start bundling 3.x for the Oct release we will include the latest CVEs.

@paulcheeseman
Copy link

paulcheeseman commented Jul 21, 2023

We don't yet bundle version 3.* so there's nothing OpenJ9 can do to help avoid CVE-2023-2975

Yes, apologies if I wasn't clear. I meant that when we start bundling 3.x we need to make sure we include this fix. I only mentioned it here because it (currently) needs to be applied manually - it hasn't yet been included in an official OpenSSL release.

@pshipton
Copy link
Member Author

For reference, this is what we did for 1.1.1 to include the latest CVE fix.
#17828

@pshipton
Copy link
Member Author

jenkins compile all jdk8,jdk11,jdk17

Signed-off-by: Peter Shipton <[email protected]>
@pshipton
Copy link
Member Author

jenkins compile all jdk8,jdk11,jdk17

@pshipton
Copy link
Member Author

jenkins compile all jdk20

@keithc-ca
Copy link
Contributor

Maybe you meant to build jdk21 (jdk20 is no longer supported on the main branches)?

@pshipton
Copy link
Member Author

That is true.

jenkins compile all jdk21

@pshipton
Copy link
Member Author

pshipton commented Aug 21, 2023

PR builds, which passed compilation on all platforms and versions.

https://openj9-jenkins.osuosl.org/job/PullRequest-OpenJ9/4267/
https://openj9-jenkins.osuosl.org/job/PullRequest-OpenJ9/4269/

@pshipton pshipton changed the title Update to openssl 3.0.x Update to openssl 3.0.10 Aug 21, 2023
@pshipton pshipton marked this pull request as ready for review August 22, 2023 13:47
@pshipton pshipton requested a review from keithc-ca August 22, 2023 13:47
pshipton added a commit to pshipton/openjdk-build that referenced this pull request Aug 22, 2023
pshipton added a commit to pshipton/openjdk-build that referenced this pull request Aug 23, 2023
doc/build-instructions/Build_Instructions_V11.md Outdated Show resolved Hide resolved
@@ -131,7 +131,7 @@ Now fetch additional sources from the Eclipse OpenJ9 project and its clone of Ec
bash get_source.sh
```

:pencil: **OpenSSL support:** If you want to build an OpenJDK with OpenJ9 binary with OpenSSL support and you do not have a built version of OpenSSL V1.0.2 or v1.1.1 available locally, you must specify `--openssl-version=<version>` where `<version>` is an OpenSSL level like 1.0.2 or 1.1.1. If the specified version of OpenSSL is already available in the standard location (`SRC_DIR/openssl`), `get_source.sh` uses it. Otherwise, the script deletes the content and downloads the specified version of OpenSSL source to the standard location and builds it. If you already have the version of OpenSSL in the standard location but you want a fresh copy, you must delete your current copy.
:pencil: **OpenSSL support:** If you want to build an OpenJDK with OpenJ9 binary with OpenSSL support and you do not have a built version of OpenSSL v3.x available locally, you must specify `--openssl-version=<version>` where `<version>` is an OpenSSL level like `3.0.10`. If the specified version of OpenSSL is already available in the standard location (`SRC_DIR/openssl`), `get_source.sh` uses it. Otherwise, the script deletes the content and downloads the specified version of OpenSSL source to the standard location and builds it. If you already have the version of OpenSSL in the standard location but you want a fresh copy, you must delete your current copy.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be updated for Java 21 (Java 20 doesn't build with the current OpenJ9 repository), but that can be done in a later pull request if you like.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'll do it separately.

Comment on lines 113 to 115
* License: https://github.com/openssl/openssl/blob/OpenSSL_1_1_1-stable/LICENSE
* Project: https://www.openssl.org/
* Source: https://github.com/openssl/openssl or https://github.com/ibmruntimes/openssl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this still here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just being cautious. OpenJ9 has build instructions for OpenJ9 jenkins builds that set v3 but other builds can do something else. Also on some platforms OpenJ9 uses whatever OpenSSL is found installed. I figure we can remove it sometime down the line when we notice, maybe a couple of years, when OpenSSL 1.1 isn't so widely used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with deferring removal of references to version 1.x, but I don't think we can (or want to) make reference to the license of every version of OpenSSL someone might want to build with. I think we should expect those people to know what they're doing and understand the implications of building against some version and/or repository other than what OpenJ9 (advises and) uses.

I also think the comments

If you built an OpenJDK with OpenJ9 that includes OpenSSL v1.x support ...

are (at best) redundant and should also be removed at the same time we update this file.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't sound like you are asking me to change anything now, if that's not the case pls clarify. The diff between 1.x and 3.x is OpenSSL changed the license, otherwise we wouldn't have two entries.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I won't object if you want to remove the references to 1.x now, but I'm not asking you to do so.

The only change I'm waiting for is to fix the typo mentioned in #14900 (comment).

Copy link
Contributor

@keithc-ca keithc-ca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No changes have been made that should invalidate the build results mentioned in #14900 (comment).

@keithc-ca keithc-ca merged commit ac49658 into eclipse-openj9:master Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants