Move crypto alg handles to Device object

This is done in preparation for multí-peer support. Fixes #55 Signed-off-by: Lev Stipakov <[email protected]> Signed-off-by: Leon Dang <[email protected]> Co-authored-by: Leon Dang <[email protected]>
OpenVPN · Nov 30, 2023 · c4b7b26 · c4b7b26
1 parent 1b8b417
commit c4b7b26
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 33 deletions.
diff --git a/Driver.cpp b/Driver.cpp
@@ -336,6 +336,10 @@ VOID OvpnEvtDeviceCleanup(WDFOBJECT obj) {
     device->Adapter = WDF_NO_HANDLE;
     ExReleaseSpinLockExclusive(&device->SpinLock, irql);
 
+    // OvpnCryptoUninitAlgHandles called outside of lock because
+    // it requires PASSIVE_LEVEL.
+    OvpnCryptoUninitAlgHandles(device->AesAlgHandle, device->ChachaAlgHandle);
+
     LOG_EXIT();
 }
 
@@ -470,6 +474,8 @@ OvpnEvtDeviceAdd(WDFDRIVER wdfDriver, PWDFDEVICE_INIT deviceInit) {
     GOTO_IF_NOT_NT_SUCCESS(done, status, OvpnBufferQueueCreate(&device->ControlRxBufferQueue));
     GOTO_IF_NOT_NT_SUCCESS(done, status, OvpnBufferQueueCreate(&device->DataRxBufferQueue));
 
+    GOTO_IF_NOT_NT_SUCCESS(done, status, OvpnCryptoInitAlgHandles(&device->AesAlgHandle, &device->ChachaAlgHandle));
+
     LOG_IF_NOT_NT_SUCCESS(status = OvpnAdapterCreate(device));
 
 done:

diff --git a/Driver.h b/Driver.h
@@ -92,6 +92,9 @@ struct OVPN_DEVICE {
     _Guarded_by_(SpinLock)
     WDFTIMER KeepaliveRecvTimer;
 
+    BCRYPT_ALG_HANDLE AesAlgHandle;
+    BCRYPT_ALG_HANDLE ChachaAlgHandle;
+
     // set from the userspace, defines TCP Maximum Segment Size
     _Guarded_by_(SpinLock)
     UINT16 MSS;

diff --git a/PropertySheet.props b/PropertySheet.props
@@ -4,7 +4,7 @@
   <PropertyGroup Label="UserMacros">
     <OVPN_DCO_VERSION_MAJOR>1</OVPN_DCO_VERSION_MAJOR>
     <OVPN_DCO_VERSION_MINOR>0</OVPN_DCO_VERSION_MINOR>
-    <OVPN_DCO_VERSION_PATCH>0</OVPN_DCO_VERSION_PATCH>
+    <OVPN_DCO_VERSION_PATCH>1</OVPN_DCO_VERSION_PATCH>
   </PropertyGroup>
   <PropertyGroup />
   <ItemDefinitionGroup>

diff --git a/crypto.cpp b/crypto.cpp
@@ -221,7 +221,7 @@ OvpnCryptoEncryptAEAD(OvpnCryptoKeySlot* keySlot, UCHAR* buf, SIZE_T len)
 
 _Use_decl_annotations_
 NTSTATUS
-OvpnCryptoNewKey(OvpnCryptoContext* cryptoContext, POVPN_CRYPTO_DATA cryptoData)
+OvpnCryptoNewKey(OvpnCryptoContext* cryptoContext, POVPN_CRYPTO_DATA cryptoData, BCRYPT_ALG_HANDLE algHandle)
 {
     OvpnCryptoKeySlot* keySlot = NULL;
     NTSTATUS status = STATUS_SUCCESS;
@@ -249,19 +249,6 @@ OvpnCryptoNewKey(OvpnCryptoContext* cryptoContext, POVPN_CRYPTO_DATA cryptoData)
             keySlot->DecKey = NULL;
         }
 
-        BCRYPT_ALG_HANDLE algHandle = NULL;
-        if (cryptoData->CipherAlg == OVPN_CIPHER_ALG_AES_GCM) {
-            algHandle = cryptoContext->AesAlgHandle;
-        }
-        else {
-            if (cryptoContext->ChachaAlgHandle == NULL) {
-                LOG_ERROR("CHACHA20-POLY1305 is not available");
-                status = STATUS_INVALID_DEVICE_REQUEST;
-                goto done;
-            }
-            algHandle = cryptoContext->ChachaAlgHandle;
-        }
-
         // generate keys from key materials
         GOTO_IF_NOT_NT_SUCCESS(done, status, BCryptGenerateSymmetricKey(algHandle, &keySlot->EncKey, NULL, 0, cryptoData->Encrypt.Key, cryptoData->Encrypt.KeyLen, 0));
         GOTO_IF_NOT_NT_SUCCESS(done, status, BCryptGenerateSymmetricKey(algHandle, &keySlot->DecKey, NULL, 0, cryptoData->Decrypt.Key, cryptoData->Decrypt.KeyLen, 0));

diff --git a/crypto.h b/crypto.h
@@ -75,9 +75,6 @@ typedef OVPN_CRYPTO_DECRYPT* POVPN_CRYPTO_DECRYPT;
 
 struct OvpnCryptoContext
 {
-    BCRYPT_ALG_HANDLE AesAlgHandle;
-    BCRYPT_ALG_HANDLE ChachaAlgHandle;
-
     OvpnCryptoKeySlot Primary;
     OvpnCryptoKeySlot Secondary;
 
@@ -101,7 +98,7 @@ OvpnCryptoUninit(_In_ OvpnCryptoContext* cryptoContext);
 
 _Must_inspect_result_
 NTSTATUS
-OvpnCryptoNewKey(_In_ OvpnCryptoContext* cryptoContext, _In_ POVPN_CRYPTO_DATA cryptoData);
+OvpnCryptoNewKey(_In_ OvpnCryptoContext* cryptoContext, _In_ POVPN_CRYPTO_DATA cryptoData, _In_opt_ BCRYPT_ALG_HANDLE algHandle);
 
 _Must_inspect_result_
 OvpnCryptoKeySlot*

diff --git a/peer.cpp b/peer.cpp
@@ -74,13 +74,8 @@ OvpnPeerNew(POVPN_DEVICE device, WDFREQUEST request)
     GOTO_IF_NOT_NT_SUCCESS(done, status, OvpnSocketInit(&driver->WskProviderNpi, &driver->WskRegistration, peer->Local.Addr4.sin_family, proto_tcp, (PSOCKADDR)&peer->Local,
         (PSOCKADDR)&peer->Remote, remoteAddrSize, device, &socket));
 
-    BCRYPT_ALG_HANDLE aesAlgHandle = NULL, chachaAlgHandle = NULL;
-    GOTO_IF_NOT_NT_SUCCESS(done, status, OvpnCryptoInitAlgHandles(&aesAlgHandle, &chachaAlgHandle));
-
     KIRQL kirql = ExAcquireSpinLockExclusive(&device->SpinLock);
     RtlZeroMemory(&device->CryptoContext, sizeof(OvpnCryptoContext));
-    device->CryptoContext.AesAlgHandle = aesAlgHandle;
-    device->CryptoContext.ChachaAlgHandle = chachaAlgHandle;
     device->Socket.Socket = socket;
     device->Socket.Tcp = proto_tcp;
     RtlZeroMemory(&device->Socket.TcpState, sizeof(OvpnSocketTcpState));
@@ -112,16 +107,11 @@ OvpnPeerDel(POVPN_DEVICE device)
         return STATUS_INVALID_DEVICE_REQUEST;
     }
 
-    BCRYPT_ALG_HANDLE aesAlgHandle = NULL, chachaAlgHandle = NULL;
-
     KIRQL kirql = ExAcquireSpinLockExclusive(&device->SpinLock);
 
     OvpnTimerDestroy(&device->KeepaliveXmitTimer);
     OvpnTimerDestroy(&device->KeepaliveRecvTimer);
 
-    aesAlgHandle = device->CryptoContext.AesAlgHandle;
-    chachaAlgHandle = device->CryptoContext.ChachaAlgHandle;
-
     OvpnCryptoUninit(&device->CryptoContext);
 
     InterlockedExchange(&device->UserspacePid, 0);
@@ -132,11 +122,9 @@ OvpnPeerDel(POVPN_DEVICE device)
     RtlZeroMemory(&device->Socket.TcpState, sizeof(OvpnSocketTcpState));
     RtlZeroMemory(&device->Socket.UdpState, sizeof(OvpnSocketUdpState));
 
-    // OvpnCryptoUninitAlgHandles and OvpnSocketClose require PASSIVE_LEVEL, so must release lock
+    // OvpnSocketClose requires PASSIVE_LEVEL, so must release lock
     ExReleaseSpinLockExclusive(&device->SpinLock, kirql);
 
-    OvpnCryptoUninitAlgHandles(aesAlgHandle, chachaAlgHandle);
-
     LOG_IF_NOT_NT_SUCCESS(OvpnSocketClose(socket));
 
     // flush buffers in control queue so that client won't get control channel messages from previous session
@@ -278,7 +266,24 @@ OvpnPeerNewKey(POVPN_DEVICE device, WDFREQUEST request)
     NTSTATUS status;
 
     GOTO_IF_NOT_NT_SUCCESS(done, status, WdfRequestRetrieveInputBuffer(request, sizeof(OVPN_CRYPTO_DATA), (PVOID*)&cryptoData, nullptr));
-    GOTO_IF_NOT_NT_SUCCESS(done, status, OvpnCryptoNewKey(&device->CryptoContext, cryptoData));
+
+    BCRYPT_ALG_HANDLE algHandle = NULL;
+    switch (cryptoData->CipherAlg) {
+    case OVPN_CIPHER_ALG_AES_GCM:
+        algHandle = device->AesAlgHandle;
+        break;
+
+    case OVPN_CIPHER_ALG_CHACHA20_POLY1305:
+        algHandle = device->ChachaAlgHandle;
+        if (algHandle == NULL) {
+            LOG_ERROR("CHACHA20-POLY1305 is not available");
+            status = STATUS_INVALID_DEVICE_REQUEST;
+            goto done;
+        }
+        break;
+    }
+
+    GOTO_IF_NOT_NT_SUCCESS(done, status, OvpnCryptoNewKey(&device->CryptoContext, cryptoData, algHandle));
 
 done:
     LOG_EXIT();