Skip to content
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

Jump to bottom

cleaned security sensitive data after usage #1366

Merged
merged 1 commit into from
Aug 10, 2021
Merged

cleaned security sensitive data after usage #1366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions CHANGES.md
View file
Open in desktop
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
View file
Open in desktop
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
155 changes: 155 additions & 0 deletions ...b/platform/test/com/sun/jna/platform/win32/Crypt32UtilClearSecuritySensitiveDataTest.java
View file
Open in desktop
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)));
}
}