Skip to content

[Bug] tlsTrustCertsFilePath does not work for OAuth2 authentication #363

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

Closed
2 tasks done
BewareMyPower opened this issue Dec 5, 2023 · 1 comment · Fixed by #364
Closed
2 tasks done

[Bug] tlsTrustCertsFilePath does not work for OAuth2 authentication #363

BewareMyPower opened this issue Dec 5, 2023 · 1 comment · Fixed by #364

Comments

@BewareMyPower
Copy link
Contributor

BewareMyPower commented Dec 5, 2023

Search before asking

  • I searched in the issues and found nothing similar.

Version

OS: macOS m1
Client: https://archive.apache.org/dist/pulsar/pulsar-client-cpp-3.4.1/

Minimal reproduce step

The same reproduce steps with #184

What did you expect to see?

What did you see instead?

Even if setTlsTrustCertsFilePath is configured, the OAuth2 authentication will still fail.

2023-12-05 15:30:28.066 ERROR [0x16d9c7000] AuthOauth2:390 | Response failed for issuerurl <...>. ErrorCode 60: SSL certificate problem: unable to get local issuer certificate passedin: 

Anything else?

It's a regression brought by #313, the tlsTrustCertsFilePath config didn't work for OAuth2.

curl.get(tokenEndPoint_, "Content-Type: application/x-www-form-urlencoded", options, nullptr);

The last argument is nullptr, i.e. CurlWrapper does not load any CA certs for AuthOauth2::authenticate.

While you can see it's applied in

if (!tlsTrustCertsFilePath_.empty()) {
tlsContext.reset(new CurlWrapper::TlsContext);
tlsContext->trustCertsFilePath = tlsTrustCertsFilePath_;
}
auto result = curl.get(wellKnownUrl, "Accept: application/json", {}, tlsContext.get());

Are you willing to submit a PR?

  • I'm willing to submit a PR!
BewareMyPower added a commit to BewareMyPower/pulsar-client-cpp that referenced this issue Dec 5, 2023
@BewareMyPower
Copy link
Contributor Author

The workflow here can describe this issue more clearly.

2023-12-05 12:06:36.958 INFO  [140093743501632] ClientConnection:189 | [<none> -> pulsar://localhost:6650] Create ClientConnection, timeout=10000
2023-12-05 12:06:36.958 INFO  [140093743501632] ConnectionPool:114 | Created connection for pulsar://localhost:6650-0
2023-12-05 12:06:36.962 INFO  [140093735237184] ClientConnection:396 | [[::1]:51810 -> [::1]:6650] Connected to broker
2023-12-05 12:06:36.968 ERROR [140093735237184] AuthOauth2:272 | Response failed for getting the well-known configuration https://dev-kt-aa9ne.us.auth0.com./ Error Code 77: error setting certificate file: /etc/ssl/certs/ca-certificates.crt
2023-12-05 12:06:36.968 ERROR [140093735237184] ClientConnection:515 | [[::1]:51810 -> [::1]:6650] Failed to establish connection: AuthenticationError
2023-12-05 12:06:36.968 ERROR [140093735237184] ClientConnection:1315 | [[::1]:51810 -> [::1]:6650] Connection closed with AuthenticationError (refCnt: 2)
2023-12-05 12:06:36.968 INFO  [140093735237184] ConnectionPool:129 | Remove connection for pulsar://localhost:6650-0
2023-12-05 12:06:36.968 ERROR [140093735237184] ClientImpl:198 | Error Checking/Getting Partition Metadata while creating producer on persistent://public/default/oauth2-test -- AuthenticationError
/home/runner/work/pulsar-client-cpp/pulsar-client-cpp/tests/oauth2/Oauth2Test.cc:84: Failure
Expected equality of these values:
  ResultOk
    Which is: Ok
  client.createProducer("oauth2-test", producer)
    Which is: AuthenticationError
[  FAILED  ] Oauth2Test.testTlsTrustFilePath (12 ms)
    const auto caPath = "/etc/ssl/certs/my-cert.crt";
    /* ... */
    ClientConfiguration conf;
    conf.setTlsTrustCertsFilePath(caPath);
    auto params = gCommonParams;
    params["private_key"] = "file://" + gKeyPath;
    conf.setAuth(AuthOauth2::create(params));

The code above shows the default CA cert (/etc/ssl/certs/ca-certificates.crt) was deleted, even if setting tlsTrustCertsFilePath with the new location (/etc/ssl/certs/my-cert.crt), the OAuth2 authentication still failed.

BewareMyPower added a commit to BewareMyPower/pulsar-client-cpp that referenced this issue Dec 12, 2023
…nux, macOS)

### Motivation

Currently we manage dependencies by ourselves:
- When running unit tests in CI, dependencies are installed from the
  Debian package management tool `apt`
- When verifying macOS build, dependencies are installed from the
  Homebrew package manager
- When verifying Windows build, dependencies are installed via vcpkg but
  no versions are specified
- When building pre-built binaries to release, dependencies are
  installed by scripts under `pkg/` directories. These scripts download
  the source code according to `dependencies.yaml` and build

It makes the pre-built binaries never tested except for the simple
manual tests when voting for a new release. As a result, regression like
apache#363 could happen.

This patch aims at integrating vcpkg to manage dependencies for all
platforms, not only for Windows.

### Modifications

Integrate the CMakeLists.txt with vcpkg by:
1. Introduce vcpkg as a submodule of this project
2. Update `vcpkg.json` to specify dependency versions.
3. Set `CMAKE_TOOLCHAIN_FILE` with the `vcpkg.cmake` in the submodule.

Then, we can simply use `find_package` to find all dependencies and
depend on them via a target like `CURL::libcurl` to have all include
directores and link libraries.

Update the `unit-tests` workflow to verify building the binaries via
vcpkg to test. The README is also updated to show how much simpler to
build with vcpkg now.

### TODO

There are still some tasks that cannot be done by vcpkg:
1. The static library (e.g. `libpulsarwithdeps.a`) that bundles all
   3rd-party dependencies.
2. The pre-built binaries are still built by scripts under `./pkg`
   directory. Specifically, vcpkg does not work with GCC 4.8 so on
   CentOS 7 we still need to build dependencies manually.

So the previous CMakeLists.txt are retained and will be used if
`INTEGRATE_VCPKG` is OFF (by default). They will be removed until the
task above can be done by vcpkg in future.
BewareMyPower added a commit to BewareMyPower/pulsar-client-cpp that referenced this issue Dec 12, 2023
…nux, macOS)

### Motivation

Currently we manage dependencies by ourselves:
- When running unit tests in CI, dependencies are installed from the
  Debian package management tool `apt`
- When verifying macOS build, dependencies are installed from the
  Homebrew package manager
- When verifying Windows build, dependencies are installed via vcpkg but
  no versions are specified
- When building pre-built binaries to release, dependencies are
  installed by scripts under `pkg/` directories. These scripts download
  the source code according to `dependencies.yaml` and build

It makes the pre-built binaries never tested except for the simple
manual tests when voting for a new release. As a result, regression like
apache#363 could happen.

This patch aims at integrating vcpkg to manage dependencies for all
platforms, not only for Windows.

### Modifications

Integrate the CMakeLists.txt with vcpkg by:
1. Introduce vcpkg as a submodule of this project
2. Update `vcpkg.json` to specify dependency versions.
3. Set `CMAKE_TOOLCHAIN_FILE` with the `vcpkg.cmake` in the submodule.

Then, we can simply use `find_package` to find all dependencies and
depend on them via a target like `CURL::libcurl` to have all include
directores and link libraries.

Update the `unit-tests` workflow to verify building the binaries via
vcpkg to test. The README is also updated to show how much simpler to
build with vcpkg now.

### TODO

There are still some tasks that cannot be done by vcpkg:
1. The static library (e.g. `libpulsarwithdeps.a`) that bundles all
   3rd-party dependencies.
2. The pre-built binaries are still built by scripts under `./pkg`
   directory. Specifically, vcpkg does not work with GCC 4.8 so on
   CentOS 7 we still need to build dependencies manually.

So the previous CMakeLists.txt are retained and will be used if
`INTEGRATE_VCPKG` is OFF (by default). They will be removed until the
task above can be done by vcpkg in future.
BewareMyPower added a commit to BewareMyPower/pulsar-client-cpp that referenced this issue Dec 12, 2023
…nux, macOS)

### Motivation

Currently we manage dependencies by ourselves:
- When running unit tests in CI, dependencies are installed from the
  Debian package management tool `apt`
- When verifying macOS build, dependencies are installed from the
  Homebrew package manager
- When verifying Windows build, dependencies are installed via vcpkg but
  no versions are specified
- When building pre-built binaries to release, dependencies are
  installed by scripts under `pkg/` directories. These scripts download
  the source code according to `dependencies.yaml` and build

It makes the pre-built binaries never tested except for the simple
manual tests when voting for a new release. As a result, regression like
apache#363 could happen.

This patch aims at integrating vcpkg to manage dependencies for all
platforms, not only for Windows.

### Modifications

Integrate the CMakeLists.txt with vcpkg by:
1. Introduce vcpkg as a submodule of this project
2. Update `vcpkg.json` to specify dependency versions.
3. Set `CMAKE_TOOLCHAIN_FILE` with the `vcpkg.cmake` in the submodule.

Then, we can simply use `find_package` to find all dependencies and
depend on them via a target like `CURL::libcurl` to have all include
directores and link libraries.

Update the `unit-tests` workflow to verify building the binaries via
vcpkg to test. The README is also updated to show how much simpler to
build with vcpkg now.

### TODO

There are still some tasks that cannot be done by vcpkg:
1. The static library (e.g. `libpulsarwithdeps.a`) that bundles all
   3rd-party dependencies.
2. The pre-built binaries are still built by scripts under `./pkg`
   directory. Specifically, vcpkg does not work with GCC 4.8 so on
   CentOS 7 we still need to build dependencies manually.

So the previous CMakeLists.txt are retained and will be used if
`INTEGRATE_VCPKG` is OFF (by default). They will be removed until the
task above can be done by vcpkg in future.
BewareMyPower added a commit to BewareMyPower/pulsar-client-cpp that referenced this issue Dec 12, 2023
…nux, macOS)

### Motivation

Currently we manage dependencies by ourselves:
- When running unit tests in CI, dependencies are installed from the
  Debian package management tool `apt`
- When verifying macOS build, dependencies are installed from the
  Homebrew package manager
- When verifying Windows build, dependencies are installed via vcpkg but
  no versions are specified
- When building pre-built binaries to release, dependencies are
  installed by scripts under `pkg/` directories. These scripts download
  the source code according to `dependencies.yaml` and build

It makes the pre-built binaries never tested except for the simple
manual tests when voting for a new release. As a result, regression like
apache#363 could happen.

This patch aims at integrating vcpkg to manage dependencies for all
platforms, not only for Windows.

### Modifications

Integrate the CMakeLists.txt with vcpkg by:
1. Introduce vcpkg as a submodule of this project
2. Update `vcpkg.json` to specify dependency versions.
3. Set `CMAKE_TOOLCHAIN_FILE` with the `vcpkg.cmake` in the submodule.

Then, we can simply use `find_package` to find all dependencies and
depend on them via a target like `CURL::libcurl` to have all include
directores and link libraries.

Update the `unit-tests` workflow to verify building the binaries via
vcpkg to test. The README is also updated to show how much simpler to
build with vcpkg now.

### TODO

There are still some tasks that cannot be done by vcpkg:
1. The static library (e.g. `libpulsarwithdeps.a`) that bundles all
   3rd-party dependencies.
2. The pre-built binaries are still built by scripts under `./pkg`
   directory. Specifically, vcpkg does not work with GCC 4.8 so on
   CentOS 7 we still need to build dependencies manually.

So the previous CMakeLists.txt are retained and will be used if
`INTEGRATE_VCPKG` is OFF (by default). They will be removed until the
task above can be done by vcpkg in future.
BewareMyPower added a commit to BewareMyPower/pulsar-client-cpp that referenced this issue Dec 12, 2023
…nux, macOS)

### Motivation

Currently we manage dependencies by ourselves:
- When running unit tests in CI, dependencies are installed from the
  Debian package management tool `apt`
- When verifying macOS build, dependencies are installed from the
  Homebrew package manager
- When verifying Windows build, dependencies are installed via vcpkg but
  no versions are specified
- When building pre-built binaries to release, dependencies are
  installed by scripts under `pkg/` directories. These scripts download
  the source code according to `dependencies.yaml` and build

It makes the pre-built binaries never tested except for the simple
manual tests when voting for a new release. As a result, regression like
apache#363 could happen.

This patch aims at integrating vcpkg to manage dependencies for all
platforms, not only for Windows.

### Modifications

Integrate the CMakeLists.txt with vcpkg by:
1. Introduce vcpkg as a submodule of this project
2. Update `vcpkg.json` to specify dependency versions.
3. Set `CMAKE_TOOLCHAIN_FILE` with the `vcpkg.cmake` in the submodule.

Then, we can simply use `find_package` to find all dependencies and
depend on them via a target like `CURL::libcurl` to have all include
directores and link libraries.

Update the `unit-tests` workflow to verify building the binaries via
vcpkg to test. The README is also updated to show how much simpler to
build with vcpkg now.

### TODO

There are still some tasks that cannot be done by vcpkg:
1. The static library (e.g. `libpulsarwithdeps.a`) that bundles all
   3rd-party dependencies.
2. The pre-built binaries are still built by scripts under `./pkg`
   directory. Specifically, vcpkg does not work with GCC 4.8 so on
   CentOS 7 we still need to build dependencies manually.

So the previous CMakeLists.txt are retained and will be used if
`INTEGRATE_VCPKG` is OFF (by default). They will be removed until the
task above can be done by vcpkg in future.

We also need to update `dependencies.yaml` to be consistent with
`vcpkg.json` if all tasks above cannot be replaced by vcpkg.
BewareMyPower added a commit that referenced this issue Dec 15, 2023
### Motivation

Currently we manage dependencies by ourselves:
- When running unit tests in CI, dependencies are installed from the
  Debian package management tool `apt`
- When verifying macOS build, dependencies are installed from the
  Homebrew package manager
- When verifying Windows build, dependencies are installed via vcpkg but
  no versions are specified
- When building pre-built binaries to release, dependencies are
  installed by scripts under `pkg/` directories. These scripts download
  the source code according to `dependencies.yaml` and build

It makes the pre-built binaries never tested except for the simple
manual tests when voting for a new release. As a result, regression like
#363 could happen.

This patch aims at integrating vcpkg to manage dependencies for all
platforms, not only for Windows.

### Modifications

Integrate the CMakeLists.txt with vcpkg by:
1. Introduce vcpkg as a submodule of this project
2. Update `vcpkg.json` to specify dependency versions.
3. Set `CMAKE_TOOLCHAIN_FILE` with the `vcpkg.cmake` in the submodule.

Then, we can simply use `find_package` to find all dependencies and
depend on them via a target like `CURL::libcurl` to have all include
directores and link libraries.

Update the `unit-tests` workflow to verify building the binaries via
vcpkg to test. The README is also updated to show how much simpler to
build with vcpkg now.

### TODO

There are still some tasks that cannot be done by vcpkg:
1. The static library (e.g. `libpulsarwithdeps.a`) that bundles all
   3rd-party dependencies.
2. The pre-built binaries are still built by scripts under `./pkg`
   directory. Specifically, vcpkg does not work with GCC 4.8 so on
   CentOS 7 we still need to build dependencies manually.

So the previous CMakeLists.txt are retained and will be used if
`INTEGRATE_VCPKG` is OFF (by default). They will be removed until the
task above can be done by vcpkg in future.

We also need to update `dependencies.yaml` to be consistent with
`vcpkg.json` if all tasks above cannot be replaced by vcpkg.
BewareMyPower added a commit to streamnative/pulsar-client-cpp that referenced this issue Jul 4, 2024
### Motivation

Currently we manage dependencies by ourselves:
- When running unit tests in CI, dependencies are installed from the
  Debian package management tool `apt`
- When verifying macOS build, dependencies are installed from the
  Homebrew package manager
- When verifying Windows build, dependencies are installed via vcpkg but
  no versions are specified
- When building pre-built binaries to release, dependencies are
  installed by scripts under `pkg/` directories. These scripts download
  the source code according to `dependencies.yaml` and build

It makes the pre-built binaries never tested except for the simple
manual tests when voting for a new release. As a result, regression like
apache#363 could happen.

This patch aims at integrating vcpkg to manage dependencies for all
platforms, not only for Windows.

### Modifications

Integrate the CMakeLists.txt with vcpkg by:
1. Introduce vcpkg as a submodule of this project
2. Update `vcpkg.json` to specify dependency versions.
3. Set `CMAKE_TOOLCHAIN_FILE` with the `vcpkg.cmake` in the submodule.

Then, we can simply use `find_package` to find all dependencies and
depend on them via a target like `CURL::libcurl` to have all include
directores and link libraries.

Update the `unit-tests` workflow to verify building the binaries via
vcpkg to test. The README is also updated to show how much simpler to
build with vcpkg now.

### TODO

There are still some tasks that cannot be done by vcpkg:
1. The static library (e.g. `libpulsarwithdeps.a`) that bundles all
   3rd-party dependencies.
2. The pre-built binaries are still built by scripts under `./pkg`
   directory. Specifically, vcpkg does not work with GCC 4.8 so on
   CentOS 7 we still need to build dependencies manually.

So the previous CMakeLists.txt are retained and will be used if
`INTEGRATE_VCPKG` is OFF (by default). They will be removed until the
task above can be done by vcpkg in future.

We also need to update `dependencies.yaml` to be consistent with
`vcpkg.json` if all tasks above cannot be replaced by vcpkg.

(cherry picked from commit 7baa312)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant