Skip to content

Commit

Permalink
Remove write access (not read, execute) for BUILTIN\Users (#579)
Browse files Browse the repository at this point in the history
We were removing all access rights for `BUILTIN\Users`, which prevents the user from using the `iotedge` CLI unless they're running as admin. This change to the Windows installer script removes write access to install folders, but preserves read and execute access.

This change also updates the docker URL in various places to the new default (replace `docker_engine` with `iotedge_moby_engine`).
  • Loading branch information
damonbarry authored Nov 29, 2018
1 parent 58b4380 commit d6b8c3a
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 21 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"dockerHostUrl": "npipe://./pipe/docker_engine",
"dockerHostUrl": "npipe://./pipe/iotedge_moby_engine",
"testSuite": [
"test-config-windows-x64.json"
]
Expand Down
2 changes: 1 addition & 1 deletion edgelet/contrib/config/windows/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -163,5 +163,5 @@ homedir: "C:\\ProgramData\\iotedge"
###############################################################################

moby_runtime:
uri: "npipe://./pipe/docker_engine"
uri: "npipe://./pipe/iotedge_moby_engine"
# network: "nat"
16 changes: 7 additions & 9 deletions edgelet/hyper-named-pipe/src/uri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,18 +149,17 @@ mod tests {

#[test]
fn url_path() {
let url = Uri::new("npipe://./pipe/docker_engine", "/containers/json?all=true").unwrap();
let url = Uri::new("npipe://./pipe/boo", "/containers/json?all=true").unwrap();
assert_eq!(url.url.path(), "/containers/json");
assert_eq!(url.url.query(), Some("all=true"));
}

#[test]
fn hyper_uri() {
let uri: HyperUri = Uri::new("npipe://./pipe/docker_engine", "/containers/json?all=true")
let uri: HyperUri = Uri::new("npipe://./pipe/boo", "/containers/json?all=true")
.unwrap()
.into();
let expected =
"npipe://5c5c2e5c706970655c646f636b65725f656e67696e65/containers/json?all=true";
let expected = "npipe://5c5c2e5c706970655c626f6f/containers/json?all=true";
assert_eq!(uri, expected.parse::<HyperUri>().unwrap());
}

Expand All @@ -184,16 +183,15 @@ mod tests {

#[test]
fn uri_host() {
let uri: HyperUri =
"npipe://5c5c2e5c706970655c646f636b65725f656e67696e65/containers/json?all=true"
.parse()
.unwrap();
let uri: HyperUri = "npipe://5c5c2e5c706970655c626f6f/containers/json?all=true"
.parse()
.unwrap();
assert_eq!(
&Uri::get_pipe_path_from_parts(
uri.scheme_part().unwrap().as_str(),
uri.host().unwrap()
).unwrap(),
"\\\\.\\pipe\\docker_engine"
"\\\\.\\pipe\\boo"
);
}
}
2 changes: 1 addition & 1 deletion edgelet/iotedged/src/config/windows/default.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,5 @@ listen:
homedir: "C:\\ProgramData\\iotedge"

moby_runtime:
uri: "npipe://./pipe/docker_engine"
uri: "npipe://./pipe/iotedge_moby_engine"
network: "nat"
23 changes: 14 additions & 9 deletions scripts/windows/setup/IotEdgeSecurityDaemon.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ function Get-SecurityDaemon {

Write-Host 'Downloading the latest version of Moby runtime...'
New-Item -Type Directory $MobyInstallDirectory | Out-Null
Remove-Permissions $MobyInstallDirectory
Remove-BuiltinWritePermissions $MobyInstallDirectory
Invoke-WebRequest `
-Uri 'https://aka.ms/iotedge-moby-engine-win-amd64-latest' `
-OutFile $mobyRuntimeArchivePath `
Expand All @@ -332,7 +332,7 @@ function Get-SecurityDaemon {
Expand-Archive $mobyRuntimeArchivePath $MobyInstallDirectory -Force

New-Item -Type Directory $MobyDataRootDirectory | Out-Null
Remove-Permissions $MobyDataRootDirectory
Remove-BuiltinWritePermissions $MobyDataRootDirectory

if ($WithMobyCli) {
Write-Host 'Downloading the latest version of Moby CLI...'
Expand Down Expand Up @@ -368,7 +368,7 @@ function Get-SecurityDaemon {
Copy-Item "$EdgeInstallDirectory\iotedged-windows\*" $EdgeInstallDirectory -Force -Recurse
}

Remove-Permissions $EdgeInstallDirectory
Remove-BuiltinWritePermissions $EdgeInstallDirectory

foreach ($name in 'mgmt', 'workload') {
# We can't bind socket files directly in Windows, so create a folder
Expand All @@ -386,7 +386,7 @@ function Get-SecurityDaemon {

New-Item -Type Directory $EdgeEventLogInstallDirectory -ErrorAction SilentlyContinue -ErrorVariable cmdErr | Out-Null
if ($? -or ($cmdErr.FullyQualifiedErrorId -eq 'DirectoryExist,Microsoft.PowerShell.Commands.NewItemCommand')) {
Remove-Permissions $EdgeEventLogInstallDirectory
Remove-BuiltinWritePermissions $EdgeEventLogInstallDirectory
Move-Item `
"$EdgeInstallDirectory\iotedged_eventlog_messages.dll" `
"$EdgeEventLogInstallDirectory\iotedged_eventlog_messages.dll" `
Expand Down Expand Up @@ -866,16 +866,21 @@ function Write-HostRed {
Write-Host -ForegroundColor Red @args
}

function Remove-Permissions([string] $Path) {
function Remove-BuiltinWritePermissions([string] $Path) {
$user = 'BUILTIN\Users'
Write-Verbose "Remove $user permission to $Path"
Invoke-Native "icacls ""$Path"" /inheritance:d"

$acl = Get-Acl -Path $Path
foreach ($access in $acl.Access) {
if ($access.IdentityReference.Value -eq $user)
{
$acl.RemoveAccessRule($access) | Out-Null
$write = [System.Security.AccessControl.FileSystemRights]::Write
foreach ($access in $acl.Access) {
if ($access.IdentityReference.Value -eq $user -and
$access.AccessControlType -eq 'Allow' -and
($access.FileSystemRights -band $write) -eq $write)
{
$rule = New-Object -TypeName System.Security.AccessControl.FileSystemAccessRule(`
$user, 'Write', $access.InheritanceFlags, $access.PropagationFlags, 'Allow')
$acl.RemoveAccessRule($rule) | Out-Null
}
}
Set-Acl -Path $Path -AclObject $acl
Expand Down

0 comments on commit d6b8c3a

Please # to comment.