search

softhsm / SoftHSMv2

SoftHSM version 2
http://www.softhsm.org/
Other
790 stars 344 forks source link

Table 10: C_GenerateKey attributes #473

Open johughes99 opened 5 years ago

johughes99 commented 5 years ago

I'm just going through my Table 10 tests for Note 4 - ie the attributes that must NOT be set when generating a key.

I find that I can set the following attributes when calling C_GenerateKey for AES:

unless I'm mis-reading the base and mechanisms documents - it shouldn't be possible to set these

rijswijk commented 5 years ago

If specified, the attributes will be overwritten by SoftHSM (see SoftHSM.cpp from around line 7158); does the spec provide any guidance on what an implementation must/should do if these attributes are specified? (it certainly doesn't make any sense to specify them, they are indeed read-only).

johughes99 commented 5 years ago

The base and current mechanism specs are pretty clear on this. The base of it all is stated in Table 10 in section 4.2 of the base specification. The table is entitled "Common footnotes for object attribute tables". Most tables in both specifications you will note that each attribute has one or more superscripts. This denotes the note/rule number in table 10. So if you look as an example - table 11 for CKA_CLASS you will notice that it was a superscript of 1. This refers to note 1 in table 10 - in that CKA_CLASS must be specified when an object is created with C_CreateObject. If you look through all the tables on attributes in both specifications you will find many references to the table 10 notes

Sometime ago I produced a spreadsheet that went through most object types and recorded which of the notes applied to each of the attributes for each of the object types. It took quite a lot of work to produce - but it does mean I can easily look up the relationship between object types, attributes and table 10 notes.

Two other things to note:

  1. so far I've only really tested out notes 3 and 4 - I hope to do others in the coming weeks
  2. SoftHSMv2 is fine with C_GenerateKeyPair. You are compliant with the spec - its just C_GenerateKey were I believe there is an issue.

Hope that helps

rijswijk commented 5 years ago

What I meant to ask was: does the spec provide any guidance in your opinion 1) if an error should be returned if these attributes are specified in the template, and if so 2) what error to return. Should it be CKR_TEMPLATE_INCONSISTENT?

johughes99 commented 5 years ago

Sorry I mis-understood. I don't believe there is fixed guidance on this - although section 4.1 and 5.1.8 has something to say. Rules could be something like:

CKR_TEMPLATE_INCOMPLETE if an mandatory is missing (e.g for notes 1,3,5 etc) CKR_ATTRIBUTE_TYPE_INVALID if its an invalid attribute (ie doesn't exist) CKR_ATTRIBUTE_VALUE_INVALID if its an invalid valid for a valid attribute type CKR_ATTRIBUTE_READ_ONLY if its a read-only attribute if you are trying to set it Anything else then perhaps return CKR_TEMPLATE_INCONSISTENT

In my testing I in general look for CKR_ATTRIBUTE_READ_ONLY, CKR_TEMPLATE_INCONSISTENT or CKR_TEMPLATE_INCOMPLETE recognising there are no exact rules on what to return.

rijswijk commented 5 years ago

So the current behaviour is this:

For the case you describe when you raised this issue, SoftHSM will overwrite anything that is not specified by the user, but instead by the token. This is due to how SoftHSM "builds" objects when generating keys: it internally relies on C_CreateObject logic, and pre-sets or overwrites certain attributes.

Did you observe any erroneous behaviour (i.e. where the values you set in the template were used over whatever the token should set)? If so, could you describe this?

I'm a bit on the fence over whether or not this behaviour should be changed. It is clearly wrong to specify the attributes you mentioned during key generation, but there doesn't seem to be any harm in not throwing an error in this case, since the values get ignored anyway, and there appears to be no inherent unsafe behaviour. Could you maybe think about this a bit and let us know your thoughts? What would be good reasons to change this behaviour?

johughes99 commented 5 years ago

I will look and think about this a bit more and get back to you next week. It just strikes me odd that SoftHSM correctly validates the attributes for C_GenerateKeyPair - but not for this call.

On testing C_CreateObject for note 1 in Table 10 I have noticed that SoftHSM doesn't check for the presence of the mandatory attributes CKA_EC_POINT or CKA_EC_PARAMS

johughes99 commented 5 years ago

In reviewing my code again it does seem SoftHSM does check for the presence of the mandatory attributes CKA_EC_POINT or CKA_EC_PARAMS when calling C_CreateObject.

So no problem there ..

johughes99 commented 5 years ago

I just thought I would let you know I've implemented a test looking at RSA attributes you must NOT specify in C_GenerateKey - e.g. CKA_PUBLIC_EXPONENT. In all there are 14 attributes. I have tested all of them. SoftHSM passes the test fine.

rijswijk commented 5 years ago

Thanks for the update, good to know SoftHSM passes the test.