-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Build fails with OpenSSL 1.1 #757
Comments
@neilstuartcraig Right, we haven't tried that yet ourselves. Thanks for the report. Patches welcome! :) |
Sorry for the slow reply @agentzh. I'd love to help but my C was never great and has got worse in the 15 years since I really did any :-). Can I perhaps do some digging to help find the issue? Also, do you have a rough idea when the module is likely to compile against openssl 1.1 - i.e. how high it is on your priority scale. I believe openssl 1.1 is going to be stable this month and should therefore be shipped with OS's and updates within a few weeks. Cheers |
All those errors are due to the fact that the SSL struct in OpenSSL 1.1 is going to be opaque. The The other ones I'm not sure how to fix. A new API is probably required to get/set those fields. |
Thanks @ghedo - i see the tests passed on your PR. |
So, the other problem is that the OCSP stapling support in ngx_lua is kind of hacky. I made openssl/openssl#1054 but it's not enough to make this work and I think we might want to find a better solution (e.g. instead of calling There are also other changes needed to make this build with OpenSSL master, I'll open a PR for that soon. I also have a patch to make NGINX build with OpenSSL master (but I haven't sent it to nginx-devel yet). |
@neilstuartcraig could you post the NGINX build errors you got with OpenSSL 1.1.0-pre5? (just to confirm the errors I fixed are the same). |
Actually, I don't think we ven need the |
Not tested though... |
@ghedo - thanks so much for picking this up and doing it so quickly, we very much appreciate it. The build errors I see (RPM build via spec file on CentOS7) are:
Is that enough info? ICIH, openssl-pre4 builds just fine for nginx (without the openresty lua module). Let me know if I can be useful in any way - testing, raising tickets, getting debug info etc. and thanks again! BTW, not sure if you know but we're using openresty in the A/V chain for BBC online now - there are a couple of blog posts e.g. this one and this one (for 'nginx' read 'openresty') - not my team (thought they're about 15 metres away from me) but i'll extend my thanks and appreciation from them too! |
Yes, those errors are the same I see. The NGINX developers said they are going to remove that whole DH-related code in 1.11.0, so it should get fixed in a few days (so my patch that fixed that code was rejected). |
Excellent! Thanks for the update @ghedo |
I opened #761 to fix some of the build errors, but since changes to OpenSSL itself are also required, it doesn't fix all the errors yet. |
Thanks for the update @ghedo :-) |
OpenSSL 1.1.0 final is out :) |
wOOt! Hopefully no repeat work needed then :-) Sent from my iPhone
|
Since v0.10.6 (building as a dyn module for 1.11.4) I am getting the following failure when building against openssl 1.1.0:
|
any progress on openssl 1.1.0+ support ? only thing missing for me to switch from libressl 2.4/openssl 1.0.2j to openssl 1.1.0 |
@centminmod OpenSSL 1.1 has both big architectural changes and also some API changes. So it will take a relatively long time on our side to port and test. |
@agentzh ok understood :) |
Hi @agentzh Cheers :-) |
@neilstuartcraig OpenSSL 1.1.0 has many more quirks than we originally expected. You can watch and help in #761 and #922. |
@neilstuartcraig BTW, our current focus is on OpenSSL 1.1.0 while the closest OpenSSL release that may get TLS 1.3 looks like to be 1.1.1, which is a different series than 1.1.0, so it may have its own even newer incompatibles and quirks :) |
Thanks @agentzh - i'll check them out, i am not sure i can help much in development as my C is rustier than anyone could imagine and wasn't good to start with! Very happy to help testing and/or with docs etc. though. i'll subscribe to the issues and keep track as best i can. |
hi @agentzh ,I use version is openresty-1.11.2.2 , openssl 1.0.2 build still is error :
|
@challengeof OpenSSL 1.0.2 is proven to work for years. The error may be an indicator you may not really use the correct versions of header files of OpenSSL. You should ensure that you are indeed using OpenSSL 1.0.2's headers. |
Hi again JFYI (apologies if you know this) but Debian 9 will be shipping very soon and comes with 1.1. Hope that helps. Any news would be gratefully received (i wish i could contribute code) :-) |
@thehowl Mind to provide the C and Lua backtraces for that error? It should be an easy fix. |
Sure, what's needed to do that? An environment flag? Running openresty by itself doesn't print any stacktrace, btw, it only prints the error message I posted, and quits. Also, do you want me to create a separate issue? @spacewander might give it a shot later, thanks! |
@thehowl You just need debug symbols and gdb for that. If you compile openresty from source then you already have debug symbols by default (unless you strip the binaries). And in gdb, you can use https://github.com/openresty/openresty-gdb-utils#lbt Just do the core dump or process inspection in gdb when the worker process crashes. |
After a lot of trying to understand how to get the backtrace because I never barely even touched gdb before, I managed to get a C backtrace (using a catchpoint on exit)
Trying to use lbt gives this
|
@thehowl Please use the gdb command |
@thehowl Also, will you re-compile your openresty with the |
@thehowl BTW, your crash is due to LuaJIT's GC64 mode's limitations on lightuserdata value ranges. It has nothing to do with OpenSSL at any rate. |
@agentzh Yep, I was aware that OpenSSL was unrelated (guess I should have pointed that out!). What I said was mostly a tip for googlers finding this thread in hope to managing to compile from source (either for leisure or, like me, necessity), pointing out that even after managing to compile with openssl, the program wouldn't work because of the issue I pointed out. |
@thehowl Then you shouldn't hijack this github issue (the title is "Build fails with OpenSSL 1.1"). |
@thehowl Anyway, will you try the following
|
@thehowl Next time please create a separate github issue to avoid hijacking existing unrelated issues. Thank you for your cooperation. |
@thehowl Seems like on ARM64, the C global variables are stored in the high address space like stacks (but we still need the output of the gdb command In that case, the following patch should fix the problem: https://github.com/openresty/lua-nginx-module/pull/1151/files Will you try it out on your side? It should be something like this:
Thanks for the report! |
@thehowl BTW, it's recommended to build OpenSSL as a shared library otherwise the linker would strip away OpenSSL symbols that are not directly referenced in the nginx binary, which may cause errors when you try to use dynamic Lua modules referencing new symbols in OpenSSL. |
Sorry for the late reply, and I understand I should have made a new issue (as I've even asked you if I should do some replies ago - though I'm at fault for not properly pointing out that the issue was unrelated in my first post, sorry about that!). Either way, getting to the issue at hand. The print on gdb returned the following:
I will now apply the patch you linked and try doing as you said. Thanks for the advice for openssl! |
@neilstuartcraig OK, the C global variables are indeed stored in high address space on ARM64, going beyond the 47-bit address space. So it is confirmed. My patch above should fix the problem on your side. Let me know how it works. |
@thehowl Yep it seems to be a known issue due to LuaJIT's implementation. Attempting to address addresses higher than If you have time, mind sharing the output of:
on your architecture? You can use the NGINX process as the |
@agentzh Still winds up in the same error, but this time with a different line to cause it. As you can see from the backtrace, frame @dndx Sure! I'm not sure if by "nginx" you actually meant openresty or the actual nginx I'm running right now, so just to be sure I included both. https://gist.github.com/thehowl/0b25712130ff0691cfec50e71c66bcef |
I went ahead and replaced the function call specified in the previous comment, and at last it, at the very least, starts listening to 0.0.0.0:80 with the default config and it gives the "Welcome to OpenResty" page. (although |
@thehowl Thanks for the info. Looks like ARM64 Linux indeed maps program image to a relatively high starting address ( Since the entire program image resides inside |
Ah, this is problematic. @thehowl Can you change the starting point of the address mapping? We cannot mask off normal pointers like |
If this is the mapping used for all Arm64 systems, then we could amend for the pointer values in an ad-hoc way. But still, it's very hacky. The easiest way is build OpenResty as 32-bit binaries, as @spacewander suggested earlier. Or we should simply patch LuaJIT for this specific address mapping in this particular system/architecture. |
Let's move this OT discussion to #1152. It's deeper than expected. |
@agentzh - My apologies, i missed your message above. I have just tried the patch above (1151.diff) and it definitely fixes some but not quite all issues. The remaining thing i can see is an error during build of:
This is with:
Any thoughts would be very welcome and I can easily try different builds now as i have a parameterised, automated build. Cheers |
@neilstuartcraig I don't think OpenSSL 1.1.x is supported by this module yet. See #761 and #922. |
Hi
I have a project in which I am doing a custom NGINX build and want to use this module. The build is an RPM build (currently on CentOS 7) and it's failing with NGINX (1.10.0) with OpenSSL 1.1-pre4 (pre5 doesn't compile with NGINX):
It would be great to get OpenSSL 1.1 compatibility (especially as it'll be stable in a few weeks) but I absolutely understand it's a pre-release so it's 100% understandable that the build fails right now. This ticket is for 2 reasons:
Happy to provide any info and/or run tests builds if needed.
Thanks for the great work!
Cheers
The text was updated successfully, but these errors were encountered: