[
https://issues.apache.org/jira/browse/YARN-2198?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14153983#comment-14153983
]
Craig Welch commented on YARN-2198:
-----------------------------------
there are a number of changes which impact common multi-platform code, has this
been tested on non-Windows with security enabled (Linux) as well as windows?
it looks like this is only a 64 bit build now, where it used to be 64 and 32. I
assume this is intentional and ok?
It would be really nice if we could start to separate out some of this new
functionality from winutils, e.g., make the elevated service functionality
independent. I see that there is a jira for doing so down the road, which is
good... it looks like the elevated privilages are just around creating local
directories and (obviously) spawning the process. If a stand-alone service
just created and set permissions on those directories, and the java code simply
checked for their existance and then moved on if they were present, I think
that a lot of the back-and-forth of the elevation could be dropped to just one
call to create the base directory and a second to spawn/hand back the output
handles. Is that correct?
service.c
// We're now transfering ownership of the duplicated handles to the caller
+ // If the RPC call fails *after* this point the handles are leaked inside
the NM process
this is a little alarming. Doesn't the close() call clean this up, regardless
of success/ fail?
Have we done any profiling to make sure we're not leaking threads, thread
stacks, memory, etc, in at least the "happy case" (and preferably some "unhappy
cases" also)? I think we need to, there's a fair bit of additional native
code, and running it for a bit with a profiler could tell us quite a bit about
whether or not we may be leaking something...
why is this conditional check different from all the others?
+ dwError = ValidateConfigurationFile();
+ if (dwError) {
nit anonimous sp anonymous
hadoop-common-project/hadoop-common/src/main/native/src/org_apache_hadoop.h
just a line added, pls revert
ElevatedFileSystem
delete()
it appears that the tests for existance, etc, are run in a non-elevated way,
while the actions are elevated. Is it possible for permissions to be such that
the non-elevated tests do not see files/directories which are present for
permission reasons? should those not be elevated also?
streamReaderThread.run - using the readLine() instead of following the simple
buffer copy idiom in ShellCommandExecutor has some efficiency issues, granted
it looks to be reading "memory sized" data so it may be no big deal, but it
would be nice to follow the buffer-copy pattern instead
ContainerExecutor
comment on comment:
On Windows the ContainerLaunch creates a temporary empty jar to workaround the
CLASSPATH length
not exactly, it looks like it creates a jar with a "special manifest of other
jars", it would be helpful to explain that in the comment so it's clear what's
going on
ContainerLaunch
public void sanitizeEnv(...)
Can we please move the process of generating a new "reference jar" out of the
sanitizeEnv method into it's own method (called ?conditionally? after
sanitizeEnv)? While there's a clear connection in terms of it's "setting up
the environment", it's building a new jar & I think it is doing more than just
manipulating variables, so it belongs in a dedicated method, which can be
called in "call()" after sanitizeEnv I believe this also means that Path
nmPrivateClasspathJarDir can be pulled from the sanitizeEnv signature.
ContainerLocalizer
LOG.info(String.format("nRet: %d", nRet)); - not sure this should be "info"
level
> Remove the need to run NodeManager as privileged account for Windows Secure
> Container Executor
> ----------------------------------------------------------------------------------------------
>
> Key: YARN-2198
> URL: https://issues.apache.org/jira/browse/YARN-2198
> Project: Hadoop YARN
> Issue Type: Improvement
> Reporter: Remus Rusanu
> Assignee: Remus Rusanu
> Labels: security, windows
> Attachments: .YARN-2198.delta.10.patch, YARN-2198.1.patch,
> YARN-2198.2.patch, YARN-2198.3.patch, YARN-2198.delta.4.patch,
> YARN-2198.delta.5.patch, YARN-2198.delta.6.patch, YARN-2198.delta.7.patch,
> YARN-2198.separation.patch, YARN-2198.trunk.10.patch,
> YARN-2198.trunk.4.patch, YARN-2198.trunk.5.patch, YARN-2198.trunk.6.patch,
> YARN-2198.trunk.8.patch, YARN-2198.trunk.9.patch
>
>
> YARN-1972 introduces a Secure Windows Container Executor. However this
> executor requires the process launching the container to be LocalSystem or a
> member of the a local Administrators group. Since the process in question is
> the NodeManager, the requirement translates to the entire NM to run as a
> privileged account, a very large surface area to review and protect.
> This proposal is to move the privileged operations into a dedicated NT
> service. The NM can run as a low privilege account and communicate with the
> privileged NT service when it needs to launch a container. This would reduce
> the surface exposed to the high privileges.
> There has to exist a secure, authenticated and authorized channel of
> communication between the NM and the privileged NT service. Possible
> alternatives are a new TCP endpoint, Java RPC etc. My proposal though would
> be to use Windows LPC (Local Procedure Calls), which is a Windows platform
> specific inter-process communication channel that satisfies all requirements
> and is easy to deploy. The privileged NT service would register and listen on
> an LPC port (NtCreatePort, NtListenPort). The NM would use JNI to interop
> with libwinutils which would host the LPC client code. The client would
> connect to the LPC port (NtConnectPort) and send a message requesting a
> container launch (NtRequestWaitReplyPort). LPC provides authentication and
> the privileged NT service can use authorization API (AuthZ) to validate the
> caller.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
