Bug #12856
closed
pkcs11_softtoken should validate session and object handles
100%
Description
PKCS#11 has the concept of session and object handles. These are just opaque handles used by a provider to identify session and objects. PKCS#11 functions that take a session or object handle as arguments are expected to return an error (CKR_SESSION_HANDLE_INVALID, CKR_OBJECT_HANDLE_INVALID, CKR_KEY_HANDLE_INVALID, etc.) when an invalid handle is passed to a function. pkcs11_softtoken uses the address of the allocated soft_session_t or soft_object_t as the handle value and uses magic values in each struct to determine if they're valid. Unfortunately, it does not first determine if the handle value is a valid address before attempting to dereference the value. This leads to situations where using an invalid handle value of UINT32_MAX or UINT64_MAX (depending on the architecture of the application) can crash the application instead of returning an error.
Updated by Jason King almost 2 years ago
Without this fix, the Google PCKS#11 test suite crashes:
[ RUN ] PKCS11Test.ParallelSessions session.cc:104: Failure Value of: session1_info.state Actual: 0 Expected: 1 session.cc:105: Failure Value of: session2_info.state Actual: 0 Expected: 1 session.cc:106: Failure Value of: session3_info.state Actual: 2 Expected: 3 [ FAILED ] PKCS11Test.ParallelSessions (10 ms) [ RUN ] PKCS11Test.SeedRandomNoSession Segmentation Fault (core dumped) root@pi:/ws/pkcs11test (unwrap)# pstack core | demangle core 'core' of 100583: ./pkcs11test -m pkcs11_softtoken.so.1 -l /usr/lib/security/amd64 fffffc7fef2d0871 handle2session () + 21 00000000005c67f3 pkcs11::test::PKCS11Test_SeedRandomNoSession_Test::TestBody() () + 41 000000000062dc3f void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) () + 69 0000000000628fd2 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) () + 4f 0000000000617c25 testing::Test::Run() () + d3 000000000061836b testing::TestInfo::Run() () + 107 00000000006188f0 testing::TestCase::Run() () + 106 000000000061cf57 testing::internal::UnitTestImpl::RunAllTests() () + 29b 000000000062ef7b bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) () + 69 0000000000629f52 bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) () + 4f 000000000061c0a3 testing::UnitTest::Run() () + 73 000000000057a9c7 main () + 78d 0000000000579ac3 _start_crt () + 83 0000000000579a28 _start () + 18
With the fix, the test suite passes the crashing test (as well as several others):
[ RUN ] PKCS11Test.SeedRandomNoSession [ OK ] PKCS11Test.SeedRandomNoSession (0 ms) [ RUN ] PKCS11Test.GenerateRandomNoSession [ OK ] PKCS11Test.GenerateRandomNoSession (0 ms) [----------] 32 tests from PKCS11Test (2 ms total) [----------] 1 test from Slot [ RUN ] Slot.NoInit [ OK ] Slot.NoInit (0 ms) [----------] 1 test from Slot (1 ms total) [----------] 26 tests from ReadOnlySessionTest [ RUN ] ReadOnlySessionTest.TokenInitWithSession [ OK ] ReadOnlySessionTest.TokenInitWithSession (0 ms) [ RUN ] ReadOnlySessionTest.InvalidSessionInfo [ OK ] ReadOnlySessionTest.InvalidSessionInfo (0 ms) [ RUN ] ReadOnlySessionTest.CloseSessionInvalid [ OK ] ReadOnlySessionTest.CloseSessionInvalid (0 ms) [ RUN ] ReadOnlySessionTest.SessionInfo ... [ RUN ] ReadOnlySessionTest.WrapUnwrap key.cc:146: Failure Value of: CK_RV_((g_fns->C_GetAttributeValue(session_, k3, &k3_get_attr, 1))) Actual: CKR_OBJECT_HANDLE_INVALID Expected: CK_RV_(0x00000000) Which is: CKR_OK key.cc:148: Failure Value of: hex_data(k3_value, k3_len) Actual: "da39a8b86743ccaa" Expected: hex_data(k1_value, k1_len) Which is: "0dad026bae377f62" Segmentation Fault (core dumped) root@pi:/ws/pkcs11test (unwrap)# pstack core | demangle core 'core' of 100595: ./pkcs11test -m pkcs11_softtoken.so.1 -l /usr/lib/security/amd64 fffffc7feefedd76 soft_crypt_cleanup (a3f070, 0, 1) + d6 fffffc7feefe0ca3 soft_delete_session (a3f070, 0, 0) + 123 fffffc7feefdc777 C_CloseSession (c4aad508c3aa2af7) + b7 00000000005a34db pkcs11::test::SessionTest::~SessionTest() () + 49 00000000005a3787 pkcs11::test::ReadOnlySessionTest::~ReadOnlySessionTest() () + 29 000000000060e6a8 pkcs11::test::ReadOnlySessionTest_WrapUnwrap_Test::~ReadOnlySessionTest_WrapUnwrap_Test() () + 24 000000000060e6c4 pkcs11::test::ReadOnlySessionTest_WrapUnwrap_Test::~ReadOnlySessionTest_WrapUnwrap_Test() () + 18 00000000006251ef testing::Test::DeleteSelf_() () + 2f
It does still end up crashing, but this is due to other issues that will be addressed in a separate bug.
Updated by Jason King almost 2 years ago
Also, as a precautionary check, I also ran all of the crypto tests, which pass as expected.
Updated by Electric Monk almost 2 years ago
- Status changed from New to Closed
- % Done changed from 0 to 100
git commit cfcec26617a5b667ad909c32c24594a670c98b2d
commit cfcec26617a5b667ad909c32c24594a670c98b2d
Author: Jason King <jason.king@joyent.com>
Date: 2020-07-06T14:09:37.000Z
12856 pkcs11_softtoken should validate session and object handles
Reviewed by: Robert Mustacchi <rm@fingolfin.org>
Approved by: Dan McDonald <danmcd@joyent.com>