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

update Civetweb 1.15 to support openssl 1.1 #1782

Merged
merged 11 commits into from
Nov 17, 2021
Merged

update Civetweb 1.15 to support openssl 1.1 #1782

merged 11 commits into from
Nov 17, 2021

Conversation

FedeDP
Copy link
Contributor

@FedeDP FedeDP commented Nov 11, 2021 •
edited by leogr
Loading

wip: this PR requires falcosecurity/libs#91 to be merged on libs!!

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

/area build

What this PR does / why we need it:

The PR does some things:

  • update civetweb to latest release (1.15)
  • properly move civetweb to its own cmake module
  • properly build civetweb fully using cmake (instead of make)
  • build civetweb disabling dynamic loading of crypto lib, linking instead openssl built with falcosecurity-libs (be it from system or a static library built with BUNDLED_DEPS). This should fix lots of incompatibilities that arose when eg: civetweb was built with openssl 1.0.0, but at runtime it found openssl1.1 on your system

Which issue(s) this PR fixes:

Fixes #1708

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

build: upgrade civetweb to v1.15

@poiana poiana added the size/M label Nov 11, 2021
@FedeDP
Copy link
Contributor Author

FedeDP commented Nov 11, 2021

Error is something that appeared on my local env too, but i guess it has nothing to do with my PR(?):

Comparing engine fields checksum in falco_engine.h to actual fields
Set of fields supported has changed (new checksum 64c449b28013e400cd4d67c993a253752199d749f820c90c4314946c999c5a9f != old checksum 3153c620bf3640fab536a2d0e42aa69f10482d741ef3fa72fd8d994b0261ec55).
Update checksum and/or version in falco_engine_version.h.

@leogr any idea?

@LucaGuerra
Copy link
Contributor

LucaGuerra commented Nov 12, 2021 •
edited
Loading

Are you compiling against a local libs repo? Perhaps the libs instance you're using has a different set of fields (maybe you are in this branch: falcosecurity/libs#126 )

@FedeDP
Copy link
Contributor Author

FedeDP commented Nov 12, 2021 •
edited
Loading

Are you compiling against a local libs repo? Perhaps the libs instance you're using has a different set of fields (maybe you are in this branch: falcosecurity/libs#126 )

Yep but i am building against falcosecurity/libs#91, thus that should make no really differences!

@FedeDP
Copy link
Contributor Author

FedeDP commented Nov 12, 2021

Nevermind, falcosecurity/libs#91 was outdated; now rebased on master. This should fix the issue!

@FedeDP
Copy link
Contributor Author

FedeDP commented Nov 12, 2021

ci/circleci: musl — Your tests failed on CircleCI

This test was fixed in #1753 PR.

@FedeDP
Copy link
Contributor Author

FedeDP commented Nov 15, 2021

Rebased on master; dropped wip commit.

@leogr
Copy link
Member

leogr commented Nov 15, 2021

/check-dco

@FedeDP
Copy link
Contributor Author

FedeDP commented Nov 15, 2021

@mstemm as you reviewed the libs part for this work, can you take a look at this one too?
Thanks!

@FedeDP FedeDP mentioned this pull request Nov 16, 2021
Merged
FedeDP and others added 11 commits November 17, 2021 09:35
@FedeDP @leogr
update(build): updated civetweb to version 1.15 to correctly support …
9c7ada0 
…openssl1.1.

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

Co-authored-by: Leonardo Grasso <me@leonardograsso.com>
@FedeDP
fix(build): build civetweb using cmake and linking to static openssl …
b45f5a6 
…built by us.

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
@FedeDP
update(build): moved civetweb to its own cmake module. Moved its patc…
771286b 
…h too.

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
@FedeDP
update(build): latest libs correctly set OPENSSL_LIBRARIES for us.
fa73bba 
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
@FedeDP
update(build): update falcosecurity libs to use my own libs repo and …
2a58a4c 
…version to be able to test the build against FedeDP:fix_ssl_1_1_get_all_data branch (not yet merged).

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
@FedeDP @leogr
update(build): civetweb depends on openssl.
87b741b 
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

Co-authored-by: Leonardo Grasso <me@leonardograsso.com>
@FedeDP
fix(build): properly share OPENSSL_INCLUDE_DIR and OPENSSL_LIBRARIES …
35a2a64 
…vars to civetweb cmake.

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
@FedeDP
update(build): dropped civetweb patch. Use different ExternalProject_…
283e888 
…Add when building with bundled openssl or not, to avoid depending on an unexhistent target.

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
@FedeDP
fix(build): properly use correct lib/lib64 folder for CIVETWEB_LIB va…
b331d78 
…riables.

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
@FedeDP
update(build): updated libs.
67ebcb6 
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
@FedeDP
update(build): switched back to falcosecurity libs on master.
a8e54e7 
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
@FedeDP
Copy link
Contributor Author

FedeDP commented Nov 17, 2021

Rebased on falco master + updated libs to use falcosecurity libs master.

@leogr
Copy link
Member

leogr commented Nov 17, 2021

/milestone 0.31.0

@poiana poiana added this to the 0.31.0 milestone Nov 17, 2021
@FedeDP FedeDP changed the title wip: update Civetweb 1.15 to support openssl 1.1 update Civetweb 1.15 to support openssl 1.1 Nov 17, 2021
leogr
leogr approved these changes Nov 17, 2021
View reviewed changes
Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

🥳

LGTM

/approve

@poiana poiana added the lgtm label Nov 17, 2021
@poiana
Copy link

poiana commented Nov 17, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, leogr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@poiana
Copy link

poiana commented Nov 17, 2021

LGTM label has been added.

Git tree hash: 4c12d1f30afa26758bd798622c8f619237780657

@FedeDP FedeDP mentioned this pull request Nov 17, 2021
Merged
@poiana poiana merged commit d9aff8d into falcosecurity:master Nov 17, 2021
@FedeDP FedeDP deleted the civetweb_1.15_openssl_1.1 branch November 17, 2021 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leogr leogr leogr approved these changes

@mstemm mstemm mstemm approved these changes

@leodido leodido Awaiting requested review from leodido

Assignees

@leogr leogr

@mstemm mstemm

Labels
approved area/build dco-signoff: yes kind/feature lgtm release-note size/M
Projects
None yet
Milestone
0.31.0
Development

Successfully merging this pull request may close these issues.

Falco webserver with SSL error: null context when constructing CivetServer
5 participants
@FedeDP @LucaGuerra @leogr @poiana @mstemm