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

todo: update to openssl 1.1.1e on March 17th #32210

Closed
sam-github opened this issue Mar 11, 2020 · 25 comments
Closed

todo: update to openssl 1.1.1e on March 17th #32210

sam-github opened this issue Mar 11, 2020 · 25 comments
Labels
help wanted Issues that need assistance from volunteers or PRs that need help to proceed. security Issues and PRs related to security. tls Issues and PRs related to the tls subsystem.

Comments

@sam-github
Copy link
Contributor

sam-github commented Mar 11, 2020

https://mta.openssl.org/pipermail/openssl-announce/2020-March/000166.html announced an upcoming OpenSSL release.

I normally do these, but if any other collaborator would like to get involved in the TLS maintenance, this is a good place to start.

The maintenance guide is pretty clear, but is moving, check the PR: #32209

If there is someone who would like to do this, please comment here, and I'll be available to help if needed. If not, I'll do it.

EDIT: and note that we are currently floating a patch, but that won't be necessary after this upcoming update:

On 11/03/2020 17:42, Sam Roberts wrote:
> Will it include ONLY the CVE fix, or will it include other fixes, such
> as to the getrandom() call on some archs?

It will include all fixes currently in the 1.1.1-dev branch including
commit eee565ec4 which is the 1.1.1 equivalent of the commit you mention.

Matt
@sam-github sam-github added help wanted Issues that need assistance from volunteers or PRs that need help to proceed. security Issues and PRs related to security. tls Issues and PRs related to the tls subsystem. labels Mar 11, 2020
@hassaanp
Copy link
Contributor

Hey, I would like to help.

I am currently running Ubuntu 18.04, but I can quickly spin off a Docker for CentOS7.1 or Ubuntu 16.04 for this.

@hassaanp
Copy link
Contributor

Update:
I have set up the required environment. OpenSSL 1.1.1e will be available at UTC 1300 on March 17th. As soon as it comes up, I will download, test and submit the PR.

@sam-github
Copy link
Contributor Author

I am currently running Ubuntu 18.04, but I can quickly spin off a Docker for CentOS7.1 or Ubuntu 16.04 for this.

@hassaanp what gives you the impression ubuntu 18.04 isn't perfectly adequate? I'm a bit concerned, is there something in the docs that suggests that?

I run Ubuntu 19.10 myself, ATM, but it shouldn't matter.

You can, btw, do a dry run right now, running through the steps, but using the 1.1.1d archive. Nothing will change, and obviously you won't PR the result, but you'll get a chance to see how the config process works.

@hassaanp
Copy link
Contributor

hassaanp commented Mar 12, 2020

I will do the dry run as per your recommendation.

In the requirements it is mentioned that only Centos 7.1 and Ubuntu 16 are tested. I assumed that is the general recommendation.

@sam-github
Copy link
Contributor Author

Thanks, I just fixed that in #32209

@hassaanp
Copy link
Contributor

I have successfully done a test run - the new version should be out in the next 6 hours.

@hassaanp
Copy link
Contributor

The update is still not available. Will recheck in an hour

@hassaanp
Copy link
Contributor

New version does not build cleanly.

This is the error output when I run make in the deps/openssl/config directory

usr/bin/perl "-I." -Mconfigdata "util/dofile.pl" \
    "-oMakefile" include/crypto/bn_conf.h.in > include/crypto/bn_conf.h
/usr/bin/perl "-I." -Mconfigdata "util/dofile.pl" \
    "-oMakefile" include/crypto/dso_conf.h.in > include/crypto/dso_conf.h
/usr/bin/perl "-I." -Mconfigdata "util/dofile.pl" \
    "-oMakefile" include/openssl/opensslconf.h.in > include/openssl/opensslconf.h
/usr/bin/perl util/mkbuildinf.pl "gcc -pthread -Wa,--noexecstack -O -DB_ENDIAN -DOPENSSL_PIC -DOPENSSL_CPUID_OBJ -DOPENSSL_BN_ASM_MONT -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DAES_ASM -DVPAES_ASM -DPOLY1305_ASM -DNDEBUG" "aix-gcc" > crypto/buildinf.h
/usr/bin/perl apps/progs.pl apps/openssl > apps/progs.h
make[1]: Leaving directory '/home/hassaan/Study/node/deps/openssl/openssl'
Move failed: No such file or directory at ./generate_gypi.pl line 65.
Makefile:50: recipe for target 'aix-gcc' failed
make: *** [aix-gcc] Error 2

The generate_gypi.pl is there where it should be. Any idea what could be wrong?

@sam-github
Copy link
Contributor Author

I will look.

@sam-github
Copy link
Contributor Author

There is no openssl-1.1.1e: https://www.openssl.org/source/

I'm not clear what you were doing when you encountered the above.

@hassaanp
Copy link
Contributor

strange, I was able to pull in using
wget https://www.openssl.org/source/openssl-1.1.1e.tar.gz

@hassaanp
Copy link
Contributor

I was able to fix the issue
There are some path changes that need to go into Makefile and generate_gypi.pl

@hassaanp
Copy link
Contributor

@sam-github
The make runs fine now
But the node build fails. There are no headers present for many of the files in openssl/openssl/crypto/ directory.These headers are named <file>_local.h
I am wondering whether I am missing some build step perhaps.

@sam-github
Copy link
Contributor Author

I reproduced, but I'm not going to spend time looking at it until openssl-1.1.1e is released. For all we know, that's an internal rc, partially built, and missing files.

@hassaanp
Copy link
Contributor

The file is now officially available on their downloads page. Also received an email from them.

 The OpenSSL project team is pleased to announce the release of
   version 1.1.1e of our open source toolkit for SSL/TLS. For details
   of changes and known issues see the release notes at:

        https://www.openssl.org/news/openssl-1.1.1-notes.html

   OpenSSL 1.1.1e is available for download via HTTP and FTP from the
   following master locations (you can find the various FTP mirrors under
   https://www.openssl.org/source/mirror.html):

     * https://www.openssl.org/source/
     * ftp://ftp.openssl.org/source/

   The distribution file name is:

    o openssl-1.1.1e.tar.gz
      Size: 9792634
      SHA1 checksum: e7105567d3e7e6353a0110f1adc81f69dbc8f732
      SHA256 checksum: 694f61ac11cb51c9bf73f54e771ff6022b0327a43bbdfa1b2f19de1662a6dcbe

   The checksums were calculated using the following commands:

    openssl sha1 openssl-1.1.1e.tar.gz
    openssl sha256 openssl-1.1.1e.tar.gz

   Yours,

   The OpenSSL Project Team.

The same error was reproduced by downloading from the official release link.

@sam-github
Copy link
Contributor Author

OK, sorry, openssl updates have been routine for a long time, and this one is not. I'll have to figure out what changed.

@sam-github
Copy link
Contributor Author

openssl/openssl#9681 looks to be responsible.

@hassaanp
Copy link
Contributor

I have been able to build it successfully - currently running tests.
I will push the code shortly if everything looks okay.

@hassaanp
Copy link
Contributor

hassaanp commented Mar 17, 2020

one test failing

...../node/test/parallel/test-tls-session-cache.js:65
        throw er;
        ^

[Error: 139969577846656:error:1409441A:SSL routines:ssl3_read_bytes:tlsv1 alert decode error:../deps/openssl/openssl/ssl/record/rec_layer_s3.c:1550:SSL alert number 50
] {
  library: 'SSL routines',
  function: 'ssl3_read_bytes',
  reason: 'tlsv1 alert decode error',
  code: 'ERR_SSL_TLSV1_ALERT_DECODE_ERROR'
}

is this cause for concern?

@hassaanp
Copy link
Contributor

I have pushed the update so you can take a look at the changes I made in

  1. generate_gypi.pl
  2. Makefiles in deps/openssl/config

One test is still failing as mentioned in my previous comment.

@hassaanp
Copy link
Contributor

I dug deeper into the test
the test wants the error code to be ECONNRESET but the returned error code is ERR_SSL_TLSV1_ALERT_DECODE_ERROR

@sam-github
Copy link
Contributor Author

^-- above discussion should move to the PR, and a stand-alone commit that fixes the test would likely be needed. And yes, its an issue, PR can't land with test regressions.

@hassaanp
Copy link
Contributor

Aknowledged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issues that need assistance from volunteers or PRs that need help to proceed. security Issues and PRs related to security. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants