Ensure ECC key attributes are aligned with the PKCS#11 Spec #119
Conversation
|
Addressing one key at a time, first patch addressed CKK_EC Public key, other to come as I implement and add test. |
simo5
force-pushed
the
ecvals
branch
2 times, most recently
from
167b206 to
acce4ca
Compare
simo5
force-pushed
the
ecvals
branch
3 times, most recently
from
cc831cf to
e45d489
Compare
|
This PR is ready for review, however, being spec compliant means a lot of tools will just fail to deal with Edwaurds and Montgomery public curves as they expect a DER encoded OCTET STRING for the public EC_POINT for these curves. However a DER OCTET STRING is incorrect even if we look at the 3.0 spec that specified a DER BIT STRING vs the 3.1 spec which specifies just a raw byte buffer. Fixing all tools to expect both encodings will take a while ... |
|
So the last commit adds a feature to enable 3.1 compatibility and handles 3.0 compatibility as default. I am not entirely sure I like it this way, most applications and most tokens will probably fail to deal with this appropriately anyway, so perhaps we should have a runtime configuration switch rather than a build time feature. A runtime switch will make it easier to deal with. The issue though is where to put this switch. One option is the config file, or we could have both a config file and an environment variable ... what a mess... |
|
Ok I pushed a fixup that completely changes the last commit, to use an env var (or a new config option) to change the behavior at runtime rather than dabbling with build time options. Tested that pkcs11-provider test fail if the env var is not set, but pass fine if the env var set. I will now propose a PR in pkcs11-provider to properly set config so that we do not break its CI when we merge this PR. |
Signed-off-by: Simo Sorce <[email protected]>
Specifically both CKA_EC_PARAMS is checd to contain a valid curve and CKA_EC_POINT is correctly formatted with DER(qx,qy) Signed-off-by: Simo Sorce <[email protected]>
Signed-off-by: Simo Sorce <[email protected]>
The initial implementation of Edwards keys used a definition partially consisent with the language in the 3.0 spec that has since changed in the 3.1 spec. Namely the 3.0 spec indicated that the public CKA_EC_POINT for CKK_EC_EDWARDS curves needed to be encoded as a DER BIT STRING, however the implementation was taking a DER OCTET STRING instead. The 3.1 spec clarifes that the CKA_EC_POINT for Edwards curves is just a buffer of bytes, with no ASN.1 encoding, just like the private CKA_VALUE. This patch adds all the necessary checks on import and aligns the implementation to the 3.1 Spec. Signed-off-by: Simo Sorce <[email protected]>
Signed-off-by: Simo Sorce <[email protected]>
Signed-off-by: Simo Sorce <[email protected]>
Signed-off-by: Simo Sorce <[email protected]>
The PKCS#11 3.0 Spec was released with a bug in the description of the public point encoding for Edwards and Montgomery curves that foooled many implementations to assume these curves would return a DER encoded octet string as the value for CKA_EC_POINT. However an errata was issued and the 3.1 spec also confirms that the CKA_EC_POINT for these curves should be just a byte array (like the private value is). It is easy to accept either form and conver it for storage, the real tricky part is returning the point to applications. Applications that only handle one form and can't deal with different encodings may get an encoding they do not like and fail. Signed-off-by: Simo Sorce <[email protected]>
Signed-off-by: Simo Sorce <[email protected]>
The different ECC key types accept values encoded in different ways.
Add code to ensure values are correctly formed on import and generation according to what the spec prescribes.
Fixes #115