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

[improve][cpp] Upgrade OpenSSL to version 1.1.1n #17538

Merged
merged 2 commits into from
Sep 13, 2022

Conversation

RobertIndie
Copy link
Member

Motivation

Currently, there exists two different OpenSSL version in the repo: 1.1.1n and 1.1.0j. This PR upgrades all these OpenSSL dependencies to 1.1.1n.

Modifications

  • Upgrade OpenSSL to version 1.1.1n

Documentation

Check the box below or label this PR directly.

Need to update docs?

  • doc-required
    (Your PR needs to update docs and you will update later)

  • doc-not-needed
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

@RobertIndie RobertIndie self-assigned this Sep 8, 2022
@RobertIndie RobertIndie added type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages component/client-c++ labels Sep 8, 2022
@RobertIndie RobertIndie added this to the 2.12.0 milestone Sep 8, 2022
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 9, 2022
@Jason918
Copy link
Contributor

@RobertIndie Please check CI.

@merlimat merlimat merged commit 06bac43 into apache:master Sep 13, 2022
@lhotari
Copy link
Member

lhotari commented Sep 13, 2022

@RobertIndie Please fix this issue:

Building debian packages fails in https://github.com/apache/pulsar/actions/runs/3019434098/jobs/4855451903#step:8:29495

../lib/.libs/libcurl.so: undefined reference to `SSL_CTX_set_keylog_callback@OPENSSL_1_1_1'
../lib/.libs/libcurl.so: undefined reference to `SSL_CTX_set_ciphersuites@OPENSSL_1_1_1'
collect2: error: ld returned 1 exit status
make[2]: *** [curl] Error 1
Makefile:839: recipe for target 'curl' failed
make[2]: Leaving directory '/curl-7.61.0/src'
make[1]: *** [all-recursive] Error 1
Makefile:1878: recipe for target 'all-recursive' failed
make[1]: Leaving directory '/curl-7.61.0/src'
Makefile:927: recipe for target 'all-recursive' failed
make: *** [all-recursive] Error 1

That slipped into master since the CI wasn't preventing merging of builds with failing checks temporarily and this PR wasn't rebased.

@BewareMyPower
Copy link
Contributor

FYI, I will handle this issue.

BewareMyPower added a commit to BewareMyPower/pulsar that referenced this pull request Sep 13, 2022
### Motivation

See apache#17538 (comment)

The root cause is when libcurl is built from source, it uses
[`ld`](https://linux.die.net/man/1/ld) to check if the `libcurl.so`
links to the correct dependencies in runtime. In Linux, a dynamic
library links to the paths of `/etc/ld.so.conf` by default. However,
different from other images like `centos:7` and `alpine`, this file
includes `/usr/lib/x86_64-linux-gnu` in `debian:9`.

```bash
$ cat /etc/ld.so.conf
include /etc/ld.so.conf.d/*.conf

$ cat /etc/ld.so.conf.d/*.conf
/usr/lib/x86_64-linux-gnu/libfakeroot
# libc default configuration
/usr/local/lib
# Multiarch support
/lib/x86_64-linux-gnu
/usr/lib/x86_64-linux-gnu
```

When libcurl is compiled, it links to the install path of libopenssl via
the `--with-ssl` option:

https://github.com/apache/pulsar/blob/1f50366768e76f1a5f7084f7972167f989ddd0af/pulsar-client-cpp/pkg/deb/Dockerfile#L85

i.e. `/usr/local/ssl/lib/libopenssl.so`.

However, after the `libcurl.so` is built, it links to
`/usr/lib/x86_64-linux-gnu/libssl.so.1.1`, see the following output:

```bash
$ ldd /usr/local/lib/libcurl.so
/usr/local/lib/libcurl.so: /usr/lib/x86_64-linux-gnu/libssl.so.1.1: version `OPENSSL_1_1_1' not found (required by /usr/local/lib/libcurl.so)
```

In `debian:9`, the default libopenssl version is 1.1.0:

```bash
$ strings /usr/lib/x86_64-linux-gnu/libssl.so.1.1 | grep OpenSSL
OpenSSL 1.1.0l  10 Sep 2019
```

The ABI compatibility is not guaranteed between 1.1.0l and 1.1.1n, see
https://abi-laboratory.pro/index.php?view=timeline&l=openssl.

### Modifications

Set the `LD_LIBRARY_PATH` to `/usr/local/ssl/lib` in the Dockerfile to
build deb package.

Actually it's not required for other images like `centos:7`, but it's
also good to add the `LD_LIBRARY_PATH` to them. So this PR set the
environment variable to them as well.
lhotari added a commit that referenced this pull request Sep 13, 2022
* [fix][cpp] Fix libcurl build failure when building deb package

### Motivation

See #17538 (comment)

The root cause is when libcurl is built from source, it uses
[`ld`](https://linux.die.net/man/1/ld) to check if the `libcurl.so`
links to the correct dependencies in runtime. In Linux, a dynamic
library links to the paths of `/etc/ld.so.conf` by default. However,
different from other images like `centos:7` and `alpine`, this file
includes `/usr/lib/x86_64-linux-gnu` in `debian:9`.

```bash
$ cat /etc/ld.so.conf
include /etc/ld.so.conf.d/*.conf

$ cat /etc/ld.so.conf.d/*.conf
/usr/lib/x86_64-linux-gnu/libfakeroot
# libc default configuration
/usr/local/lib
# Multiarch support
/lib/x86_64-linux-gnu
/usr/lib/x86_64-linux-gnu
```

When libcurl is compiled, it links to the install path of libopenssl via
the `--with-ssl` option:

https://github.com/apache/pulsar/blob/1f50366768e76f1a5f7084f7972167f989ddd0af/pulsar-client-cpp/pkg/deb/Dockerfile#L85

i.e. `/usr/local/ssl/lib/libopenssl.so`.

However, after the `libcurl.so` is built, it links to
`/usr/lib/x86_64-linux-gnu/libssl.so.1.1`, see the following output:

```bash
$ ldd /usr/local/lib/libcurl.so
/usr/local/lib/libcurl.so: /usr/lib/x86_64-linux-gnu/libssl.so.1.1: version `OPENSSL_1_1_1' not found (required by /usr/local/lib/libcurl.so)
```

In `debian:9`, the default libopenssl version is 1.1.0:

```bash
$ strings /usr/lib/x86_64-linux-gnu/libssl.so.1.1 | grep OpenSSL
OpenSSL 1.1.0l  10 Sep 2019
```

The ABI compatibility is not guaranteed between 1.1.0l and 1.1.1n, see
https://abi-laboratory.pro/index.php?view=timeline&l=openssl.

### Modifications

Set the `LD_LIBRARY_PATH` to `/usr/local/ssl/lib` in the Dockerfile to
build deb package.

Actually it's not required for other images like `centos:7`, but it's
also good to add the `LD_LIBRARY_PATH` to them. So this PR set the
environment variable to them as well.

* Fix workflow so that cpp-tests isn't skipped

* Revisit workflow fix to cover doc only workflows too

* Fix multi-line condition

Co-authored-by: Lari Hotari <[email protected]>
tisonkun pushed a commit to tisonkun/pulsar that referenced this pull request Sep 14, 2022
tisonkun pushed a commit to tisonkun/pulsar that referenced this pull request Sep 14, 2022
…e#17614)

* [fix][cpp] Fix libcurl build failure when building deb package

### Motivation

See apache#17538 (comment)

The root cause is when libcurl is built from source, it uses
[`ld`](https://linux.die.net/man/1/ld) to check if the `libcurl.so`
links to the correct dependencies in runtime. In Linux, a dynamic
library links to the paths of `/etc/ld.so.conf` by default. However,
different from other images like `centos:7` and `alpine`, this file
includes `/usr/lib/x86_64-linux-gnu` in `debian:9`.

```bash
$ cat /etc/ld.so.conf
include /etc/ld.so.conf.d/*.conf

$ cat /etc/ld.so.conf.d/*.conf
/usr/lib/x86_64-linux-gnu/libfakeroot
# libc default configuration
/usr/local/lib
# Multiarch support
/lib/x86_64-linux-gnu
/usr/lib/x86_64-linux-gnu
```

When libcurl is compiled, it links to the install path of libopenssl via
the `--with-ssl` option:

https://github.com/apache/pulsar/blob/1f50366768e76f1a5f7084f7972167f989ddd0af/pulsar-client-cpp/pkg/deb/Dockerfile#L85

i.e. `/usr/local/ssl/lib/libopenssl.so`.

However, after the `libcurl.so` is built, it links to
`/usr/lib/x86_64-linux-gnu/libssl.so.1.1`, see the following output:

```bash
$ ldd /usr/local/lib/libcurl.so
/usr/local/lib/libcurl.so: /usr/lib/x86_64-linux-gnu/libssl.so.1.1: version `OPENSSL_1_1_1' not found (required by /usr/local/lib/libcurl.so)
```

In `debian:9`, the default libopenssl version is 1.1.0:

```bash
$ strings /usr/lib/x86_64-linux-gnu/libssl.so.1.1 | grep OpenSSL
OpenSSL 1.1.0l  10 Sep 2019
```

The ABI compatibility is not guaranteed between 1.1.0l and 1.1.1n, see
https://abi-laboratory.pro/index.php?view=timeline&l=openssl.

### Modifications

Set the `LD_LIBRARY_PATH` to `/usr/local/ssl/lib` in the Dockerfile to
build deb package.

Actually it's not required for other images like `centos:7`, but it's
also good to add the `LD_LIBRARY_PATH` to them. So this PR set the
environment variable to them as well.

* Fix workflow so that cpp-tests isn't skipped

* Revisit workflow fix to cover doc only workflows too

* Fix multi-line condition

Co-authored-by: Lari Hotari <[email protected]>
@Jason918
Copy link
Contributor

@RobertIndie Can you help cherry-pick this to branch-2.10? There are some conflicts.

@BewareMyPower
Copy link
Contributor

We should cherry-pick #17614 as well because it's a fix on this PR.

Jason918 pushed a commit that referenced this pull request Sep 17, 2022
Jason918 pushed a commit that referenced this pull request Sep 17, 2022
* [fix][cpp] Fix libcurl build failure when building deb package

See #17538 (comment)

The root cause is when libcurl is built from source, it uses
[`ld`](https://linux.die.net/man/1/ld) to check if the `libcurl.so`
links to the correct dependencies in runtime. In Linux, a dynamic
library links to the paths of `/etc/ld.so.conf` by default. However,
different from other images like `centos:7` and `alpine`, this file
includes `/usr/lib/x86_64-linux-gnu` in `debian:9`.

```bash
$ cat /etc/ld.so.conf
include /etc/ld.so.conf.d/*.conf

$ cat /etc/ld.so.conf.d/*.conf
/usr/lib/x86_64-linux-gnu/libfakeroot
/usr/local/lib
/lib/x86_64-linux-gnu
/usr/lib/x86_64-linux-gnu
```

When libcurl is compiled, it links to the install path of libopenssl via
the `--with-ssl` option:

https://github.com/apache/pulsar/blob/1f50366768e76f1a5f7084f7972167f989ddd0af/pulsar-client-cpp/pkg/deb/Dockerfile#L85

i.e. `/usr/local/ssl/lib/libopenssl.so`.

However, after the `libcurl.so` is built, it links to
`/usr/lib/x86_64-linux-gnu/libssl.so.1.1`, see the following output:

```bash
$ ldd /usr/local/lib/libcurl.so
/usr/local/lib/libcurl.so: /usr/lib/x86_64-linux-gnu/libssl.so.1.1: version `OPENSSL_1_1_1' not found (required by /usr/local/lib/libcurl.so)
```

In `debian:9`, the default libopenssl version is 1.1.0:

```bash
$ strings /usr/lib/x86_64-linux-gnu/libssl.so.1.1 | grep OpenSSL
OpenSSL 1.1.0l  10 Sep 2019
```

The ABI compatibility is not guaranteed between 1.1.0l and 1.1.1n, see
https://abi-laboratory.pro/index.php?view=timeline&l=openssl.

Set the `LD_LIBRARY_PATH` to `/usr/local/ssl/lib` in the Dockerfile to
build deb package.

Actually it's not required for other images like `centos:7`, but it's
also good to add the `LD_LIBRARY_PATH` to them. So this PR set the
environment variable to them as well.

* Fix workflow so that cpp-tests isn't skipped

* Revisit workflow fix to cover doc only workflows too

* Fix multi-line condition

Co-authored-by: Lari Hotari <[email protected]>
(cherry picked from commit 0754ea1)
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Sep 20, 2022
(cherry picked from commit 06bac43)
(cherry picked from commit fa66f85)
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Sep 20, 2022
…e#17614)

* [fix][cpp] Fix libcurl build failure when building deb package

See apache#17538 (comment)

The root cause is when libcurl is built from source, it uses
[`ld`](https://linux.die.net/man/1/ld) to check if the `libcurl.so`
links to the correct dependencies in runtime. In Linux, a dynamic
library links to the paths of `/etc/ld.so.conf` by default. However,
different from other images like `centos:7` and `alpine`, this file
includes `/usr/lib/x86_64-linux-gnu` in `debian:9`.

```bash
$ cat /etc/ld.so.conf
include /etc/ld.so.conf.d/*.conf

$ cat /etc/ld.so.conf.d/*.conf
/usr/lib/x86_64-linux-gnu/libfakeroot
/usr/local/lib
/lib/x86_64-linux-gnu
/usr/lib/x86_64-linux-gnu
```

When libcurl is compiled, it links to the install path of libopenssl via
the `--with-ssl` option:

https://github.com/apache/pulsar/blob/1f50366768e76f1a5f7084f7972167f989ddd0af/pulsar-client-cpp/pkg/deb/Dockerfile#L85

i.e. `/usr/local/ssl/lib/libopenssl.so`.

However, after the `libcurl.so` is built, it links to
`/usr/lib/x86_64-linux-gnu/libssl.so.1.1`, see the following output:

```bash
$ ldd /usr/local/lib/libcurl.so
/usr/local/lib/libcurl.so: /usr/lib/x86_64-linux-gnu/libssl.so.1.1: version `OPENSSL_1_1_1' not found (required by /usr/local/lib/libcurl.so)
```

In `debian:9`, the default libopenssl version is 1.1.0:

```bash
$ strings /usr/lib/x86_64-linux-gnu/libssl.so.1.1 | grep OpenSSL
OpenSSL 1.1.0l  10 Sep 2019
```

The ABI compatibility is not guaranteed between 1.1.0l and 1.1.1n, see
https://abi-laboratory.pro/index.php?view=timeline&l=openssl.

Set the `LD_LIBRARY_PATH` to `/usr/local/ssl/lib` in the Dockerfile to
build deb package.

Actually it's not required for other images like `centos:7`, but it's
also good to add the `LD_LIBRARY_PATH` to them. So this PR set the
environment variable to them as well.

* Fix workflow so that cpp-tests isn't skipped

* Revisit workflow fix to cover doc only workflows too

* Fix multi-line condition

Co-authored-by: Lari Hotari <[email protected]>
(cherry picked from commit 0754ea1)
(cherry picked from commit 7162c30)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-2.10 doc-not-needed Your PR changes do not impact docs release/2.10.2 type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants