Support Unix and Windows #98
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
corhere
force-pushed
the
windows-support
branch
5 times, most recently
from
c468784 to
2dd8bc9
Compare
|
Apologies for the force-push spam; it took me a few tries to get the GitHub Actions workflow in a good state. (I never did discover where on the macOS runner image libcrypto.1.1.dylib is hiding...) This PR should be ready for review now. |
qmuntal
approved these changes
Aug 8, 2023
corhere
force-pushed
the
windows-support
branch
from
2dd8bc9 to
f2707ce
Compare
dagood
reviewed
Aug 9, 2023
corhere
force-pushed
the
windows-support
branch
from
f2707ce to
b779902
Compare
derekparker
requested changes
Aug 16, 2023
derekparker
approved these changes
Aug 16, 2023
dagood
approved these changes
Aug 16, 2023
qmuntal
reviewed
Aug 25, 2023
ueno
approved these changes
Aug 25, 2023
qmuntal
requested changes
Aug 25, 2023
There was a problem hiding this comment.
Some files have been added since this PR was posted, i.e. https://github.com/golang-fips/openssl/blob/v2/tls1prf.go and https://github.com/golang-fips/openssl/blob/v2/tls1prf_test.go. Please update their build constraints too.
The Linux limitation is somewhat artificial. The only platform-specific parts of the module are dynamic linking and loading of the OpenSSL library, and the OpenSSL 1.0.2 threading callbacks. The implementations of both are already portable to any Unix-like OS as dlfcn.h and pthreads are part of the POSIX standard. The only issue with the dynamic loader implementation is that it assumes the naming convention for the library shared object is "libcrypto.so.<version>" which limits support for darwin and other platforms which follow a different convention. Change openssl.Init() and openssl.CheckVersion() to take the library file name as its argument to make the bindings agnostic to the target platform's library file naming convention. Widen the build constraints to allow the module with the dlopen()-based loader to be built on any OS which satisfies the "unix" Go build constraint. Windows's dynamic library loader API is shaped very similarly to POSIX's, aside from the function names. Add support for Windows by adding Windows implementations for linking and loading the OpenSSL library, and the threading callbacks. Signed-off-by: Cory Snider <csnider@mirantis.com>
The functions bn_lebin2bn, bn_bn2lebinpad and bn_bn2binpad are not exported on Windows builds of OpenSSL 1.0.2. Emulate their behaviour to support linking against OpenSSL 1.0.2 on Windows. Signed-off-by: Cory Snider <csnider@mirantis.com>
In some cases, particularly on Windows [1], the OpenSSL library may have been linked against a different C runtime library than the rest of the program. When this is the case, OpenSSL allocates and frees objects on a different heap than the C heap which cgo and statically-linked C code allocates on. This is not an issue with objects which are allocated and freed through typed constructors and destructors exported from OpenSSL, nor with objects for which we explicitly allocate and free, as those are consistently freed back to the same heap they were allocated from. But there are a couple of situations where OpenSSL takes ownership of an object we allocate, and we take ownership of an object OpenSSL allocates. Freeing an object onto a different heap than it was allocated from will corrupt one heap and leak memory from the other. Use the OpenSSL library functions to allocate and free those objects, respectively, so as not to corrupt the heaps. [1]: https://learn.microsoft.com/en-us/cpp/c-runtime-library/potential-errors-passing-crt-objects-across-dll-boundaries?view=msvc-170 Signed-off-by: Cory Snider <csnider@mirantis.com>
Run the test suite using the OpenSSL versions preinstalled on the respective GHA runner images. Signed-off-by: Cory Snider <csnider@mirantis.com>
On Apple clang, at least, assigning a string literal to a const char *
variable elicits a warning.
./goopenssl.h:119:25: warning: assigning to 'const unsigned char *'
from 'char[1]' converts between pointers to integer types where one
is of the unique plain 'char' type and the other is not
[-Wpointer-sign]
Silence the warnings by casting the offending string literals to
unsigned.
Signed-off-by: Cory Snider <csnider@mirantis.com>
corhere
force-pushed
the
windows-support
branch
from
6ffa40b to
f15d212
Compare
qmuntal
approved these changes
Aug 25, 2023
|
Merging. Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add support for any target which satisfies the build constraint
cgo && (windows || unix).The Linux limitation is somewhat artificial. The only platform-specific parts of the module are dynamic linking and loading of the OpenSSL library, and the OpenSSL 1.0.2 threading callbacks. The implementations of both are already portable to any Unix-like OS as
dlfcn.hand pthreads are part of the POSIX standard. The only issue with the dynamic loader implementation is that it assumes the naming convention for the library shared object is"libcrypto.so.<version>"which limits support for darwin and other platforms which follow a different convention. Changeopenssl.Init()andopenssl.CheckVersion()to take the library file name as its argument to make the bindings agnostic to the target platform's library file naming convention. Widen the build constraints to allow the module with thedlopen()-based loader to be built on any OS which satisfies the "unix" Go build constraint.Windows's dynamic library loader API is shaped very similarly to POSIX's, aside from the function names. Add support for Windows by adding Windows implementations for linking and loading the OpenSSL library, and the threading callbacks.
Tested on Windows Server 2022 with OpenSSL 1.0.2zh-fips (Safelogic Cryptocomply), and macOS Ventura 13.4 (x86_64) with Homebrew OpenSSL 1.1.1 and OpenSSL 3.0.8.