-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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.
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
Include Windows OS in Bootstrap initializeNatives() check for definitelyRunningAsRoot() #4656
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
45f999e
Bump jna from 5.11.0 to 5.12.1
dbwiddis 50cd0be
Include Windows OS in check for definitelyRunningAsRoot()
dbwiddis 8b35d6d
Combine try/catch blocks
dbwiddis 96473ce
Someone sniped my PR number between predicting and pushing
dbwiddis 02a4a21
Update SHAs
dbwiddis 5e25c87
Merge branch 'main' of github.com:opensearch-project/OpenSearch into
dbwiddis File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
b1e93a735caea94f503e95e6fe79bf9cdc1e985d |
This file was deleted.
Oops, something went wrong.
124 changes: 124 additions & 0 deletions
124
server/src/main/java/org/opensearch/bootstrap/JNAAdvapi32Library.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,124 @@ | ||
/* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
* | ||
* The OpenSearch Contributors require contributions made to | ||
* this file be licensed under the Apache-2.0 license or a | ||
* compatible open source license. | ||
*/ | ||
|
||
package org.opensearch.bootstrap; | ||
|
||
import com.sun.jna.Native; | ||
import com.sun.jna.Pointer; | ||
import com.sun.jna.ptr.IntByReference; | ||
import com.sun.jna.ptr.PointerByReference; | ||
import com.sun.jna.Structure; | ||
import org.apache.logging.log4j.LogManager; | ||
import org.apache.logging.log4j.Logger; | ||
import org.apache.lucene.util.Constants; | ||
|
||
import java.util.List; | ||
|
||
/** | ||
* Library for Windows/Advapi32 | ||
* | ||
* @opensearch.internal | ||
*/ | ||
final class JNAAdvapi32Library { | ||
|
||
private static final Logger logger = LogManager.getLogger(JNAAdvapi32Library.class); | ||
|
||
private static final class Holder { | ||
private static final JNAAdvapi32Library instance = new JNAAdvapi32Library(); | ||
} | ||
|
||
private JNAAdvapi32Library() { | ||
if (Constants.WINDOWS) { | ||
try { | ||
Native.register("advapi32"); | ||
logger.debug("windows/Advapi32 library loaded"); | ||
} catch (NoClassDefFoundError e) { | ||
logger.warn("JNA not found. native methods and handlers will be disabled."); | ||
} catch (UnsatisfiedLinkError e) { | ||
logger.warn("unable to link Windows/Advapi32 library. native methods and handlers will be disabled."); | ||
} | ||
} | ||
} | ||
|
||
static JNAAdvapi32Library getInstance() { | ||
return Holder.instance; | ||
} | ||
|
||
/** | ||
* Access right required to query an access token. | ||
* Used by {@link #OpenProcessToken(Pointer, int, PointerByReference)}. | ||
* | ||
* https://learn.microsoft.com/en-us/windows/win32/secauthz/access-rights-for-access-token-objects | ||
*/ | ||
public static final int TOKEN_QUERY = 0x0008; | ||
|
||
/** | ||
* TOKEN_INFORMATION_CLASS enumeration value that specifies the type of information being assigned to or retrieved from an access token. | ||
* Used by {@link #GetTokenInformation(Pointer, int, Structure, int, IntByReference)}. | ||
* | ||
* https://learn.microsoft.com/en-us/windows/win32/api/winnt/ne-winnt-token_information_class | ||
*/ | ||
public static final int TOKEN_ELEVATION = 0x14; | ||
|
||
/** | ||
* Native call to the Advapi32 API to open the access token associated with a process. | ||
* | ||
* @param processHandle Handle to the process whose access token is opened. | ||
* The process must have the PROCESS_QUERY_INFORMATION access permission. | ||
* @param desiredAccess Specifies an access mask that specifies the requested types of access to the access token. | ||
* These requested access types are compared with the discretionary access control list (DACL) of the token to determine which accesses are granted or denied. | ||
* @param tokenHandle Pointer to a handle that identifies the newly opened access token when the function returns. | ||
* @return If the function succeeds, the return value is true. | ||
* If the function fails, the return value is false. | ||
* To get extended error information, call GetLastError. | ||
*/ | ||
native boolean OpenProcessToken(Pointer processHandle, int desiredAccess, PointerByReference tokenHandle); | ||
|
||
/** | ||
* Retrieves a specified type of information about an access token. | ||
* The calling process must have appropriate access rights to obtain the information. | ||
* | ||
* @param tokenHandle Handle to an access token from which information is retrieved. | ||
* If TokenInformationClass specifies TokenSource, the handle must have TOKEN_QUERY_SOURCE access. | ||
* For all other TokenInformationClass values, the handle must have TOKEN_QUERY access. | ||
* @param tokenInformationClass Specifies a value from the TOKEN_INFORMATION_CLASS enumerated type to identify the type of information the function retrieves. | ||
* @param tokenInformation Pointer to a buffer the function fills with the requested information. | ||
* The structure put into this buffer depends upon the type of information specified by the TokenInformationClass parameter. | ||
* @param tokenInformationLength Specifies the size, in bytes, of the buffer pointed to by the TokenInformation parameter. | ||
* If TokenInformation is NULL, this parameter must be zero. | ||
* @param returnLength Pointer to a variable that receives the number of bytes needed for the buffer pointed to by the TokenInformation parameter. | ||
* If this value is larger than the value specified in the TokenInformationLength parameter, the function fails and stores no data in the buffer. | ||
* @return If the function succeeds, the return value is true. | ||
* If the function fails, the return value is zero. | ||
* To get extended error information, call GetLastError. | ||
*/ | ||
native boolean GetTokenInformation( | ||
Pointer tokenHandle, | ||
int tokenInformationClass, | ||
Structure tokenInformation, | ||
int tokenInformationLength, | ||
IntByReference returnLength | ||
); | ||
|
||
/** | ||
* The TOKEN_ELEVATION structure indicates whether a token has elevated privileges. | ||
* | ||
* https://learn.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-token_elevation | ||
*/ | ||
public static class TokenElevation extends Structure { | ||
/** | ||
* A nonzero value if the token has elevated privileges; otherwise, a zero value. | ||
*/ | ||
public int TokenIsElevated; | ||
|
||
@Override | ||
protected List<String> getFieldOrder() { | ||
return List.of("TokenIsElevated"); | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why aren't we using the JNA advapi32 wrapper?
https://github.com/java-native-access/jna/blob/030411b909d5dfd249b1df09a7f24c44babcae64/contrib/platform/src/com/sun/jna/platform/win32/Advapi32.java#L701
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we are only using the
jna
dependency, notjna-platform
. See also theJNAKernel32Library
andJNACLibrary
classes which could be eliminated.jna-platform
is a 1.3MB JAR and also introduces a modular transitive dependency onjava.desktop
under JPMS. Happy to switch the code to use this added dependency, but given the very limited use of library mappings, I don't think it's needed (yet).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't love copy-pasting code from 3P libraries to save 1.3MB because that's how we miss fixes in the library, but I don't have a strong opinion about this. I'll hit approve on this PR and we can fix this later. How do you feel about this issue @nknize?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's more likely with util-based functions, but these mappings are rather straightforward and I actually researched the original API reference to confirm correctness for these. I do admit to copying the javadocs (derived from Win32API) from from some guy named @dblock and I don't think he made a mistake. Your opinion of him may vary though. 😁
More seriously, though, there's a point where there are too many native mappings and we should switch to a dependency. I don't think we're there yet (three classes with a handful of functions in them) but we are probably at least halfway there.