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

libwebsocket: fix compile with mbedtls and update to 4.3.3 #24071

Merged
merged 2 commits into from May 15, 2024

Conversation

orangepizza
Copy link
Contributor

upstreamed patches was removed in process, and add dependency for MBEDTLS_VERSION_C as this package needs that to compile when used mbedtls as backend. (not sure how it worked before?)
mbedtls 3.6 moved one of function used in this into private header, patch redefined it to be able to compiled. a more detailed fix will be needed form upstream, but it should be enough for us.

Signed-off-by: Seo Suchan <tjtncks@gmail.com>
@orangepizza orangepizza changed the title libwebsocket: fix compile with mbedtls and update to 4.0.3 libwebsocket: fix compile with mbedtls and update to 4.3.3 May 3, 2024
@dddaniel
Copy link

dddaniel commented May 6, 2024

@orangepizza, besides that the -DDISABLE_WERROR=ON option is broken in the openwrt Makefile.

`@@ -29,7 +29,7 @@ include $(INCLUDE_DIR)/package.mk
include $(INCLUDE_DIR)/cmake.mk

CMAKE_OPTIONS += -DLWS_IPV6=$(if $(CONFIG_IPV6),ON,OFF)
-CMAKE_OPTIONS += -DISABLE_WERROR=ON
+CMAKE_OPTIONS += -DDISABLE_WERROR=ON
`

@orangepizza
Copy link
Contributor Author

It'd needed patch anyway because it has silent dependent on mbedtls_version_c but I think I can patch it to use mecro string instead, removing that

@dddaniel
Copy link

dddaniel commented May 6, 2024

-DDISABLE_WERROR would just be short term compile fix anyway.
Those implicit declaration errors will tun into linker errors when building applications that use libwebsockets.

touch /home/daniel/deboer/openwrt/build_dir/target-x86_64_musl/libwebsockets-mbedtls/libwebsockets-4.3.3/.configured_68b329da9893e34099c7d8ad5cb9c940 rm -f /home/daniel/deboer/openwrt/build_dir/target-x86_64_musl/libwebsockets-mbedtls/libwebsockets-4.3.3/.built touch /home/daniel/deboer/openwrt/build_dir/target-x86_64_musl/libwebsockets-mbedtls/libwebsockets-4.3.3/.built_check MAKEFLAGS="" /home/daniel/deboer/openwrt/staging_dir/host/bin/ninja -j1 -C /home/daniel/deboer/openwrt/build_dir/target-x86_64_musl/libwebsockets-mbedtls/libwebsockets-4.3.3 ninja: Entering directory /home/daniel/deboer/openwrt/build_dir/target-x86_64_musl/libwebsockets-mbedtls/libwebsockets-4.3.3'
[23/190] Building C object lib/CMakeFiles/websockets.dir/tls/mbedtls/mbedtls-extensions.c.o
/home/daniel/deboer/openwrt/build_dir/target-x86_64_musl/libwebsockets-mbedtls/libwebsockets-4.3.3/lib/tls/mbedtls/mbedtls-extensions.c: In function 'lws_mbedtls_x509_parse_general_name':
/home/daniel/deboer/openwrt/build_dir/target-x86_64_musl/libwebsockets-mbedtls/libwebsockets-4.3.3/lib/tls/mbedtls/mbedtls-extensions.c:253:23: warning: implicit declaration of function 'mbedtls_x509_get_name'; did you mean 'mbedtls_pk_get_name'? [-Wimplicit-function-declaration]
253 | ret = mbedtls_x509_get_name( p, end, &rfc822Name );
| ^~~~~~~~~~~~~~~~~~~~~
| mbedtls_pk_get_name
[33/190] Building C object lib/CMakeFiles/websockets.dir/core/context.c.o
/home/daniel/deboer/openwrt/build_dir/target-x86_64_musl/libwebsockets-mbedtls/libwebsockets-4.3.3/lib/core/context.c: In function 'lws_create_context':
/home/daniel/deboer/openwrt/build_dir/target-x86_64_musl/libwebsockets-mbedtls/libwebsockets-4.3.3/lib/core/context.c:791:9: warning: implicit declaration of function 'mbedtls_version_get_string' [-Wimplicit-function-declaration]
791 | mbedtls_version_get_string(mbedtls_version);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
[116/190] Building C object lib/CMakeFiles/websockets_shared.dir/tls/mbedtls/mbedtls-extensions.c.o
/home/daniel/deboer/openwrt/build_dir/target-x86_64_musl/libwebsockets-mbedtls/libwebsockets-4.3.3/lib/tls/mbedtls/mbedtls-extensions.c: In function 'lws_mbedtls_x509_parse_general_name':
/home/daniel/deboer/openwrt/build_dir/target-x86_64_musl/libwebsockets-mbedtls/libwebsockets-4.3.3/lib/tls/mbedtls/mbedtls-extensions.c:253:23: warning: implicit declaration of function 'mbedtls_x509_get_name'; did you mean 'mbedtls_pk_get_name'? [-Wimplicit-function-declaration]
253 | ret = mbedtls_x509_get_name( p, end, &rfc822Name );
| ^~~~~~~~~~~~~~~~~~~~~
| mbedtls_pk_get_name
[126/190] Building C object lib/CMakeFiles/websockets_shared.dir/core/context.c.o
/home/daniel/deboer/openwrt/build_dir/target-x86_64_musl/libwebsockets-mbedtls/libwebsockets-4.3.3/lib/core/context.c: In function 'lws_create_context':
/home/daniel/deboer/openwrt/build_dir/target-x86_64_musl/libwebsockets-mbedtls/libwebsockets-4.3.3/lib/core/context.c:791:9: warning: implicit declaration of function 'mbedtls_version_get_string' [-Wimplicit-function-declaration]
791 | mbedtls_version_get_string(mbedtls_version);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
[190/190] Linking C shared library lib/libwebsockets-evlib_uloop.so
`

Looks like 4.3.3 is broken with recent mbedtls, and so is 4.3.2. Must be fixed upstream.

@dddaniel
Copy link

dddaniel commented May 7, 2024

@orangepizza, This is working for me: master...dddaniel:packages:master

@orangepizza
Copy link
Contributor Author

this commit already fixed reason it caused error: not really needed to disable werror
this got merged upstream, but upstream doesn't look like it'd have frequent patch releases
warmcat/libwebsockets@1e18a65

@dddaniel
Copy link

dddaniel commented May 7, 2024

this commit already fixed reason it caused error: not really needed to disable werror this got merged upstream, but upstream doesn't look like it'd have frequent patch releases warmcat/libwebsockets@1e18a65

Do programs that link against libwebsockets compile successfully with your commit ?
I had issues with the upstream main branch of libwebsockets and your backported commit.
Libwebsockets itself compiles, but linking against it is giving me this:

/usr/bin/ld: /usr/local/lib/libwebsockets.so: undefined reference to `private_mbedtls_x509_get_name'

orangepizza referenced this pull request in warmcat/libwebsockets May 7, 2024
they moved mbedtls_x509_get_name into interal zone.

Signed-off-by: Seo Suchan <tjtncks@gmail.com>
@orangepizza
Copy link
Contributor Author

reverted to my original fix, this surely works with something linked with it complied

@neheb
Copy link
Contributor

neheb commented May 8, 2024

Squash these commits. Also, don't use .diff files.

@orangepizza
Copy link
Contributor Author

did that, but doesn't make refresh strips any patch headers, making it a diff?

@neheb
Copy link
Contributor

neheb commented May 10, 2024

quilt does not strip patch headers.

@neheb
Copy link
Contributor

neheb commented May 10, 2024

Run

‘’’
make package/libwebsockets/{clean,refresh}
‘’’

3.6 removed mbedtls_x509_get_cert into private header, redefined it in resonable place to temperatly fix it, and make it not depend on mbedtls_version_C. everything is upstreamed so won't need when upstrea release 4.3.4

Signed-off-by: Seo Suchan <tjtncks@gmail.com>
@@ -61,7 +61,7 @@ endef
define Package/libwebsockets-mbedtls
$(call Package/$(PKG_NAME)/Default)
TITLE += (mbedTLS)
DEPENDS += +libmbedtls
DEPENDS += +libmbedtls @MBEDTLS_VERSION_C
Copy link
Member

Choose a reason for hiding this comment

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

We should probably activate this option in the main repository by default. Could you create a PR for that too please.

@neheb neheb merged commit feb64a0 into openwrt:master May 15, 2024
12 checks passed
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 this pull request may close these issues.

None yet

4 participants