Skip to content

Commit 4eccc62

Browse files
committed
Store the current object ID in token
When a token is stored and then reloaded during `C_Initialize`, the token object IDs are reset to zero. Which means new objects will end up with the same IDs as loaded objects. This is probably a "bad thing", so let's not do that. With this patch the next object ID is stored with the token. Also: * Fix MD5 breaking tests with wolfSSL#8895 change. * Fix parallel tests storage race condition
1 parent 1c3df16 commit 4eccc62

19 files changed

Lines changed: 599 additions & 31 deletions

.github/workflows/build-workflow.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ jobs:
3434
- name: wolfssl configure
3535
working-directory: ./wolfssl
3636
run: |
37-
./configure --enable-cryptocb --enable-aescfb --enable-rsapss --enable-keygen --enable-pwdbased --enable-scrypt \
37+
./configure --enable-cryptocb --enable-aescfb --enable-rsapss --enable-keygen --enable-pwdbased --enable-scrypt --enable-md5 \
3838
C_EXTRA_FLAGS="-DWOLFSSL_PUBLIC_MP -DWC_RSA_DIRECT"
3939
- name: wolfssl make install
4040
working-directory: ./wolfssl

.github/workflows/clang-tidy.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ jobs:
6969
git clone https://github.com/wolfSSL/wolfssl.git
7070
cd wolfssl
7171
./autogen.sh
72-
./configure --enable-cryptocb --enable-aescfb --enable-rsapss --enable-keygen --enable-pwdbased --enable-scrypt \
72+
./configure --enable-cryptocb --enable-aescfb --enable-rsapss --enable-keygen --enable-pwdbased --enable-scrypt --enable-md5 \
7373
C_EXTRA_FLAGS="-DWOLFSSL_PUBLIC_MP -DWC_RSA_DIRECT"
7474
make -j$(nproc)
7575
sudo make install

.github/workflows/nss-cmsutil-test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ jobs:
158158
git clone https://github.com/wolfSSL/wolfssl.git --branch ${{ env.WOLFSSL_VERSION }} --depth 1
159159
cd wolfssl
160160
./autogen.sh
161-
./configure --enable-aescfb --enable-cryptocb --enable-rsapss --enable-keygen --enable-pwdbased --enable-scrypt --enable-cmac --enable-aesctr --enable-aesccm C_EXTRA_FLAGS="-DWOLFSSL_PUBLIC_MP -DWC_RSA_DIRECT -DHAVE_AES_ECB -D_GNU_SOURCE"
161+
./configure --enable-aescfb --enable-cryptocb --enable-rsapss --enable-keygen --enable-pwdbased --enable-scrypt --enable-cmac --enable-aesctr --enable-aesccm --enable-md5 C_EXTRA_FLAGS="-DWOLFSSL_PUBLIC_MP -DWC_RSA_DIRECT -DHAVE_AES_ECB -D_GNU_SOURCE"
162162
make
163163
164164
- name: Install wolfSSL

.github/workflows/nss-pdfsig-test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ jobs:
178178
git clone https://github.com/wolfSSL/wolfssl.git --branch ${{ env.WOLFSSL_VERSION }} --depth 1
179179
cd wolfssl
180180
./autogen.sh
181-
./configure --enable-aescfb --enable-cryptocb --enable-rsapss --enable-keygen --enable-pwdbased --enable-scrypt --enable-cmac --enable-aesctr --enable-aesccm C_EXTRA_FLAGS="-DWOLFSSL_PUBLIC_MP -DWC_RSA_DIRECT -DHAVE_AES_ECB -D_GNU_SOURCE"
181+
./configure --enable-aescfb --enable-cryptocb --enable-rsapss --enable-keygen --enable-pwdbased --enable-scrypt --enable-cmac --enable-aesctr --enable-aesccm --enable-md5 C_EXTRA_FLAGS="-DWOLFSSL_PUBLIC_MP -DWC_RSA_DIRECT -DHAVE_AES_ECB -D_GNU_SOURCE"
182182
make
183183
184184
- name: Install wolfSSL

.github/workflows/nss-ssltap-test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ jobs:
137137
git clone https://github.com/wolfSSL/wolfssl.git --branch ${{ env.WOLFSSL_VERSION }} --depth 1
138138
cd wolfssl
139139
./autogen.sh
140-
./configure --enable-aescfb --enable-cryptocb --enable-rsapss --enable-keygen --enable-pwdbased --enable-scrypt --enable-cmac --enable-aesctr --enable-aesccm C_EXTRA_FLAGS="-DWOLFSSL_PUBLIC_MP -DWC_RSA_DIRECT -DHAVE_AES_ECB -D_GNU_SOURCE"
140+
./configure --enable-aescfb --enable-cryptocb --enable-rsapss --enable-keygen --enable-pwdbased --enable-scrypt --enable-cmac --enable-aesctr --enable-aesccm --enable-md5 C_EXTRA_FLAGS="-DWOLFSSL_PUBLIC_MP -DWC_RSA_DIRECT -DHAVE_AES_ECB -D_GNU_SOURCE"
141141
make
142142
143143
- name: Install wolfSSL

.github/workflows/sanitizer-tests.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ jobs:
7272
export LDFLAGS="-fsanitize=undefined"
7373
;;
7474
esac
75-
./configure --enable-cryptocb --enable-aescfb --enable-rsapss --enable-keygen --enable-pwdbased --enable-scrypt --enable-debug \
75+
./configure --enable-cryptocb --enable-aescfb --enable-rsapss --enable-keygen --enable-pwdbased --enable-scrypt --enable-md5 --enable-debug \
7676
C_EXTRA_FLAGS="-DWOLFSSL_PUBLIC_MP -DWC_RSA_DIRECT"
7777
- name: wolfssl make
7878
working-directory: ./wolfssl

.github/workflows/storage-upgrade-test.yml

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -97,17 +97,11 @@ jobs:
9797
echo "=== Copying storage files from ${{ matrix.base-ref.name }} to PR branch ==="
9898
9999
# Create directories if they don't exist
100-
mkdir -p pr-branch/tests
101100
mkdir -p pr-branch/store
102101
103-
# Copy test storage files
104-
if [ -d "${{ matrix.base-ref.branch-dir }}/tests" ]; then
105-
cp -v ${{ matrix.base-ref.branch-dir }}/tests/wp* pr-branch/tests/ 2>/dev/null || echo "No wp* files in ${{ matrix.base-ref.branch-dir }}/tests/"
106-
fi
107-
108102
# Copy store files
109103
if [ -d "${{ matrix.base-ref.branch-dir }}/store" ]; then
110-
cp -v ${{ matrix.base-ref.branch-dir }}/store/wp* pr-branch/store/ 2>/dev/null || echo "No wp* files in ${{ matrix.base-ref.branch-dir }}/store/"
104+
cp -rv ${{ matrix.base-ref.branch-dir }}/store/* pr-branch/store/ 2>/dev/null || echo "No files in ${{ matrix.base-ref.branch-dir }}/store/"
111105
fi
112106
113107
echo "=== Storage file copy completed ==="
@@ -119,10 +113,8 @@ jobs:
119113
echo "This tests that the PR can read storage files created by ${{ matrix.base-ref.name }} branch"
120114
121115
# List the copied files for verification
122-
echo "Files in tests directory:"
123-
ls -la tests/wp* 2>/dev/null || echo "No wp* files in tests/"
124116
echo "Files in store directory:"
125-
ls -la store/wp* 2>/dev/null || echo "No wp* files in store/"
117+
ls -la store/* 2>/dev/null || echo "No wp* files in store/"
126118
127119
# Run the tests with the copied storage files
128120
make test
@@ -137,8 +129,7 @@ jobs:
137129
name: storage-upgrade-test-artifacts-${{ matrix.base-ref.name }}
138130
path: |
139131
pr-branch/test-suite.log
140-
${{ matrix.base-ref.branch-dir }}/store/wp*
141-
${{ matrix.base-ref.branch-dir }}/tests/wp*
132+
${{ matrix.base-ref.branch-dir }}/store/*
142133
${{ matrix.base-ref.branch-dir }}/test-suite.log
143134
retention-days: 5
144135

.gitignore

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ coverage.info
3434
tests/pkcs11test
3535
tests/pkcs11mtt
3636
tests/pkcs11str
37+
tests/object_id_uniqueness_test
38+
tests/rsa_session_persistence_test
39+
tests/debug_test
40+
tests/token_path_test
3741
examples/add_aes_key
3842
examples/add_hmac_key
3943
examples/add_rsa_key

configure.ac

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -339,9 +339,9 @@ AC_ARG_ENABLE([md5],
339339
)
340340
if test "$ENABLED_MD5" = "no"
341341
then
342-
AM_CFLAGS="$AM_CFLAGS -DNO_MD5"
342+
AM_CFLAGS="$AM_CFLAGS -DNO_WOLFPKCS11_MD5"
343343
else
344-
DISABLE_DEFS="$DISABLE_DEFS -DNO_MD5"
344+
DISABLE_DEFS="$DISABLE_DEFS -DNO_WOLFPKCS11_MD5"
345345
fi
346346

347347
AC_ARG_ENABLE([sha],

src/internal.c

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,7 @@ typedef struct WP11_Token {
477477
WP11_Object* object; /* Linked list of token objects */
478478
int objCnt; /* Count of objects on token */
479479
int tokenFlags; /* Flags for token */
480+
int nextObjId;
480481
} WP11_Token;
481482

482483
struct WP11_Slot {
@@ -486,7 +487,6 @@ struct WP11_Slot {
486487
WP11_Lock lock; /* Lock for access to slot info */
487488

488489
int devId;
489-
int nextObjId;
490490
#ifdef WOLFPKCS11_TPM
491491
WOLFTPM2_DEV tpmDev;
492492
WOLFTPM2_KEY tpmSrk;
@@ -880,6 +880,7 @@ static int wolfPKCS11_Store_GetMaxSize(int type, int variableSz)
880880
FIELD_SIZE(WP11_Token, seed) +
881881
FIELD_SIZE(WP11_Token, objCnt) +
882882
FIELD_SIZE(WP11_Token, tokenFlags) +
883+
FIELD_SIZE(WP11_Token, nextObjId) +
883884
variableSz /* soPinLen + userPinLen + (objCnt * long) */
884885
;
885886
break;
@@ -3987,6 +3988,7 @@ static int wp11_Token_Init(WP11_Token* token, const char* label)
39873988
if (ret == 0) {
39883989
token->state = WP11_TOKEN_STATE_INITIALIZED;
39893990
token->loginState = WP11_APP_STATE_RW_PUBLIC;
3991+
token->nextObjId = 1;
39903992
XMEMCPY(token->label, label, sizeof(token->label));
39913993
}
39923994

@@ -4130,8 +4132,16 @@ static int wp11_Token_Load(WP11_Slot* slot, int tokenId, WP11_Token* token)
41304132
if (token->soPinLen > 0) {
41314133
token->tokenFlags |= WP11_TOKEN_FLAG_SO_PIN_SET;
41324134
}
4135+
token->nextObjId = 1;
41334136
ret = 0;
41344137
}
4138+
else {
4139+
ret = wp11_storage_read_int(storage, &token->nextObjId);
4140+
if (ret == BUFFER_E || token->nextObjId == 0) {
4141+
token->nextObjId = 1;
4142+
ret = 0;
4143+
}
4144+
}
41354145
}
41364146

41374147
wp11_storage_close(storage);
@@ -4264,6 +4274,11 @@ static int wp11_Token_Store(WP11_Token* token, int tokenId)
42644274
ret = wp11_storage_write_int(storage, token->tokenFlags);
42654275
}
42664276

4277+
if (ret == 0) {
4278+
/* Write next object id. (4) */
4279+
ret = wp11_storage_write_int(storage, token->nextObjId);
4280+
}
4281+
42674282
wp11_storage_close(storage);
42684283

42694284
object = token->object;
@@ -4414,7 +4429,6 @@ static int wp11_Slot_Init(WP11_Slot* slot, int id)
44144429

44154430
XMEMSET(slot, 0, sizeof(*slot));
44164431
slot->id = id;
4417-
slot->nextObjId = 1;
44184432
slot->token.state = WP11_TOKEN_STATE_UNKNOWN;
44194433
slot->token.tokenFlags = 0;
44204434

@@ -6074,7 +6088,7 @@ int WP11_Session_AddObject(WP11_Session* session, int onToken,
60746088
/* Get next item in list after this object has been added. */
60756089
next = token->object;
60766090
/* Determine handle value */
6077-
object->handle = OBJ_HANDLE(onToken, session->slot->nextObjId++);
6091+
object->handle = OBJ_HANDLE(onToken, token->nextObjId++);
60786092
object->next = next;
60796093
token->object = object;
60806094
}
@@ -6092,7 +6106,7 @@ int WP11_Session_AddObject(WP11_Session* session, int onToken,
60926106
/* Get next item in list after this object has been added. */
60936107
next = session->object;
60946108
/* Determine handle value */
6095-
object->handle = OBJ_HANDLE(onToken, session->slot->nextObjId++);
6109+
object->handle = OBJ_HANDLE(onToken, token->nextObjId++);
60966110
object->next = next;
60976111
session->object = object;
60986112
object->session = session;

0 commit comments

Comments
 (0)