Explicitly set CKA_ID to zero-length byte string on PRIVATE_KEY causes assertion on retrieval

[space88man]

I am getting the following assertion on testing corner cases where CKA_ID is explicity set to
zero-length byte string. It works for public keys and certs, but not for private keys.

/usr/include/c++/8/bits/stl_vector.h:950: std::vector<_Tp, _Alloc>::const_reference std::vector<_Tp, 
_Alloc>::operator[](std::vector<_Tp, _Alloc>::size_type) const [with _Tp = unsigned char; _Alloc = 
SecureAllocator<unsigned char>; std::vector<_Tp, _Alloc>::const_reference = const unsigned char&; 
std::vector<_Tp, _Alloc>::size_type = long unsigned int]: Assertion '__builtin_expect(__n < this->size(), true)' failed.
Aborted (core dumped)

SoftHSM2v2 is 2.5.0 (from Fedora 29 x86_64)

How to reproduce;

1. Create a private key on token (don't use CKA_ID in the template). Observe that retrieving CKA_ID works: returns zero-length byte string.
2. C_SetAttributeValue to set CKA_ID explicitly to zero-length byte string
3. Retrieve CKA_ID to a non-NULL buffer of length 1 (this is what pkcs11-tool would do)
4. Get the above assertion

The first call (with NULL_PTR buffer works)

9: C_GetAttributeValue
2019-03-04 13:26:07.348
[in] hSession = 0x1
[in] hObject = 0x2
[in] pTemplate[1]: 
    CKA_ID                0000000000000000 / 0
[out] pTemplate[1]: 
    CKA_ID                0000000000000000 / 0
Returned:  0 CKR_OK

When you make the second call pointing to a non-NULL buffer, the assertion appears.

[space88man]

Backtrace:

#0  0x00007ffff7dff53f in raise () from /lib64/libc.so.6
#1  0x00007ffff7de9895 in abort () from /lib64/libc.so.6
#2  0x00007ffff7a42638 in std::__replacement_assert (
    __file=__file@entry=0x7ffff7a5fcb8 "/usr/include/c++/8/bits/stl_vector.h", __line=__line@entry=950, 
    __function=__function@entry=0x7ffff7a62d00 <std::vector<unsigned char, SecureAllocator<unsigned char> >::operator[](unsigned long) const::__PRETTY_FUNCTION__> "std::vector<_Tp, _Alloc>::const_reference std::vector<_Tp, _Alloc>::operator[](std::vector<_Tp, _Alloc>::size_type) const [with _Tp = unsigned char; _Alloc = SecureAllocator<unsigned char>; std::vecto"..., __condition=__condition@entry=0x7ffff7a5fc88 "__builtin_expect(__n < this->size(), true)")
    at /usr/include/c++/8/x86_64-redhat-linux/bits/c++config.h:2391
#3  0x00007ffff7a41225 in std::vector<unsigned char, SecureAllocator<unsigned char> >::operator[] (__n=<optimized out>, 
    this=<optimized out>) at ByteString.cpp:194
#4  ByteString::const_byte_str (this=0x7fffffffc750) at ByteString.cpp:196
#5  0x00007ffff7a414f9 in ByteString::const_byte_str (this=this@entry=0x7fffffffc750)
    at /usr/include/c++/8/bits/stl_vector.h:948
#6  0x00007ffff79eab1d in P11Attribute::retrieve (this=<optimized out>, token=token@entry=0x459210, 
    isPrivate=isPrivate@entry=true, pValue=0x7fffffffc9c0, pulValueLen=pulValueLen@entry=0x7fffffffcdd0)
    at P11Attributes.cpp:343
#7  0x00007ffff79eca7a in P11Object::loadTemplate (this=0x445630, token=token@entry=0x459210, 
    pTemplate=pTemplate@entry=0x7fffffffcdc0, ulAttributeCount=ulAttributeCount@entry=1) at P11Objects.cpp:161
#8  0x00007ffff7a1fcce in SoftHSM::C_GetAttributeValue (this=<optimized out>, hSession=<optimized out>, 
    hObject=<optimized out>, pTemplate=0x7fffffffcdc0, ulCount=1) at SoftHSM.cpp:1753
--Type <RET> for more, q to quit, c to continue without paging--
#9  0x00007ffff79de5b3 in C_GetAttributeValue (hSession=1, hObject=2, pTemplate=0x7fffffffcdc0, ulCount=1)
    at main.cpp:497
#10 0x00007ffff7fbdc1d in C_GetAttributeValue (hSession=1, hObject=2, pTemplate=0x7fffffffcdc0, ulCount=1)
    at pkcs11-spy.c:718
#11 0x0000000000401477 in main (argc=2, argv=0x7fffffffcf48) at p11debug.c:68

[space88man]

Reproducer
p11debug.zip

[space88man]

Once in this state, pkcs11-tool can delete the object but not retrieve it. Same for p11tool.
The only way out is to C_SetAttributeValue to a non-zero length byte string.

It does not seem possible to return to the pristine state where the object had an implicit CKA_ID of zero-length byte string.

[halderen]

I've not been able to reproduce this properly, I haven't tried the Fedora packages yet. But some questions regarding the procedure:

• How was the private key created. Was it imported a with softhsm2-uti/p11tool or another way? Or was it created using a C_GenerateKeyPair? Is the public key also in the repository when the call fails?
• In the reproduction path you mention "When you make the second call pointing to a non-NULL buffer, the assertion appears.", here you are referring to the C_SetAttributeValue call right? Otherwise if you mean the second call to C_GetAttributeValue it would contrict itself.

[halderen]

Find attached my test program, if this still also gives an error with you (it doesn't on my system), we have to look at it system-specific.
testissue.zip

[space88man]

1. private key creation (note CKA_ID is not in the template):

pkcs11-tool --module /usr/lib64/pkcs11/libsofthsm2.so -k --key-type EC:prime256v1 -a key-01 
pkcs11-tool --module /usr/lib64/pkcs11/libsofthsm2.so -O #works as expected 

2. I meant the second call to C_GetAtttributeValue (with non-NULL target pointer - you need to run the program twice).

First run - will correctly return the implicit CKA_ID value (zero-len byte string) then set CKA_ID explicitly,

Second run - first call - with NULL target pointer correctly reports the ulValueLen is zero; second call - now set pValueLen to non-NULL and attempt to receive this zero-length data value

3. I am able to reproduce with your program

LD_PRELOAD=/usr/lib64/pkcs11/libsofthsm2.so ./testissue 
logging in
created key pair 2 3
object should be in 0x7fff42db6ad0
destroyed public key 2
object get attribute len=0
object set attribute
/usr/include/c++/8/bits/stl_vector.h:950: std::vector<_Tp, _Alloc>::const_reference std::vector<_Tp, _Alloc>::operator[](std::vector<_Tp, _Alloc>::size_type) const [with _Tp = unsigned char; _Alloc = SecureAllocator<unsigned char>; std::vector<_Tp, _Alloc>::const_reference = const unsigned char&; std::vector<_Tp, _Alloc>::size_type = long unsigned int]: Assertion '__builtin_expect(__n < this->size(), true)' failed.
Aborted (core dumped)

Same backtrace at P11Attribute::retrieve

[halderen]

Identical test program indeed fails under Fedora (that one took a long time to install though....)

[space88man]

@halderen I think it is due to Fedora's packaging use of -Wp,-D_GLIBCXX_ASSERTIONS.

If I rebuild without that preprocessor directive it works.

_GLIBCXX_ASSERTIONS

Undefined by default. When defined, enables extra error checking in the form of precondition assertions, such as bounds checking in strings and null pointer checks when dereferencing smart pointers. 

[space88man]

Root cause: P11Attributes.cpp value.const_byte_str(): we are taking reference: &bytesString[0] on a zero-length ByteString. On decrypting the serialized value, we don't check that the return value has size != 0.

Minimal test case:

Compile thus:

g++ -c p11test.cpp -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -O2 -g
g++ -o p11test  p11test.o

p11test.zip

[space88man]

"can't safely get address of empty vector"

Suggests use of vector::data() method: If size() is 0, data() may or may not return a null pointer.
External references:

1. can't safely get address of empty vector rstudio/rstudio#3677
2. Assertion failures when running on Fedora 28 rstudio/httpuv#133

[Jakuje]

I am getting burnt by the same thing on Fedora with softhsm2-dump-file:

00 00 00 00 00 00 01 01 CKA_SUBJECT
00 00 00 00 00 00 00 03 byte string attribute
00 00 00 00 00 00 00 00 (length 0)
/usr/include/c++/8/bits/stl_vector.h:932: std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::operator[](std::vector<_Tp, _Alloc>::size_type) [with _Tp = unsigned char; _Alloc = std::allocator<unsigned char>; std::vector<_Tp, _Alloc>::reference = unsigned char&; std::vector<_Tp, _Alloc>::size_type = long unsigned int]: Assertion '__builtin_expect(__n < this->size(), true)' failed.
[...]
(gdb) bt
#0  0x00007ffff7ad153f in raise () from /lib64/libc.so.6
#1  0x00007ffff7abb895 in abort () from /lib64/libc.so.6
#2  0x0000555555561398 in std::__replacement_assert (__file=<optimized out>, __line=<optimized out>, 
    __function=<optimized out>, __condition=<optimized out>)
    at /usr/include/c++/8/x86_64-redhat-linux/bits/c++config.h:2391
#3  0x000055555555731f in std::vector<unsigned char, std::allocator<unsigned char> >::operator[] (
    __n=0, this=0x7fffffffd120) at softhsm2-dump-file.cpp:161
#4  readBytes (stream=0x55555557ae70, value=std::vector of length 0, capacity 0)
    at softhsm2-dump-file.cpp:158
#5  0x0000555555560fe6 in dump (stream=0x55555557ae70) at softhsm2-dump-file.cpp:441
#6  0x0000555555556805 in main (argc=<optimized out>, argv=0x7fffffffd278) at softhsm2-dump-file.cpp:535

[space88man]

Has this repo switched to C++11?