P11-kit ignores packing and CK_LONG definition on non-windows systems.

[RickyDoug]

The P11-kit does not implement 1-byte packing per the PKCS11 specification, which clearly states this. There are bug reports relating to this as well where we see a mismatch between lengths. At the very least, the packing should be configurable as should be the type of CK_ULONG, CK_LONG, which are assumed to be long int, instead of using stdint and specifying the size, this decision is left up to the compiler do decide how bit CK_ULONG is. Since a long can be 32, 64 or something larger, this is an issue with the implementation used.
It is unfortunate that the same header file that is incorrect is being used across a wide variety of projects, and mis-implementing the PKCS11 API.

[ueno]

Could you be more specific about this, at least link to the bug reports you mentioned? The statement is about how the PKCS#11 specification is designed, so the structures defined there are by nature aligned without any padding, not a guidance for implementation. The pkcs11.h file distributed by p11-kit (common/pkcs11.h) does not include <stdint.h> but defines CK_ULONG to be unsigned long, which is a machine word.

[RickyDoug]

I haven't delved into very many of these reports, but this one is likely related:
#376
Other projects have reports of segfaults on on calling C_Initialize, which is packing related.

But the issues is twofold. Unsigned long can be either 32 bits or 64 bits, and not specifying a size is at the least bad practice.
The second issue is not packing the structures results in an API that does not comply with PKCS11, according to OASIS:

"2.1 Structure packing
Cryptoki structures are packed to occupy as little space as is possible. Cryptoki structures SHALL be
packed with 1-byte alignment", Following along the doc it says the definition of 'SHALL' is in
https://www.ietf.org/rfc/rfc2119.txt, which says :

1. MUST This word, or the terms "REQUIRED" or "SHALL", mean that the definition is an absolute requirement of the specification.

So what this really means is the header file supplied by g10 Code/Andreas Jellinghaus and propagated by Red Hat is absolutely not compliant with the specification on anything other than Windows.

The oasis writers do not appear to know about anything besides windows, so they apparently did not realize or care that GCC also packs structures to 1-byte alignment. It doesn't excuse the fact the it is very simple to add padding to any of these header files, or at the very least make it configurable.

I think the bigger issue is that the pkcs11.h file in this project is used a lot of places, and this is a problem because now we have a fully compatible implementation on Windows but not on anything else.
I can supply a patch that at least makes this a configurable option so maybe someone, somewhere will finally start building compatible APIs.

[ueno]

I haven't delved into very many of these reports, but this one is likely related:
#376
Other projects have reports of segfaults on on calling C_Initialize, which is packing related.

It's about the p11-kit RPC protocol, which has nothing to do with pkcs11.h you are talking here.

[ueno]

Linking to opendnssec/SoftHSMv2#701 to avoid the same discussion spreading across multiple issues. I generally agree with @dengert that this should be first discussed with OASIS. The reference header files have this problem already; for example, CK_INFO is 88 bytes with 8-byte alignment, while it is 76 bytes with 1-byte alignment, not because of the CK_ULONG definition but because the CK_VERSION fields (2 bytes) are padded.

That said, I'm not sure if it can be fixed by simply updating the header file, as many PKCS#11 modules (either open source or proprietary) are built upon those definitions (i.e., the reference header). If some of the applications switch to assuming the packed format, all access to those modules will break (and even if it works, that would be slower).

[dengert]

For cross platform, some network byte order, size of CK_LONG and CK_ULONG and padding, would need to be defined. Besides p11-kit, are there any other applications that send PKCS11 structures over the network? ASN1 could also be used to transmit PKCS11 between platforms.

[RickyDoug]

Those are all valid points, but adding the ability to configure these things to known items would at least allow package managers to choose their poison, which is what I provided for the softhsmv2....It defaults to the long int/unpacked, but it adds the ability to pack and to choose the size of CK_ULONG, CK_LONG. This also exposed some potential issues in the SoftHSMv2 that just assumed CK_ULONG is a unsigned long int. If we can at least propagate configurability, it's at least a step in the right direction.

[RickyDoug]

@dengert byte order is a whole other can of worms. You are, of course right, a full cross platform would include this, but on the same system it's not necessary. Ideally, the PKCS11 API would have a call that allowed one to discover all of this information rather than just taking a plunge and segfaulting when the caller is wrong. And yes, there are other applications that send things over more than just networks, ASN1 is one of those, but conceivably anything that serializes/deserializes could go across any transport. Once you hit byte order, it becomes a really messy endeavor that has no hope of being performant.