Project

General

Profile

Bug #12856

pkcs11_softtoken should validate session and object handles

Added by Jason King about 2 months ago. Updated 29 days ago.

Status:
Closed
Priority:
Normal
Assignee:
Jason King
Category:
lib - userland libraries
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:
730

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.

History

#1

Updated by Electric Monk about 1 month ago

  • Gerrit CR set to 730
#2

Updated by Jason King about 1 month 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.

#3

Updated by Jason King about 1 month ago

Also, as a precautionary check, I also ran all of the crypto tests, which pass as expected.

#4

Updated by Electric Monk 29 days 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>

Also available in: Atom PDF