Added additional security functions

[amarcionek]

Added to Advapi32: SetFileSecurity, GetSecurityInfo and SetSecurityInfo
Added to NtDll: NtSetSecurityObject and NtQuerySecurityObject

[matthiasblaesing]

Thank you for your work - I had a look at the changes and judging by my limited knowledge of the win32 api this looks good - however I did not yet merge the changes, as I had problems running the unittests.

The tests were run on Windows 10 64 Bit on a Oracle 8u92 jdk. The locale of the system and account is german. The errors were extracted from the html output of the unittests.

Run as administrator

testGetSetFileSecurityNoSACL

SetFileSecurity(C:\Users\matthias\AppData\Local\Temp\3043101914889.text) expected: but was:

junit.framework.AssertionFailedError: SetFileSecurity(C:\Users\matthias\AppData\Local\Temp\3043101914889.text) expected: but was:
at com.sun.jna.platform.win32.Advapi32Test.testGetSetFileSecurityNoSACL(Advapi32Test.java:1027)

Run as user

testGetSetSecurityInfoForFileWithSACL
GetSecurityInfo(C:\Users\matthias\AppData\Local\Temp\3532430559659.text) expected:<0> but was:<1314>

junit.framework.AssertionFailedError: GetSecurityInfo(C:\Users\matthias\AppData\Local\Temp\3532430559659.text) expected:<0> but was:<1314>
at com.sun.jna.platform.win32.Advapi32Test.testGetSetSecurityInfoForFileWithSACL(Advapi32Test.java:1161)

testSetNamedSecurityInfoForFileWithSACL
GetNamedSecurityInfo(C:\Users\matthias\AppData\Local\Temp\3534604437538.text) expected:<0> but was:<1314>

junit.framework.AssertionFailedError: GetNamedSecurityInfo(C:\Users\matthias\AppData\Local\Temp\3534604437538.text) expected:<0> but was:<1314>
at com.sun.jna.platform.win32.Advapi32Test.testSetNamedSecurityInfoForFileWithSACL(Advapi32Test.java:1412)

testGetNamedSecurityInfoForFileWithSACL
GetNamedSecurityInfo(C:\Users\matthias\AppData\Local\Temp\3536673711025.text) expected:<0> but was:<1314>

junit.framework.AssertionFailedError: GetNamedSecurityInfo(C:\Users\matthias\AppData\Local\Temp\3536673711025.text) expected:<0> but was:<1314>
at com.sun.jna.platform.win32.Advapi32Test.testGetNamedSecurityInfoForFileWithSACL(Advapi32Test.java:1284)

testGetSetFileSecurityNoSACL
SetFileSecurity(C:\Users\matthias\AppData\Local\Temp\3537592487457.text) expected: but was:

junit.framework.AssertionFailedError: SetFileSecurity(C:\Users\matthias\AppData\Local\Temp\3537592487457.text) expected: but was:
at com.sun.jna.platform.win32.Advapi32Test.testGetSetFileSecurityNoSACL(Advapi32Test.java:1027)

Could you please have a look at this?

[matthiasblaesing]

I rebased this changeset onto #686 and added a GC protection in testGetSetFileSecurityNoSACL (synchronized(memorySecurity) {} at the end). All in my current run on x64 resulted in:

546 tests passed, 1 test failed, 7 tests skipped

The one failing tests looks like a timing problem. As #686 is based on an issue open by yourself, I'd like to ask you to have a look at it and be prepared to rebase this changeset on top of that.

Thank you!

[amarcionek]

I looked at #686 in with a focus on the Security functions and it looks good. I also made one comment. I'll rebase this when its submitted.

Also, testGetSetFileSecurityNoSACL I think was failing due with ERROR_NOACCESS and was due to the memorySecurity.getPointer(0) argument. memorySecurity is itself already a pointer, so just passing that in seemed to fix it. I'll add that here.

Finally, on the 'Run as User' tests, I think what's happening here is that your user account doesn't already have the SE_SECURITY_NAME and SE_RESTORE_NAME privileges. I was able to reproduce this as well if I didn't Run As Administrator (even though my account IS a local administrator.) While AdjustTokenPrivileges returns true, checking last error after calling it returns ERROR_NOT_ALL_ASSIGNED, which is part of the API defined behavior. I don't think there is a way around this because a user account cannot add privileges to a process that do not already exist, it can only enable or disable existing ones.

Let me know if there is anything else you want me to look into, otherwise I'll monitor 686 for the merge.

[matthiasblaesing]

Could ppDacl and ppSacl replaced with WinNT.ACL?

[matthiasblaesing]

Thanks - I added a few comments inline. They come mostly down to: Would it be more sensible to use the already defined structures (WinNT.ACL, WinNT.PSID, WinNT.SECURITY_DESCRIPTOR) for the method signatures.

Apart from one test failure (testGetSetFileSecurityNoSACL) the current iteration run through cleanly. The failing test failed with error code (GetLastError) of 998 - ERROR_NOACCESS. See inline comment.

[matthiasblaesing]

@amarcionek: Ok - I had a quick test and please ignore the hints to use the present structures - that does not work. None the less I had to fix two issues to make this work on 64bit and 32bit. With the three changes below I have the tests passing on 32bit and 64bit oracle VM.

Here the wrong pointer is passed to native side:

--- a/contrib/platform/test/com/sun/jna/platform/win32/Advapi32Test.java
+++ b/contrib/platform/test/com/sun/jna/platform/win32/Advapi32Test.java
@@ -1030,7 +1030,7 @@
                          Advapi32.INSTANCE.SetFileSecurity(
                                  filePath,
                                  infoType,
-                                 memorySecurity.getPointer(0)));
+                                 memorySecurity));
         } finally {
             file.delete();
         }

ULONGs are defined to be 32bit wide - they are mapped to java integers (this works on 64bit, but fails on 32bit):

--- a/contrib/platform/src/com/sun/jna/platform/win32/NtDll.java
+++ b/contrib/platform/src/com/sun/jna/platform/win32/NtDll.java
@@ -104,5 +104,5 @@
      * @return
      *  NtQuerySecurityObject returns STATUS_SUCCESS or an appropriate error status.
      */
-    public int NtQuerySecurityObject(HANDLE handle, int SecurityInformation, Pointer SecurityDescriptor, long Length, LongByReference LengthNeeded);
+    public int NtQuerySecurityObject(HANDLE handle, int SecurityInformation, Pointer SecurityDescriptor, int Length, IntByReference LengthNeeded);
 }

--- a/contrib/platform/test/com/sun/jna/platform/win32/NtDllTest.java
+++ b/contrib/platform/test/com/sun/jna/platform/win32/NtDllTest.java
@@ -81,9 +81,9 @@
                         null);
             assertFalse("Failed to create file handle: " + filePath, WinBase.INVALID_HANDLE_VALUE.equals(hFile));

-            long Length = 64 * 1024;
+            int Length = 64 * 1024;
             Memory SecurityDescriptor = new Memory(Length);
-            LongByReference LengthNeeded = new LongByReference();
+            IntByReference LengthNeeded = new IntByReference();

             assertEquals("NtQuerySecurityObject(" + filePath + ")", 0,
                     NtDll.INSTANCE.NtQuerySecurityObject(
@@ -92,6 +92,11 @@
                             SecurityDescriptor,
                             Length,
                             LengthNeeded));
+            
+            assertTrue(LengthNeeded.getValue() > 0);
+            
+            assertTrue(LengthNeeded.getValue() < 64 * 1024);
+
             assertEquals("NtSetSecurityObject(" + filePath + ")", 0,
                     NtDll.INSTANCE.NtSetSecurityObject(
                             hFile,

[amarcionek]

Changes made. Thanks for reviewing them. As you likely know, I did have a hard time implementing the structure based versions of the functions. I don't think many will work because some of them are indirect pointers into other structures. There probably is a way to do some of the setters as structures, but the getters are harder. I ended up modeling after the existing security function definitions such as GetFileSecurity and SetFileSecurity that use Pointer and PointerByReference.

[matthiasblaesing]

Thank you - I squashed the changesets together and merged that into master.