-
Notifications
You must be signed in to change notification settings - Fork 96
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
P11-kit ignores packing and CK_LONG definition on non-windows systems. #488
Comments
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 ( |
I haven't delved into very many of these reports, but this one is likely 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. "2.1 Structure packing
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. |
It's about the p11-kit RPC protocol, which has nothing to do with pkcs11.h you are talking here. |
Linking to softhsm/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, 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). |
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. |
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. |
@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. |
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.
The text was updated successfully, but these errors were encountered: