interpreter: loading libcrypto in an unsafe way #12214
Comments
|
This error message seems to indicate that an unversioned library filed of libssl was loaded and it want's to be loaded as a versioned file. |
|
The code is public there, but also let me know if there is anything you want me to try out and report back |
|
I think it's the same thing if you use the interpreter with the sample http server. I get the same thing. I think the link flags for openssl changed recently and that broke something. |
|
Is that on an M1? Now that I'm thinking about it, I might started getting that just on M1. There's also cl-plus-ssl/cl-plus-ssl#114 (I didn't ready it yet) |
|
This might also be the fix: cl-plus-ssl/cl-plus-ssl#115 |
|
(but to be honest I'm not sure what's going on... it seems we need to use versioned libcrypto and libssl libraries, fake them because those files don't actually exist... but I don't know how we can tell dlopen the exact path to open, hmmm...) |
I wish, still don’t have one :( This is on an intel |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sure! How do you do that from a |
|
This appears to be working correctly in compiled mode, right? So the system linker probably does something to make this work. We just have to find out how we can emulate that for the interpreter's loader. |
|
With latest bug fixes(not merged), the interpreter runs all |
|
@straight-shoota @beta-ziliani Note that if we launch the interpreter as experimental, particularly on Mac OS, then running an http server with the interpreter doesn't work because of this issue. |
|
This sheds a bit more light on this: https://developer.apple.com/forums/thread/119429 But it's still not conclusive what exactly we're doing wrong and what needs to be done to fix it. I don't have a Mac. Can someone who observes this problem run a couple commands?
|
|
Here you go! |
|
All right, so it looks like the system linker resolved the symlink from Can you try this patch? diff --git i/src/compiler/crystal/loader/unix.cr w/src/compiler/crystal/loader/unix.cr
index fb26f5842..bef0e7433 100644
--- i/src/compiler/crystal/loader/unix.cr
+++ w/src/compiler/crystal/loader/unix.cr
@@ -85,6 +85,9 @@ class Crystal::Loader
end
def load_file?(path : String | ::Path) : Bool
+ if File.symlink?(path)
+ path = File.real_path(path)
+ end
handle = open_library(path.to_s)
return false unless handle |
|
I jus tried it. It didn't work. I added It actually seems it's dying on |
|
What's interesting is that neither of those files ("/usr/lib/libz.dylib" and "/usr/lib/libssl.dylib") exist on my machine. |
|
The explanation is in
so the path is not really taken into account except for local libs (not our case). |
|
Oh, then according to this man page |
|
yes, but that won't solve this problem 😭 |
|
I would expect this to solve exactly this problem. If we give |
|
The problem is that the documentation already suggests that dlopen chops the directory off. I still tried, and indeed it doesn't work. |
|
Well then I guess this becomes the quest to find a spell which convinces MacOS' |
|
Did you look at this? https://github.com/cl-plus-ssl/cl-plus-ssl/pull/115/files It seems linking to some magic numbered versions should do the trick, but I don't know. |
|
Thanks for the tip @cyangle. I am unfortunately not able to get it to work yet. The path you gave and then I checked what the built binary was linking, which was a slightly different path but the same idea |
|
@will I'm using locally built crystal. $ otool -L .build/crystal
.build/crystal:
/usr/local/opt/llvm@14/lib/libLLVM.dylib (compatibility version 1.0.0, current version 14.0.6)
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1300.32.0)
/usr/local/opt/pcre/lib/libpcre.1.dylib (compatibility version 4.0.0, current version 4.13.0)
/usr/local/opt/bdw-gc/lib/libgc.1.dylib (compatibility version 7.0.0, current version 7.1.0)
/usr/local/opt/libevent/lib/libevent-2.1.7.dylib (compatibility version 8.0.0, current version 8.1.0)
/usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
/usr/lib/libffi.dylib (compatibility version 1.0.0, current version 30.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1319.0.0) |
|
From BigSur onwards,I found that changing |
|
OK, I got it working with two changes. Add the lib path of the relevant version of ssl and crypto libs: Added specific version to linker in {% if flag?(:win32) %}
...
{% elsif flag?(:darwin) %}
@[Link("X.3")]
{% else %}
... |
|
I can reproduce the loader error with HEAD version of crystal: $ crystal i src/cli.cr
WARNING: /usr/local/Cellar/crystal/HEAD-fd88012_1/libexec/crystal is loading libcrypto in an unsafe way
Abort trap: 6And I got a different error with the env var: $ env DYLD_LIBRARY_PATH=/usr/local/Cellar/openssl@1.1/1.1.1q/lib crystal i src/cli.cr
In /usr/local/Cellar/crystal/HEAD-fd88012_1/share/crystal/src/log/main.cr:36:21
36 | private Top = Log.for("")
^--
In /usr/local/Cellar/crystal/HEAD-fd88012_1/share/crystal/src/log/main.cr:36:21
36 | private Top = Log.for("")
^--
Error: compiling Log.for("")
.....
Error: compiling self >= arg0
In /usr/local/Cellar/crystal/HEAD-fd88012_1/share/crystal/src/class.cr:74:11
74 | other._lte(self)
^---
In /usr/local/Cellar/crystal/HEAD-fd88012_1/share/crystal/src/class.cr:74:11
74 | other._lte(self)
^---
Error: BUG: missing upcast_distinct from DB::PoolResourceLost(DB::Connection).class to DB::PoolResourceLost(DB::Connection)+.class (Crystal::GenericClassInstanceMetaclassType to Crystal::VirtualMetaclassType) |
|
Yes, by "got it working" I mean just the library issue. But I also got these other (unrelated) issues. |
|
In order to fix it for real, we need to somehow get the version number and plug it in the name of the library. This line gets me what I need in bash, but I don't know how to plug this into the ldflags: outputs |
|
@beta-ziliani What's |
|
Oops! updated. What that scripts attempts to do: store the output of |
|
So basically it's just |
|
hehe yes! |
|
|
|
It's great to know how this can be made to work. This is unnecessary for native codegen because the linker on MacOS handles resolving the basic lib argument to the versioned instance internally. We should probably implement the same functionality in our loader implementation. The goal is that programs should work the same way whether executed natively or in the interpreter. We can implement the workaround as a quick fix, but we should eventually fix it in the loader. But I guess for that we'd probably need some more information on how it actually works inside the linker, which is hard to judge from just a single trial-and-error obtained workaround that achieves (probably) the same result. |
|
Could |
|
The loader is indeed missing processing for the |
|
@HertzDevil @straight-shoota Thank you! I just tried it and I still get the same error. Was this fixed in Windows, or did you try it on a Mac? |
|
This was tested on an M2, non-macOS systems should not have the abort stub |
|
This is what I get:
How can I debug what's wrong in my machine? Maybe I have a path override somewhere that makes it not work? |
|
This patch should print diagnostics for the load order: --- i/src/compiler/crystal/loader.cr
+++ w/src/compiler/crystal/loader.cr
@@ -85,11 +85,14 @@ class Crystal::Loader
def load_library?(libname : String) : Bool
if ::Path::SEPARATORS.any? { |separator| libname.includes?(separator) }
+ STDERR.puts "Loading file #{libname}"
return load_file?(::Path[libname].expand)
end
+ STDERR.puts "Loading lib #{libname}"
@search_paths.each do |directory|
library_path = File.join(directory, Loader.library_filename(libname))
+ STDERR.puts " Trying #{library_path}"
return true if load_file?(library_path)
end
diff --git i/src/compiler/crystal/loader/unix.cr w/src/compiler/crystal/loader/unix.cr
index fb26f5842..7793a0c26 100644
--- i/src/compiler/crystal/loader/unix.cr
+++ w/src/compiler/crystal/loader/unix.cr
@@ -81,6 +81,7 @@ class Crystal::Loader
end
def load_file(path : String | ::Path) : Nil
+ STDERR.puts "Loading file #{path}"
load_file?(path) || raise LoadError.new_dl_error "cannot load #{path}"
end
|
|
What do these things look like?
|
|
@straight-shoota Thanks! Here's the output I got: Here's the value of
I'm not sure where is that in the code.
It's |
On https://github.com/crunchydata/bridge-cli on cafe7b977599bff94035d870129daf40912a1851 and
Normal crystal builds and runs the main file fine
but
crystal icomplainsI tried to grab a backtrace if that's at all helpful using
CRYSTAL_PATH="lib:/Users/will/code/crystal/src" CRYSTAL_CONFIG_LIBRARY_PATH="/usr/local/lib" CRYSTAL_LIBRARY_PATH="/usr/local/lib" lldb ../crystal/.build/crystal i src/cli.crThe text was updated successfully, but these errors were encountered: