[
https://issues.apache.org/jira/browse/YARN-2198?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14166718#comment-14166718
]
Remus Rusanu commented on YARN-2198:
------------------------------------
> libwinutils.c CreateLogonForUser - confusing name, makes me think a new- >
> CreateLogonTokenForUser?
RR: Fixed
> TestWinUtils - can we add testing specific to security?
RR: Tracked by YARN-2636
> ContainerLaunch launchContainer - nit, why "userName" here, it's user
> everywhere else
RR fixed
> getLocalWrapperScriptBuilder - why not an override instead of conditional
> (see below wrt WindowsContainerExecutor)
>WindowsSecureContainerExecutor - I really think there should be a
>"WindowsContainerExecutor"
RR: I left both as is, predates the WSCE
> 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?
RR: correct. x86 was not possible to build from mvn, and was not required.
> 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?
RR: I actually intentionally avoided that. The LCE does it, and the result is a
lot of duplication between Java code in Default Container Executor and C code
in the native container-executor. With the WSCE I preferred to keep the logic
in Java and use native methods just for primitive operations.
>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?
RR: I added some more comments to clarify that only a process kill or hardware
error can fail after this point. An atomic transfer is not possible.
> why is this conditional check different from all the others?
RR: fixed
> nit anonimous sp anonymous
RR: fixed
> just a line added, pls revert
RR: fixed
> 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?
RR: It is not possible under a correct configured deployment. Explicit
overwriting permissions can deny this, but that will always be possible (eg.
deny permission explicitly to LocalSystem).
> 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
RR: I forgot to address this. Todo.
> ContainerExecutor comment on comment:
RR: Fixed
> ContainerLaunch public void sanitizeEnv(...)
RR: This predates WSCE, I left it as is
> ContainerLocalizer LOG.info(String.format("nRet: %d", nRet)); - not sure this
> should be "info" level
RR: todo, forgot to address it
> getContainerClasspathJarPrivateDir not used in ContainerExecutor.java, we can
> remove that.
RR: fixed
> Unnecessary format change only in YarnConfiguration.we can revert
RR: fixed
> Multiple places exceed 80 column limit code convention.
RR: I think I fixed all new Java code
> DefaultContainerExecutor#buildCommandExecutor
RR: Fixed
> 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.11.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)
