p11-kit-client.dll for Windows #730
Conversation
These were the things I tested (mostly copied from #405): To test the resulting client, I ran p11-kit server on linux (WSL), (though I had to use the official p11-kit-0.25.10.tar.xz release tarball, using ubuntu's p11-kit server produced errors...): Click here to show commands to create virtual Smartcard via softhsm, create cert, start p11-kit server ...In the msys2 mingw64 shell where p11-kit (branch from this PR) was compiled: All three consumers of p11-kit-client.dll seemed to work as expected with the changes from this PR. |
p11-kit/rpc-message.c
Outdated
| { | ||
| CK_ATTRIBUTE_PTR attr; | ||
| CK_ULONG i; | ||
| uint32_t valueLenSerialized; |
There was a problem hiding this comment.
Use snail case instead of camel case: value_len_serialized.
p11-kit/rpc-message.c
Outdated
| /* And the attribute buffer length */ | ||
| p11_rpc_buffer_add_uint32 (msg->output, attr->pValue ? attr->ulValueLen : 0); | ||
| valueLenSerialized = attr->pValue ? attr->ulValueLen : 0; | ||
| /* For ULONG attribures we always serialize the lenght as 8 even if it is not 8 on the current platform */ |
p11-kit/rpc-message.c
Outdated
| valueLenSerialized = attr->pValue ? attr->ulValueLen : 0; | ||
| /* For ULONG attribures we always serialize the lenght as 8 even if it is not 8 on the current platform */ | ||
| if (sizeof(CK_ULONG) != 8 && valueLenSerialized == sizeof(CK_ULONG) && map_attribute_to_value_type(attr->type) == P11_RPC_VALUE_ULONG) | ||
| valueLenSerialized = 8; |
There was a problem hiding this comment.
Maybe good to have a helper function for this? Something like:
static uint32_t
p11_rpc_buffer_add_value_length (p11_rpc_buffer *buffer, p11_rpc_value_type type, CK_ULONG value_length);|
I was trying to use this patched version today at work where I'd actually like to have a working version of p11-kit-client.dll. My real use case is with the opencs pkcs11 module. And contrary to softhsm that one seems to use handle values bigger than 32bit. But as CK_ULONG and therfore CK_OBJECT_HANDLE is only 32 bit on Windows the handle values get truncated and all further calls using the truncated handle values return invalid handle errors (I think it was CKR_OBJECT_HANDLE_INVALID ?). I guess to properly connect from a p11-kit-client on platform with 32bit long to a p11-kit server on a 64bit long platform we'd need to have some kind of dictionary, that gets filled when a 64bit object handle gets deserialized into a 32bit CK_OBJECT_HANDLE and translated back to the correct 64bit value when serializing the value again for another api function call... As a workaround I tried to run a 32 bit version of the p11-kit server on the linux side (as the object handle just looked like pointer values in pkcs11-spy), so my hope was that this would lead to all object handles being small enough. This was the case but lead me to find out that apparently on 32 bit linux they type long is also just 32 bit, so the impact of changing the wireprotocoll for those platforms is bigger then I expected. (When connecting with the windows p11-kit-client (with the patch from this pull request to normalize ULONG attribute serialized size to 8) to the 32bit p11-kit server, no pkcs11 function actually errored out but I guess some values somewhere were overwritten because several api function calls later strange things started to happen in the opencs pkcs11 module...) I built another p11-kit-client.dll version without the ULONG attribute size normalization (basically state of the master branch) and got further with that one. Instead I tried to build a 32 bit version of p11-kit for linux myself using |
8f1706c to
b575e49
Compare
|
Ok, I do have it working now in my "production" environment with an physical smartcard usbstick. The 32bit p11-kit server version I compiled myself does actually work, it just somehow does not work together with a pkcs11-spy in between it and the opensc pkcs11 module. To recap: 64bit p11-kit-client.dll with normalization of ULONG attribute length to 8 works when connecting to a x86_64 p11-kit server (without the normalization) on linux if the pkcs11 module generates CK_OBJECT_HANDLE values that are below 0xffffffff (e.g. softhsm). 64bit p11-kit-client.dll with normalization of ULONG attribute length to 8 does not work when connecting to a i386 p11-kit server (without the normalization) on linux. 64bit p11-kit-client.dll without normalization of ULONG attribute length to 8 does not work when connecting to a x86_64 p11-kit server (without the normalization) on linux. 64bit p11-kit-client.dll without normalization of ULONG attribute length to 8 works when connecting to a i386 p11-kit server (without the normalization) on linux. And not tested but based on my understanding: If the normalization of ULONG attribute length to 8 from this PR were to be added, then a p11-kit-client.so on linux i386 does not work when connecting to a i386 p11-kit server (without the normalization) on linux. Also (for now) not tested and just based on my understanding: All "does not work" would be fixed, if the server side did already include the normalization to 8 from this PR. This is obviously not a great result, especially as this change could break existing workflows, if someone updates either client or server but not the other. My suggestion: We do not normalize the ULONG attribute length when serializing but instead always ignore the sent length when deserializing ULONG attributes (on server and client side, on all platforms, not just where sizeof ULONG is 4). This would mean that no existing workflow breaks when only one side (server or client) is updated to the version with the change. But as soon as both sides are updated this would still allow communication between systems with ULONG size 8 and 4. While client with ULONG size 4 (Windows (x64 or x32) or linux x32) can only connect to a server with ULONG size 8 if it does not use big CK_OBJECT_HANDLE values, connecting from client with ULONG size 8 to a server with ULONG size 4 should in theory always work then. |
In principle there is no need to serialize the length at all in this case, as the only allowed length value here is sizeof(CK_ULONG) anyway. But by not changing the serialization the wire protocoll does not change so no existing useages can break. But as soon as both client and server use a version with this patch, communication between platforms with different sizes of ULONG is possible (e.g. Windows to Linux x64).
b575e49 to
3df81c6
Compare
|
Short update: Though at least this version does (as intended) seem to not break any existing working configuration based on my short tests. I kept the previous version of this PR in this branch for reference. |
This fixes #405.
The change to
p11-kit/client.cis exactly as originally suggested by @spfendtner in #405.The changes in
p11-kit/rpc-message.cchange the serialization of attributes on systems where size of unsigned long is not 8 bytes. As far as I know, this should only be the case for Windows and some uncommon linux variants (32 bit MIPS?).With this change the wireprotocol is always identical and allows to connect with a p11-kit-client.dll to a p11-kit server running on Linux.
The length field for ULONG attributes is now always serialized as value 8.
In principle there is no need to serialize the length at all for ULONG attributes,
as the only allowed length value here is sizeof(CK_ULONG) anyway.
But by always serializing as 8 the wire protocoll does only change on the
few platforms where
sizeof(CK_ULONG)!=8.As there never was a p11-kit-client on Windows, I hope this is ok.
As was commented in the issue,
LDFLAGS+=" -lintl"was required to build on windows, so I addedlibintl_depsto the dependencies of the mock_sources inp11-kit/meson.build, as some of those were failing without.The
shared_module('p11-kit-client',call inp11-kit/meson.buildis the same as before, just two spaces indentation removed, as I completely removed theif host_system != 'windows'condition.