Skip to content

Add support for DES3 wrap/unwrap #812

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
juvex-fi wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Add support for DES3 wrap/unwrap #812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f80dc12
adds support for key wrap based on des3
Oct 29, 2021
7efd531
Added support for CKM_DES3_ECB wrap/unwrap functionality.
jukkakangas Aug 25, 2025
5d712e3
Addressed the CodeRabbit bot issues.
jukkakangas Aug 25, 2025
5cd5a55
Addressed more CodeRabbit bot issues.
jukkakangas Aug 25, 2025
2245eb1
Addressed more CodeRabbit bot issues.
jukkakangas Aug 25, 2025
045d4fb
Addressed more CodeRabbit bot issues.
jukkakangas Aug 25, 2025
00c91b3
Added key length checks.
jukkakangas Aug 26, 2025
e86110b
Fixed len checks.
jukkakangas Aug 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/lib/P11Attributes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,9 +410,10 @@ CK_RV P11Attribute::update(Token* token, bool isPrivate, CK_VOID_PTR pValue, CK_
// combinations of other attributes) for a particular library and token. Whether or not a
// given non-Cryptoki attribute is read-only is obviously outside the scope of Cryptoki.

// Attributes cannot be changed if CKA_MODIFIABLE is set to false
if (!isModifiable() && op != OBJECT_OP_GENERATE && op != OBJECT_OP_CREATE) {
ERROR_MSG("An object is with CKA_MODIFIABLE set to false is not modifiable");
// Attributes cannot be changed if CKA_MODIFIABLE is false.
// Note: During CREATE/GENERATE/UNWRAP the object is in creation; allow setting attributes in the same op.
if (!isModifiable() && op != OBJECT_OP_GENERATE && op != OBJECT_OP_CREATE && op != OBJECT_OP_UNWRAP) {
Copy link
Contributor

@bukka bukka Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already being fixed in #728

ERROR_MSG("An object with CKA_MODIFIABLE set to false is not modifiable");
return CKR_ATTRIBUTE_READ_ONLY;
}

Expand Down
229 changes: 159 additions & 70 deletions src/lib/SoftHSM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1138,13 +1138,12 @@ CK_RV SoftHSM::C_GetMechanismInfo(CK_SLOT_ID slotID, CK_MECHANISM_TYPE type, CK_
/* FALLTHROUGH */
#endif
case CKM_DES3_CBC:
pInfo->flags |= CKF_WRAP;
/* FALLTHROUGH */
case CKM_DES3_ECB:
// Key size is not in use
pInfo->ulMinKeySize = 0;
pInfo->ulMaxKeySize = 0;
pInfo->flags |= CKF_ENCRYPT | CKF_DECRYPT;
pInfo->flags |= CKF_ENCRYPT | CKF_DECRYPT | CKF_WRAP | CKF_UNWRAP;
break;
case CKM_DES3_CMAC:
// Key size is not in use
Expand Down Expand Up @@ -6269,6 +6268,7 @@ CK_RV SoftHSM::WrapKeySym
// Get the symmetric algorithm matching the mechanism
SymAlgo::Type algo = SymAlgo::Unknown;
SymWrap::Type mode = SymWrap::Unknown;
SymMode::Type sym_mode = SymMode::Unknown;
size_t bb = 8;
size_t blocksize = 0;
auto wrappedlen = keydata.size();
Expand All @@ -6291,22 +6291,44 @@ CK_RV SoftHSM::WrapKeySym
#endif
case CKM_AES_CBC:
algo = SymAlgo::AES;
sym_mode = SymMode::CBC;
blocksize = 16;
if ((wrappedlen % 16) != 0)
return CKR_KEY_SIZE_RANGE;
break;

case CKM_AES_CBC_PAD:
blocksize = 16;
wrappedlen = RFC5652Pad(keydata, blocksize);
algo = SymAlgo::AES;
break;

case CKM_DES3_CBC:
algo = SymAlgo::DES3;
sym_mode = SymMode::CBC;
break;

case CKM_DES3_CBC_PAD:
blocksize = 8;
blocksize = 8;
wrappedlen = RFC5652Pad(keydata, blocksize);
algo = SymAlgo::DES3;
sym_mode = SymMode::CBC;
bb = 7;
break;

case CKM_DES3_CBC:
if ((wrappedlen % 8) != 0)
return CKR_KEY_SIZE_RANGE;
blocksize = 8;
algo = SymAlgo::DES3;
sym_mode = SymMode::CBC;
bb = 7;
break;

case CKM_DES3_ECB:
// ECB mode has no IV; keep blocksize=0 so iv remains empty
if ((wrappedlen % 8) != 0)
return CKR_KEY_SIZE_RANGE;
blocksize = 0;
algo = SymAlgo::DES3;
sym_mode = SymMode::ECB;
bb = 7;
break;
Comment on lines 6293 to 6332
Copy link

@coderabbitai coderabbitai bot Aug 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Add targeted tests for new 3DES ECB wrap/unwrap and template enforcement

  • Wrap/unwrap CKM_DES3_ECB with:
    • correct IV handling (none), correct multiples-of-8
    • wrong ciphertext length → CKR_WRAPPED_KEY_LEN_RANGE
    • corrupt last block/padding (CBC_PAD) → CKR_WRAPPED_KEY_INVALID
  • Unwrap secret key with CKA_VALUE_LEN present:
    • matching length → OK
    • mismatching length → CKR_TEMPLATE_INCONSISTENT

I can draft unit tests exercising these cases if helpful.

Also applies to: 6923-6944, 7256-7312

🏁 Script executed:

#!/bin/bash
# Sanity: ensure the new paths are compiled-in and referenced
rg -n 'CKM_DES3_ECB' -C3 src/lib/SoftHSM.cpp
rg -n 'CKA_VALUE_LEN' -C2 src/lib/SoftHSM.cpp

Length of output: 13141

Add tests for 3DES ECB wrap/unwrap and CKA_VALUE_LEN enforcement

Please add unit tests covering the new DES3 ECB code paths and the template‐length checks:

• Wrap (SoftHSM.cpp ~6323–6332)
– CKM_DES3_ECB: ulParameterLen must be 0 (no IV)
– ciphertext lengths that are multiples of 8 → success
– non-8-aligned lengths → CKR_WRAPPED_KEY_LEN_RANGE

• Unwrap (SoftHSM.cpp ~7306–7312)
– CKM_DES3_ECB: ulParameterLen must be 0
– ulWrappedKeyLen multiple of 8 → success
– non-aligned lengths → CKR_WRAPPED_KEY_LEN_RANGE
– for CBC_PAD variants, corrupt padding → CKR_WRAPPED_KEY_INVALID

• Secret-key template enforcement (SoftHSM.cpp ~7408–7510)
– haveRequestedValueLen && keydata.size() == requestedValueLen → OK
– mismatched CKA_VALUE_LEN → CKR_TEMPLATE_INCONSISTENT

Let me know if you’d like me to draft those unit tests.

Copy link

@jukkakangas jukkakangas Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai generate unit tests

Copy link
Contributor

@bukka bukka Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the test should be part of this PR rather than a different PR generated by Code Rabbit. Those tests should be also cleaned up and verified. The reason is that this should be tested in CI before merging.

Copy link

@coderabbitai coderabbitai bot Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


default:
Expand All @@ -6333,13 +6355,18 @@ CK_RV SoftHSM::WrapKeySym
switch(pMechanism->mechanism) {

case CKM_AES_CBC:
case CKM_AES_CBC_PAD:
case CKM_AES_CBC_PAD:
case CKM_DES3_CBC:
case CKM_DES3_CBC_PAD:
iv.resize(blocksize);
memcpy(&iv[0], pMechanism->pParameter, blocksize);

if (!cipher->encryptInit(wrappingkey, SymMode::CBC, iv, false))
case CKM_DES3_CBC_PAD:
case CKM_DES3_ECB:
if (blocksize > 0) {
iv.resize(blocksize);
memcpy(&iv[0], pMechanism->pParameter, blocksize);
} else {
iv.wipe();
}

if (!cipher->encryptInit(wrappingkey, sym_mode, iv, false))
{
cipher->recycleKey(wrappingkey);
CryptoFactory::i()->recycleSymmetricAlgorithm(cipher);
Expand Down Expand Up @@ -6612,6 +6639,15 @@ CK_RV SoftHSM::C_WrapKey
pMechanism->ulParameterLen != 16)
return CKR_ARGUMENTS_BAD;
break;
case CKM_DES3_ECB:
if (pMechanism->ulParameterLen != 0)
return CKR_ARGUMENTS_BAD; // no IV for ECB; allow NULL or non-NULL pointer when length==0
break;
case CKM_DES3_CBC:
case CKM_DES3_CBC_PAD:
if (pMechanism->pParameter == NULL_PTR || pMechanism->ulParameterLen != 8)
return CKR_ARGUMENTS_BAD;
break;
default:
return CKR_MECHANISM_INVALID;
}
Expand Down Expand Up @@ -6652,9 +6688,11 @@ CK_RV SoftHSM::C_WrapKey
return CKR_WRAPPING_KEY_TYPE_INCONSISTENT;
if ((pMechanism->mechanism == CKM_AES_CBC || pMechanism->mechanism == CKM_AES_CBC_PAD) && wrapKey->getUnsignedLongValue(CKA_KEY_TYPE, CKK_VENDOR_DEFINED) != CKK_AES)
return CKR_WRAPPING_KEY_TYPE_INCONSISTENT;
if (pMechanism->mechanism == CKM_DES3_CBC && (wrapKey->getUnsignedLongValue(CKA_KEY_TYPE, CKK_VENDOR_DEFINED) != CKK_DES2 ||
wrapKey->getUnsignedLongValue(CKA_KEY_TYPE, CKK_VENDOR_DEFINED) != CKK_DES3))
return CKR_WRAPPING_KEY_TYPE_INCONSISTENT;
if ((pMechanism->mechanism == CKM_DES3_CBC || pMechanism->mechanism == CKM_DES3_CBC_PAD || pMechanism->mechanism == CKM_DES3_ECB) &&
((wrapKey->getUnsignedLongValue(CKA_KEY_TYPE, CKK_VENDOR_DEFINED) != CKK_DES2 &&
wrapKey->getUnsignedLongValue(CKA_KEY_TYPE, CKK_VENDOR_DEFINED) != CKK_DES3))) {
return CKR_WRAPPING_KEY_TYPE_INCONSISTENT;
}

// Check if the wrapping key can be used for wrapping
if (wrapKey->getBooleanValue(CKA_WRAP, false) == false)
Expand Down Expand Up @@ -6865,6 +6903,7 @@ CK_RV SoftHSM::UnwrapKeySym
// Get the symmetric algorithm matching the mechanism
SymAlgo::Type algo = SymAlgo::Unknown;
SymWrap::Type mode = SymWrap::Unknown;
SymMode::Type sym_mode = SymMode::Unknown;
size_t bb = 8;
size_t blocksize = 0;

Expand All @@ -6881,16 +6920,28 @@ CK_RV SoftHSM::UnwrapKeySym
mode = SymWrap::AES_KEYWRAP_PAD;
break;
#endif
case CKM_AES_CBC_PAD:
case CKM_AES_CBC_PAD:
algo = SymAlgo::AES;
sym_mode = SymMode::CBC;
blocksize = 16;
break;

case CKM_DES3_CBC_PAD:

case CKM_DES3_CBC_PAD:
case CKM_DES3_CBC:
algo = SymAlgo::DES3;
sym_mode = SymMode::CBC;
blocksize = 8;
break;

bb = 7;
break;

case CKM_DES3_ECB:
algo = SymAlgo::DES3;
sym_mode = SymMode::ECB;
// ECB mode has no IV; keep blocksize=0 so iv remains empty
blocksize = 0;
bb = 7;
break;

default:
return CKR_MECHANISM_INVALID;
}
Expand All @@ -6916,39 +6967,43 @@ CK_RV SoftHSM::UnwrapKeySym

switch(pMechanism->mechanism) {

case CKM_AES_CBC_PAD:
case CKM_DES3_CBC_PAD:
iv.resize(blocksize);
memcpy(&iv[0], pMechanism->pParameter, blocksize);

if (!cipher->decryptInit(unwrappingkey, SymMode::CBC, iv, false))
{
cipher->recycleKey(unwrappingkey);
CryptoFactory::i()->recycleSymmetricAlgorithm(cipher);
return CKR_MECHANISM_INVALID;
}
if (!cipher->decryptUpdate(wrapped, keydata))
{
cipher->recycleKey(unwrappingkey);
CryptoFactory::i()->recycleSymmetricAlgorithm(cipher);
return CKR_GENERAL_ERROR;
}
// Finalize encryption
if (!cipher->decryptFinal(decryptedFinal))
{
cipher->recycleKey(unwrappingkey);
CryptoFactory::i()->recycleSymmetricAlgorithm(cipher);
return CKR_GENERAL_ERROR;
}
keydata += decryptedFinal;
case CKM_AES_CBC_PAD:
case CKM_DES3_CBC_PAD:
case CKM_DES3_CBC:
case CKM_DES3_ECB:
if (blocksize > 0) {
iv.resize(blocksize);
memcpy(&iv[0], pMechanism->pParameter, blocksize);
} else {
iv.wipe();
}

if(!RFC5652Unpad(keydata,blocksize))
{
cipher->recycleKey(unwrappingkey);
CryptoFactory::i()->recycleSymmetricAlgorithm(cipher);
return CKR_GENERAL_ERROR; // TODO should be another error
}
break;
if (!cipher->decryptInit(unwrappingkey, sym_mode, iv, false)) {
cipher->recycleKey(unwrappingkey);
CryptoFactory::i()->recycleSymmetricAlgorithm(cipher);
return CKR_MECHANISM_INVALID;
}
if (!cipher->decryptUpdate(wrapped, keydata)) {
cipher->recycleKey(unwrappingkey);
CryptoFactory::i()->recycleSymmetricAlgorithm(cipher);
return CKR_GENERAL_ERROR;
}
// Finalize decryption
if (!cipher->decryptFinal(decryptedFinal)) {
cipher->recycleKey(unwrappingkey);
CryptoFactory::i()->recycleSymmetricAlgorithm(cipher);
return CKR_WRAPPED_KEY_INVALID;
}
keydata += decryptedFinal;

if (pMechanism->mechanism == CKM_AES_CBC_PAD || pMechanism->mechanism == CKM_DES3_CBC_PAD) {
if (!RFC5652Unpad(keydata, blocksize)) {
cipher->recycleKey(unwrappingkey);
CryptoFactory::i()->recycleSymmetricAlgorithm(cipher);
return CKR_WRAPPED_KEY_INVALID;
}
}
break;

default:
// Unwrap the key
Expand Down Expand Up @@ -7227,20 +7282,34 @@ CK_RV SoftHSM::C_UnwrapKey
return rv;
break;

case CKM_AES_CBC_PAD:
// TODO check block length
if (pMechanism->pParameter == NULL_PTR ||
pMechanism->ulParameterLen != 16)
case CKM_AES_CBC_PAD:
if (pMechanism->pParameter == NULL_PTR || pMechanism->ulParameterLen != 16)
return CKR_ARGUMENTS_BAD;
if (ulWrappedKeyLen == 0 || (ulWrappedKeyLen % 16) != 0)
return CKR_WRAPPED_KEY_LEN_RANGE;
break;

case CKM_DES3_CBC_PAD:
// TODO check block length
if (pMechanism->pParameter == NULL_PTR ||
pMechanism->ulParameterLen != 8)
case CKM_DES3_CBC_PAD:
if (pMechanism->pParameter == NULL_PTR || pMechanism->ulParameterLen != 8)
return CKR_ARGUMENTS_BAD;
if (ulWrappedKeyLen == 0 || (ulWrappedKeyLen % 8) != 0)
return CKR_WRAPPED_KEY_LEN_RANGE;
break;

case CKM_DES3_CBC:
if (pMechanism->pParameter == NULL_PTR || pMechanism->ulParameterLen != 8)
return CKR_ARGUMENTS_BAD;
if ((ulWrappedKeyLen % 8) != 0)
return CKR_WRAPPED_KEY_LEN_RANGE;
break;


case CKM_DES3_ECB:
if (pMechanism->ulParameterLen != 0)
return CKR_ARGUMENTS_BAD; // ECB takes no IV; allow NULL or non-NULL pointer when length==0
if ((ulWrappedKeyLen % 8) != 0)
return CKR_WRAPPED_KEY_LEN_RANGE;
break;

default:
return CKR_MECHANISM_INVALID;
}
Expand Down Expand Up @@ -7279,12 +7348,14 @@ CK_RV SoftHSM::C_UnwrapKey
if ((pMechanism->mechanism == CKM_RSA_PKCS || pMechanism->mechanism == CKM_RSA_PKCS_OAEP || pMechanism->mechanism == CKM_RSA_AES_KEY_WRAP) &&
unwrapKey->getUnsignedLongValue(CKA_KEY_TYPE, CKK_VENDOR_DEFINED) != CKK_RSA)
return CKR_UNWRAPPING_KEY_TYPE_INCONSISTENT;
if ((pMechanism->mechanism == CKM_AES_CBC || pMechanism->mechanism == CKM_AES_CBC_PAD) && unwrapKey->getUnsignedLongValue(CKA_KEY_TYPE, CKK_VENDOR_DEFINED) != CKK_AES)
return CKR_WRAPPING_KEY_TYPE_INCONSISTENT;
if (pMechanism->mechanism == CKM_DES3_CBC && (unwrapKey->getUnsignedLongValue(CKA_KEY_TYPE, CKK_VENDOR_DEFINED) != CKK_DES2 ||
unwrapKey->getUnsignedLongValue(CKA_KEY_TYPE, CKK_VENDOR_DEFINED) != CKK_DES3))
return CKR_WRAPPING_KEY_TYPE_INCONSISTENT;

if ((pMechanism->mechanism == CKM_AES_CBC || pMechanism->mechanism == CKM_AES_CBC_PAD) &&
unwrapKey->getUnsignedLongValue(CKA_KEY_TYPE, CKK_VENDOR_DEFINED) != CKK_AES)
return CKR_UNWRAPPING_KEY_TYPE_INCONSISTENT;
if ((pMechanism->mechanism == CKM_DES3_CBC_PAD || pMechanism->mechanism == CKM_DES3_CBC || pMechanism->mechanism == CKM_DES3_ECB) &&
(unwrapKey->getUnsignedLongValue(CKA_KEY_TYPE, CKK_VENDOR_DEFINED) != CKK_DES2 &&
unwrapKey->getUnsignedLongValue(CKA_KEY_TYPE, CKK_VENDOR_DEFINED) != CKK_DES3))
return CKR_UNWRAPPING_KEY_TYPE_INCONSISTENT;

// Check if the unwrapping key can be used for unwrapping
if (unwrapKey->getBooleanValue(CKA_UNWRAP, false) == false)
return CKR_KEY_FUNCTION_NOT_PERMITTED;
Expand Down Expand Up @@ -7334,7 +7405,11 @@ CK_RV SoftHSM::C_UnwrapKey
};
CK_ULONG secretAttribsCount = 4;

// Add the additional
// Capture optional CKA_VALUE_LEN from the template (for compatibility)
CK_ULONG requestedValueLen = 0;
bool haveRequestedValueLen = false;

// Add the additional attributes, skipping read-only for unwrap and capturing CKA_VALUE_LEN
if (ulCount > (maxAttribs - secretAttribsCount))
return CKR_TEMPLATE_INCONSISTENT;
for (CK_ULONG i = 0; i < ulCount; ++i)
Expand All @@ -7346,6 +7421,14 @@ CK_RV SoftHSM::C_UnwrapKey
case CKA_PRIVATE:
case CKA_KEY_TYPE:
continue;
case CKA_VALUE_LEN:
// Accept a caller-provided length for compatibility, but do not set it here; we will verify after unwrap
if (pTemplate[i].ulValueLen != sizeof(CK_ULONG)) {
return CKR_TEMPLATE_INCONSISTENT;
}
requestedValueLen = *(CK_ULONG *) pTemplate[i].pValue;
haveRequestedValueLen = true;
continue;
default:
secretAttribs[secretAttribsCount++] = pTemplate[i];
}
Expand Down Expand Up @@ -7420,6 +7503,12 @@ CK_RV SoftHSM::C_UnwrapKey
return rv;
}

// If provided, enforce CKA_VALUE_LEN only for secret keys
if (objClass == CKO_SECRET_KEY && haveRequestedValueLen && keydata.size() != requestedValueLen)
{
return CKR_TEMPLATE_INCONSISTENT;
}

// Create the secret object using C_CreateObject
rv = this->CreateObject(hSession, secretAttribs, secretAttribsCount, hKey, OBJECT_OP_UNWRAP);

Expand Down Expand Up @@ -12044,7 +12133,7 @@ CK_RV SoftHSM::deriveSymmetric
// attributes set to CK_TRUE
bool bNeverExtractable = baseKey->getBooleanValue(CKA_NEVER_EXTRACTABLE, false) &&
otherKey->getBooleanValue(CKA_NEVER_EXTRACTABLE, false);
bOK = bOK && osobject->setAttribute(CKA_ALWAYS_SENSITIVE, bNeverExtractable);
bOK = bOK && osobject->setAttribute(CKA_NEVER_EXTRACTABLE, bNeverExtractable);
Copy link
Contributor

@bukka bukka Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an unrelated change which should be in its own PR with a test.

}
else if (pMechanism->mechanism == CKM_CONCATENATE_BASE_AND_DATA ||
pMechanism->mechanism == CKM_CONCATENATE_DATA_AND_BASE)
Expand Down
7 changes: 5 additions & 2 deletions src/lib/crypto/BotanDES.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,15 @@ std::string BotanDES::getCipher() const
switch (currentKey->getBitLen())
{
case 56:
// People shouldn't really be using 56-bit DES keys, generate a warning
DEBUG_MSG("CAUTION: use of 56-bit DES keys is not recommended!");
case 64:
// Single-DES (effective 56-bit) is weak; warn irrespective of representation
DEBUG_MSG("CAUTION: use of single-DES keys (effective 56-bit) is not recommended!");
algo = "DES";
break;
case 112:
case 128:
case 168:
case 192:
algo = "TripleDES";
break;
default:
Expand Down
Loading
Loading