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.

Sign up for GitHub

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

Jump to bottom

Wolfssl #17965

Draft
wants to merge 31 commits into
base: master
Choose a base branch
from
Draft

Wolfssl #17965

wants to merge 31 commits into from

Conversation

thiagoftsm
Copy link
Contributor

@thiagoftsm thiagoftsm commented Jun 19, 2024 •
edited
Loading

Summary

Fixes #6509

This PR is also fixing some issues when we compile netdata disabling ACLK.

Test Plan
  1. Before to compile and test this branch, it is necessary to install wolfssl on your system. The default compilation of the library won't allow everything necessary to have ACLK debug messages and cloud, to fix this I compiled it using the following options:
# ./autogen.sh
# CFLAGS="-DOPENSSL_EXTRA -DHAVE_SECRET_CALLBACK -DWOLFSSL_TRUST_PEER_CERT" ./configure --prefix=/usr --enable-all --enable-static --enable-iopool --enable-secure-renegotiation --enable-sni
# make  CFLAGS="-DOPENSSL_EXTRA -DHAVE_SECRET_CALLBACK -DWOLFSSL_TRUST_PEER_CERT"
# make install
# ldconfig
  1. Compile this branch without to modify any option.
  2. Compile this branch enabling Wolfssl .
  3. To confirm that this PR is working, you can access our API (https://localhost:19999/api/v1/info). It is suggested to use info endpoint, because you can test both SSL and streaming with it.
Additional Information

To support ACLK and cloud, it will be necessary to do more changes in the source code. So this is the first step to bring the library.

This PR was tested on scenarios:

Server Server SSL Lib Client Client SSL Lib
Slackware current OpenSSL Arch Open SSL
Slackware current WolfSSL Arch Open SSL
Slackware current OpenSSL Ubuntu 23.04 Open SSL
Slackware current OpenSSL Ubuntu 23.04 Wolf SSL
Slackware current WolfSSL Ubuntu 23.04 Open SSL
Slackware current WolfSSL Ubuntu 23.04 Wolf SSL
For users: How does this change affect me?

@thiagoftsm thiagoftsm marked this pull request as draft June 19, 2024 23:19
@thiagoftsm thiagoftsm force-pushed the wolfssl branch 2 times, most recently from 37cf389 to 2fb62b7 Compare June 24, 2024 21:11
@github-actions github-actions bot added area/collectors Everything related to data collection collectors/plugins.d labels Jun 25, 2024
@thiagoftsm thiagoftsm changed the title [WIP] Wolfssl Wolfssl Jun 25, 2024
@thiagoftsm thiagoftsm marked this pull request as ready for review June 25, 2024 03:12
@thiagoftsm
Copy link
Contributor Author

@Ferroin, I remember you mentioned having a unique variable in the CMakeLists.txt for both libraries, but I can't recall the exact details. Please feel free to push a commit or leave a comment so I can make the necessary changes.

@stelfrag
Copy link
Collaborator

@thiagoftsm check the conflict please

@thiagoftsm
Copy link
Contributor Author

@thiagoftsm check the conflict please

I fixed the conflict and I retest. Everything is working on my environment.

Ferroin
Ferroin reviewed Jul 12, 2024
View reviewed changes
Copy link
Member

@Ferroin Ferroin left a comment

Choose a reason for hiding this comment

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

Only three comments on the CMake code, but otherwise looks good to me.

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@thiagoftsm thiagoftsm force-pushed the wolfssl branch 2 times, most recently from 930b5e1 to d2fff24 Compare July 14, 2024 15:30
Ferroin
Ferroin requested changes Jul 16, 2024
View reviewed changes
CMakeLists.txt Outdated
Comment on lines 154 to 155
set(ENABLE_OPENSSL True)
set(ENABLE_WOLFSSL False)
Copy link
Member

Choose a reason for hiding this comment

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

My meaning here was that ENABLE_WOLFSSL should be an option, but ENABLE_OPENSSL should be set to true or false based on the value of that option.

Framed a bit differently, exactly one of these variables must be True. We can’t build if both are False (because we decided to have a hard dependency on a TLS implementation) or both are True (because that would break the abstraction layer). The original implementation you had here ensured that it wasn’t possible for both to be True, but not that it wasn’t possible for both to be False.

What I’m suggesting is essentially this:

Suggested change
set(ENABLE_OPENSSL True)
set(ENABLE_WOLFSSL False)
option(ENABLE_WOLFSSL "Use WolfSSL in place of OpenSSL for TLS and crypto support" False)
if(ENABLE_WOLFSSL)
set(ENABLE_OPENSSL False)
else()
set(ENABLE_OPENSSL True)
endif()

@thiagoftsm
wolfssl: Change aclk.c
a97f9cb
@thiagoftsm
wolfssl: Change mqtt_wss_client
880ed5b
@thiagoftsm
wolfssl: Change ws_client
cb5932c
@thiagoftsm
wolfssl: Change claim.c
d190d35
@thiagoftsm
wolfssl: Change buildinfo.
a3dfc65
@thiagoftsm
wolfssl: Change commands
2a0da41
@thiagoftsm
wolfssl: Change api_v2.c
5b6cc3f
@thiagoftsm
wolfssl: Change contexts/worker.c
e6cc8f7
@thiagoftsm
wolfssl: Change database/engine/rrdengine.h
56f3240
@thiagoftsm
wolfssl: Change rrdfunctions-inflight
5f78de4
@thiagoftsm
wolfssl: Change rrdhost
7b64593
@thiagoftsm
wolfssl: Add new SSL header
eee274d
@thiagoftsm
ebpf_cgroup: Update libnetdata
60b2142
@thiagoftsm
ebpf_cgroup: Add README.md
92ddaad
@thiagoftsm
ebpf_cgroup: Update security.c
bc32185
@thiagoftsm
ebpf_cgroup: Update security.h
8b8cc0b
@thiagoftsm
ebpf_cgroup: Update streaming/receiver.c
fb40a17
@thiagoftsm
ebpf_cgroup: Update streaming/sender.c
f161307
@thiagoftsm
ebpf_cgroup: Update web_api_v1
d46bc74
@thiagoftsm
win_sys_info: Update CMakeLists
6eeb77e
@thiagoftsm
win_sys_info: Fix compilation
552f5ea
@thiagoftsm
wolfssl: fix compilation on Ubuntu 23.04
4f46dbb
@thiagoftsm
wolfssl: Use only one pattern for database files
6bb4ede
@thiagoftsm
wolfssl: Finish changes to have a unique pattern
036cbd1
@thiagoftsm
wolfssl: Change pluginsd
fba182d
@thiagoftsm
wolfssl: accept suggestion
f5130ed
@thiagoftsm
wolfssl: Add new function
5fbd9fe
@thiagoftsm
update_libbpf: Disable wolfssl by default again
33b6e0e
@thiagoftsm
update_libbpf: Change the way SSL library is defined
3939496
@thiagoftsm
wolfssl: Adjust options
a9fd054
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ferroin Ferroin Ferroin requested changes

@vkalintiris vkalintiris Awaiting requested review from vkalintiris vkalintiris is a code owner

@Ancairon Ancairon Awaiting requested review from Ancairon Ancairon is a code owner

@stelfrag stelfrag Awaiting requested review from stelfrag stelfrag is a code owner

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
area/aclk area/build Build system (autotools and cmake). area/claim area/collectors Everything related to data collection area/daemon area/database area/docs area/packaging Packaging and operating systems support area/streaming area/web collectors/plugins.d
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for other TLS/SSL libraries than OpenSSL
3 participants
@thiagoftsm @stelfrag @Ferroin