CKA_ALLOWED_MECHANISMS: Encoded as hex buffer instead of sequence in certain C code paths

[vjardin]

From emitter.c the snprintf("%lu") will have a different behavior if it is run using 32 bits or 64 bits. So, the content of the SQL file will be different. It means, for instance, if the content of the database is build/populated using a 32 compiled version of tpm2-pkcs11, then another set of applications running 64 bits on the same system won't be able to use properly the contents.

It seems to be due to the 3 following print formats:

85:        snprintf(strkey, sizeof(strkey), "%lu", k);
128:                snprintf(keyvaluebuf, sizeof(keyvaluebuf), "%lu", *v);
176:                snprintf(keyvalue, sizeof(keyvalue), "%lu", v);

what if we replace them with snprintf("xxx %"PRIu64" xxx") ? so the encoding would remain consistent on any execution environment ?

[vjardin]

notes:

src/pkcs11.h:#define CKA_ALLOWED_MECHANISMS     (CKF_ARRAY_ATTRIBUTE | 0x600UL)

it means:

CKA_ALLOWED_MECHANISMS=0x40000600

so, doing select id, tokid, attrs from tobjects where id = 1; leads to the following attrs:

---
!!map {
   ...
  ? !!int "1073743360"  -> 0x40000600
  : !!str "030000000900000001000000060000004000000041000000420000000d0000000e000000430000004400000045000000",
   ...
}

Per http://docs.oasis-open.org/pkcs11/pkcs11-base/v2.40/os/pkcs11-base-v2.40-os.html, the str is an array of:

typedef CK_ULONG CK_MECHANISM_TYPE

which is an array of 12 entries with:

1 03000000
2 09000000
3 01000000
4 06000000
5 40000000
6 41000000
7 42000000
8 0d000000
9 0e000000
0 43000000
1 44000000
2 45000000

but they are 32 bits entries :(

In face, the issue is that any arrays of attributes are converted using the handler TYPE_BYTE_INT_SEQ

ADD_ATTR_HANDLER(CKA_ALLOWED_MECHANISMS, TYPE_BYTE_INT_SEQ),

which is the only case of attrs.c

@williamcroberts > I'd like to read your opinion about a possible fix ? What if we enforce that all writes() and reads() of TYPE_BYTE_INT_SEQ should always be a 32 bits integer, so on 64 bit systems, the array of mechanism remains the same ones ?

But then, if we do so, how to management the compatibilities of 64 bit systems ?

The other way around: enforce 64 bits -> it would break the 32 bits systems.

Another option: how to detect during runtime that the array of mechanism was written 32 bits or 64 bits so we can run back to 32 bits (if it was 64 bits) or run back to 64 bits (if it was 32 bits) ?

[vjardin]

Following some deeper analysis, it is even more weird, there are 2 sets of data into the sql files for the CKA_ALLOWED_MECHANISMS:

bad on 32 bits/64 bits:

  ? !!int "1073743360"
  : !!str "030000000900000001000000060000004000000041000000420000000d0000000e000000430000004400000045000000",

good:

   ? !!int "1073743360"
  : !!seq [
    !!int "3",
    !!int "9",
    !!int "1",
    !!int "64",
    !!int "65",
    !!int "66",
    !!int "13",
    !!int "14",
    !!int "67",
    !!int "68",
  ],

I do not understand how the bad case entries have been added into the sqlite file !!!

So, maybe the previous statement of trying to convert 32/64 bits is not the problem. The root case, is how/why was a string used for 40000600 / 1073743360 / CKA_ALLOWED_MECHANISMS

[vjardin]

We can notice that the base cases do match this list from https://github.com/tpm2-software/tpm2-pkcs11/blob/edda4765435b7769ab460dcfd455c4aed0170597/src/lib/attrs.c#L874

CK_RV rsa_gen_mechs(attr_list *new_pub_attrs, attr_list *new_priv_attrs) {

    /* XXX These are hardcoded for now */
    CK_MECHANISM_TYPE t[] = {
        CKM_RSA_X_509,
        CKM_RSA_PKCS_OAEP,
        CKM_RSA_PKCS,
        CKM_SHA1_RSA_PKCS,
        CKM_SHA256_RSA_PKCS,
        CKM_SHA384_RSA_PKCS,
        CKM_SHA512_RSA_PKCS,
        CKM_RSA_PKCS_PSS,
        CKM_SHA1_RSA_PKCS_PSS,
        CKM_SHA256_RSA_PKCS_PSS,
        CKM_SHA384_RSA_PKCS_PSS,
        CKM_SHA512_RSA_PKCS_PSS,
    };

[vjardin]

@williamcroberts > it seems that the only case when it could occur is when: https://github.com/tpm2-software/tpm2-pkcs11/blob/edda4765435b7769ab460dcfd455c4aed0170597/src/lib/emitter.c#L95 overwrites the TYPE_BYTE_HEX_STR

[vjardin]

rsa_gen_mechs() is calling attr_list_add_buf() which sets the memtype to TYPE_BYTE_HEX_STR instead of TYPE_BYTE_INT_SEQ

[hoehman]

Hello, I am running into a problem with this issue in the context of upgrading an existing database from version 4 to 7 with library version 1.8.0.

I seem to have these similarly malformed entries for allowed mechanisms in my database:

  ? !!int "1073743360"
  : !!str "4110000042100000",

And even a mixed one:

 ? !!int "1073743360"
  : !!seq [
    !!int "4164",
    !!int "4165",
    !!int "4166",
    !!str "1",
    !!str "0",
    !!str "2",
    !!str "4",
  ],

Looks like this issue could be fixed in the latest release (1.9.0, #808) but I am in the unfortunate situation where this malformed entry in the database actually breaks the database upgrade routine when upgrading to version 7.

DB Upgrade failed: "'in <string>' requires string as left operand, not int" 
Traceback (most recent call last): 
  File "/usr/share/tpm2-pkcs11/tpm2_ptool.py", line 6, in <module> 
    tool.main() 
  File "/usr/share/tpm2-pkcs11/tpm2_pkcs11/tpm2_ptool.py", line 26, in main 
    commandlet.init('A tool for manipulating the tpm2-pkcs11 database') 
  File "/usr/share/tpm2-pkcs11/tpm2_pkcs11/command.py", line 102, in init 
    commandlet.get()[d['which']](d) 
  File "/usr/share/tpm2-pkcs11/tpm2_pkcs11/commandlets_keys.py", line 699, in __call__ 
    objects = super(LinkCommand, self).__call__(args) 
  File "/usr/share/tpm2-pkcs11/tpm2_pkcs11/commandlets_keys.py", line 164, in __call__ 
    with Db(path) as db: 
  File "/usr/share/tpm2-pkcs11/tpm2_pkcs11/db.py", line 39, in __enter__ 
    self._create() 
  File "/usr/share/tpm2-pkcs11/tpm2_pkcs11/db.py", line 708, in _create 
    self._do_create() 
  File "/usr/share/tpm2-pkcs11/tpm2_pkcs11/db.py", line 697, in _do_create 
    raise e 
  File "/usr/share/tpm2-pkcs11/tpm2_pkcs11/db.py", line 691, in _do_create 
    self.update_db(old_version, VERSION) 
  File "/usr/share/tpm2-pkcs11/tpm2_pkcs11/db.py", line 545, in update_db 
    getattr(self, '_update_on_{}'.format(x))(dbbakcon) 
  File "/usr/share/tpm2-pkcs11/tpm2_pkcs11/db.py", line 528, in _update_on_7 
    0 in attrs[CKA_ALLOWED_MECHANISMS]: 
TypeError: 'in <string>' requires string as left operand, not int

I tested that it also breaks in the C-side upgrade path, unfortunately. I'm expecting that the database patching in the latest version cannot fix my issue as the failure happens before the upgrade step that would fix it..?

[vjardin]

I don't understand: isn't it fixed if you upgrade directly to the version 1.9 ?

Are you running 32 or 64 bits ?

[hoehman]

Hi @vjardin

Thank you for the response! I just did a test upgrading the library version directly to 1.9.0.

Indeed, the database upgrade from schema version 4 is unsuccessful as it still crashes in _update_on_7 when the fix to that would only happen in the next step, _update_on_8 (Python side).

I'm running a 32-bit system.

[williamcroberts]

I wonder if the simplest work around would be to copy the sqlite db to a 64 bit (as well as the same endianess) machine and run tpm2_ptool dbup --path=path/to/database on it.

[vjardin]

please, @hoehman , can you confirm if @williamcroberts 's tip did solve your problem ?