Skip to content

Commit

Permalink
Merge pull request #1366 from dmytro-sheyko/bug/1362_cryptProtectData…
Browse files Browse the repository at this point in the history
…_clean

cleaned security sensitive data after usage
  • Loading branch information
matthiasblaesing committed Aug 10, 2021
2 parents 0f2f881 + 4b156fb commit 80105c3
Show file tree
Hide file tree
Showing 3 changed files with 199 additions and 17 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Bug Fixes
---------
* [#1343](https://github.com/java-native-access/jna/issues/1343): `c.s.j.p.mac.CoreFoundation.CFStringRef#stringValue` buffer needs space for a null byte - [@dbwiddis](https://github.com/dbwiddis).
* [#1351](https://github.com/java-native-access/jna/issues/1351): Define `c.s.j.p.unix.size_t.ByReference` and fix macOS sysctl `size_t *` parameters - [@dbwiddis](https://github.com/dbwiddis).
* [#1362](https://github.com/java-native-access/jna/issues/1362): Clear security sensitive data after usage in c.s.j.p.win32.Crypt32Util#cryptUnprotectData and #cryptUnprotectData - [@dmytro-sheyko](https://github.com/dmytro-sheyko).

Release 5.8.0
=============
Expand Down
60 changes: 43 additions & 17 deletions contrib/platform/src/com/sun/jna/platform/win32/Crypt32Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
*/
package com.sun.jna.platform.win32;

import java.util.Arrays;

import com.sun.jna.Pointer;
import com.sun.jna.Memory;
import com.sun.jna.Native;
Expand Down Expand Up @@ -80,17 +82,44 @@ public static byte[] cryptProtectData(byte[] data, byte[] entropy, int flags,
DATA_BLOB pDataIn = new DATA_BLOB(data);
DATA_BLOB pDataProtected = new DATA_BLOB();
DATA_BLOB pEntropy = (entropy == null) ? null : new DATA_BLOB(entropy);
Win32Exception err = null;
byte[] protectedData = null;
try {
if (! Crypt32.INSTANCE.CryptProtectData(pDataIn, description,
pEntropy, null, prompt, flags, pDataProtected)) {
throw new Win32Exception(Kernel32.INSTANCE.GetLastError());
err = new Win32Exception(Kernel32.INSTANCE.GetLastError());
} else {
protectedData = pDataProtected.getData();
}
return pDataProtected.getData();
} finally {
if (pDataIn.pbData != null) {
pDataIn.pbData.clear(pDataIn.cbData);
}
if (pEntropy != null && pEntropy.pbData != null) {
pEntropy.pbData.clear(pEntropy.cbData);
}
if (pDataProtected.pbData != null) {
Kernel32Util.freeLocalMemory(pDataProtected.pbData);
pDataProtected.pbData.clear(pDataProtected.cbData);
try {
Kernel32Util.freeLocalMemory(pDataProtected.pbData);
} catch(Win32Exception e) {
if (err == null) {
err = e;
} else {
err.addSuppressedReflected(e);
}
}
}
}

if (err != null) {
if (protectedData != null) {
Arrays.fill(protectedData, (byte) 0);
}
throw err;
}

return protectedData;
}

/**
Expand Down Expand Up @@ -135,18 +164,24 @@ public static byte[] cryptUnprotectData(byte[] data, byte[] entropy, int flags,
DATA_BLOB pDataIn = new DATA_BLOB(data);
DATA_BLOB pDataUnprotected = new DATA_BLOB();
DATA_BLOB pEntropy = (entropy == null) ? null : new DATA_BLOB(entropy);
PointerByReference pDescription = new PointerByReference();
Win32Exception err = null;
byte[] unProtectedData = null;
try {
if (! Crypt32.INSTANCE.CryptUnprotectData(pDataIn, pDescription,
if (! Crypt32.INSTANCE.CryptUnprotectData(pDataIn, null,
pEntropy, null, prompt, flags, pDataUnprotected)) {
err = new Win32Exception(Kernel32.INSTANCE.GetLastError());
} else {
unProtectedData = pDataUnprotected.getData();
}
} finally {
if (pDataIn.pbData != null) {
pDataIn.pbData.clear(pDataIn.cbData);
}
if (pEntropy != null && pEntropy.pbData != null) {
pEntropy.pbData.clear(pEntropy.cbData);
}
if (pDataUnprotected.pbData != null) {
pDataUnprotected.pbData.clear(pDataUnprotected.cbData);
try {
Kernel32Util.freeLocalMemory(pDataUnprotected.pbData);
} catch(Win32Exception e) {
Expand All @@ -157,21 +192,12 @@ public static byte[] cryptUnprotectData(byte[] data, byte[] entropy, int flags,
}
}
}

if (pDescription.getValue() != null) {
try {
Kernel32Util.freeLocalMemory(pDescription.getValue());
} catch(Win32Exception e) {
if (err == null) {
err = e;
} else {
err.addSuppressedReflected(e);
}
}
}
}

if (err != null) {
if (unProtectedData != null) {
Arrays.fill(unProtectedData, (byte) 0);
}
throw err;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
/* Copyright (c) 2021 Dmytro Sheyko, All Rights Reserved
*
* The contents of this file is dual-licensed under 2
* alternative Open Source/Free licenses: LGPL 2.1 or later and
* Apache License 2.0. (starting with JNA version 4.0.0).
*
* You can freely decide which license you want to apply to
* the project.
*
* You may obtain a copy of the LGPL License at:
*
* http://www.gnu.org/licenses/licenses.html
*
* A copy is also included in the downloadable source code package
* containing JNA, in file "LGPL2.1".
*
* You may obtain a copy of the Apache License at:
*
* http://www.apache.org/licenses/
*
* A copy is also included in the downloadable source code package
* containing JNA, in file "AL2.0".
*/
package com.sun.jna.platform.win32;

import java.lang.ref.WeakReference;
import java.lang.reflect.Field;
import java.util.Arrays;

import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ErrorCollector;

import com.sun.jna.Memory;

import static org.hamcrest.CoreMatchers.is;

/**
* https://github.com/java-native-access/jna/issues/1362
*/
public class Crypt32UtilClearSecuritySensitiveDataTest {
@Rule public ErrorCollector errors = new ErrorCollector();
Field fieldMemory_HEAD;
Field fieldLinkedReference_next;
Field fieldLinkedReference_prev;

@Before
public void setUp() throws NoSuchFieldException, ClassNotFoundException {
fieldMemory_HEAD = Memory.class.getDeclaredField("HEAD");
fieldMemory_HEAD.setAccessible(true);
Class<?> classLinkedReference = Class.forName("com.sun.jna.Memory$LinkedReference");
fieldLinkedReference_next = classLinkedReference.getDeclaredField("next");
fieldLinkedReference_next.setAccessible(true);
fieldLinkedReference_prev = classLinkedReference.getDeclaredField("prev");
fieldLinkedReference_prev.setAccessible(true);
}

boolean stillHover(byte[] sample) throws IllegalAccessException {
Object head = fieldMemory_HEAD.get(null);
return stillHover(sample, head, fieldLinkedReference_next) || stillHover(sample, head, fieldLinkedReference_prev);
}

boolean stillHover(byte[] sample, Object head, Field fieldNext) throws IllegalAccessException {
Object next = head;
do {
Object curr = next;
Memory memory = (Memory) ((WeakReference<?>) curr).get();
byte[] array = memory.getByteArray(0, (int) memory.size());
if (Arrays.equals(array, sample)) {
return true;
}
next = fieldNext.get(curr);
} while (next != null);
return false;
}

@After
public void tearDown() {
Memory.disposeAll();
}

@Test
public void testEncryption() throws IllegalAccessException {
byte[] original = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, };
Crypt32Util.cryptProtectData(original, null, WinCrypt.CRYPTPROTECT_UI_FORBIDDEN, "", null);

errors.checkThat("original is still hover", false, is(stillHover(original)));
}

@Test
public void testDecryption() throws IllegalAccessException {
byte[] original = { 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, };
byte[] encrypted = Crypt32Util.cryptProtectData(original, null, WinCrypt.CRYPTPROTECT_UI_FORBIDDEN, "", null);
Crypt32Util.cryptUnprotectData(encrypted, null, WinCrypt.CRYPTPROTECT_UI_FORBIDDEN, null);

errors.checkThat("original is still hover", false, is(stillHover(original)));
errors.checkThat("encrypted is still hover", false, is(stillHover(encrypted)));
}

@Test
public void testEncryptionWithEntropy() throws IllegalAccessException {
byte[] original = { 25, 26, 27, 28, 29, 30, };
byte[] entropy = { 21, 20, 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, };
Crypt32Util.cryptProtectData(original, entropy, WinCrypt.CRYPTPROTECT_UI_FORBIDDEN, "", null);

errors.checkThat("original is still hover", false, is(stillHover(original)));
errors.checkThat("entropy is still hover", false, is(stillHover(entropy)));
}

@Test
public void testDecryptionWithEntropy() throws IllegalAccessException {
byte[] original = { 31, 32, 33, 34, };
byte[] entropy = { 41, 40, 39, 38, 37, 36, 35, 34, 33, 32, 31, 30, 29, 28, 27, 26, 25, 24, 23, 22, };
byte[] encrypted = Crypt32Util.cryptProtectData(original, entropy, WinCrypt.CRYPTPROTECT_UI_FORBIDDEN, "", null);
Crypt32Util.cryptUnprotectData(encrypted, entropy, WinCrypt.CRYPTPROTECT_UI_FORBIDDEN, null);

errors.checkThat("original is still hover", false, is(stillHover(original)));
errors.checkThat("entropy is still hover", false, is(stillHover(entropy)));
errors.checkThat("encrypted is still hover", false, is(stillHover(encrypted)));
}

@Test
public void testUnsuccessfulDecryption() throws IllegalAccessException {
byte[] original = { 35, 36, 37, 38, 39, 40, 41, };
try {
Crypt32Util.cryptUnprotectData(original, null, WinCrypt.CRYPTPROTECT_UI_FORBIDDEN, null);
errors.addError(new AssertionError("Win32Exception is expected"));
} catch (Win32Exception e) {
// ok, expected
}

errors.checkThat("original is still hover", false, is(stillHover(original)));
}

@Test
public void testUnsuccessfulDecryptionBadEntropy() throws IllegalAccessException {
byte[] original = { 42, 43, };
byte[] entropy0 = { 44, 45, 46, };
byte[] entropy1 = { 47, 48, 49, 50, };
byte[] encrypted = Crypt32Util.cryptProtectData(original, entropy0, WinCrypt.CRYPTPROTECT_UI_FORBIDDEN, "", null);
try {
Crypt32Util.cryptUnprotectData(encrypted, entropy1, WinCrypt.CRYPTPROTECT_UI_FORBIDDEN, null);
errors.addError(new AssertionError("Win32Exception is expected"));
} catch (Win32Exception e) {
// ok, expected
}

errors.checkThat("original is still hover", false, is(stillHover(original)));
errors.checkThat("entropy0 is still hover", false, is(stillHover(entropy0)));
errors.checkThat("entropy1 is still hover", false, is(stillHover(entropy1)));
errors.checkThat("encrypted is still hover", false, is(stillHover(encrypted)));
}
}

0 comments on commit 80105c3

Please sign in to comment.