[Windows] Cannot write to hosts with elevation/sudo

[mikerockett]

I've just installed the Windows x64 0.2.0 version of hostess (Windows 10), and it states it can't write to my hosts file, even when using sudo or an elevated console.

Any ideas as to what could be causing this?

PS: I've created a scoop app for this in my bucket.

[mikerockett]

Not all that familiar with Go, but I've managed to get as far as showing the actual error behind hostess not being able to write to the hosts file:

$ go run cmd/hostess/hostess.go fix
open C:\Windows\System32\drivers\etc\hosts: The parameter is incorrect.
exit status 1

After changing line 77 of commands.go to:

MaybeError(c, err.Error())

[mikerockett]

Have toyed around with this - again, not terribly familar with the language.

Lines 192+ of hostfile.go:

// Save writes the Hostfile to disk to /etc/hosts or to the location specified
// by the HOSTESS_PATH environment variable (if set). Due to Windows file locking,
// it's best to truncate the file before opening it for writing.
func (h *Hostfile) Save() error {
	err := os.Truncate(h.Path, 0);
	if err != nil {
		return err
	}

	file, err := os.OpenFile(h.Path, os.O_RDWR|os.O_APPEND, 0644)
	if err != nil {
		return err
	}

	defer file.Close()
	_, err = file.Write(h.Format())

	return err
}

This SO answer explains that Windows locks files, and that it would need to be truncated without an open handle. So the change above does just that.

Not sure if it impacts other platforms (we could do a runtime detection if it does) or anything else, and so I'm not submitting as a PR - please let me know if I should go ahead and do that.

[cbednarski]

@mikerockett Thanks for looking into this! The locking semantics you linked to seem a bit strange to me. This code worked on Windows 7 so I'm curious what has changed here in 10. I have not had a chance to look into this yet but I will take some time soon.

Did the fix you suggested work for you? It seems reasonable that this can work on any OS but if not we can put it behind a platform build flag. If you're not familiar with Go this can be a bit complicated but if you want to take a stab at it there is some information here.

[mikerockett]

@cbednarski - You're welcome. The suggested fix does indeed work. However, I think W10 has changed something somewhere because my hosts file wasn't actually working at after hostess converted it to the hostess-format. Where multiple hosts are defined on a single line (127.0.0.1 host1 host2), only host1 would load. Because I was in a rush to fix something on a dev-site, I reverted to the original one-host-per-line format. Unable to check now as I'm on a different PC without my dev-stack.

I assume a platform build flag tells the compiler to build using specific code for a specific platform? (May sound silly to ask, but I haven't worked with non-runtime languages in a long time, having moved over to web-dev.)

[cbednarski]

The suggested fix does indeed work. However, I think W10 has changed something somewhere because my hosts file wasn't actually working at after hostess converted it to the hostess-format.

Oh also interesting. I may need to add a Windows-specific formatter in that case.

I assume a platform build flag tells the compiler to build using specific code for a specific platform? (May sound silly to ask, but I haven't worked with non-runtime languages in a long time, having moved over to web-dev.)

Correct. The Go compiler can be instructed to include a certain file only when compiling a Windows binary (for example). Simple example: docker_default.go has a build flag that says "only include this on not Windows" while docker_windows.go will only build when Go is compiling for Windows (based on the filename).

The other way to do this is to use a runtime check for the OS name. I do this currently to set the default host file path. If the implementation starts to get messy with an if block that's a good hint that the build flag is a better approach.

[mikerockett]

Thanks for the info - handy to know. If I garner some time, it would be a good idea for me to play around with Go - looks like a great language to work with.

The other way to do this is to use a runtime check for the OS name. I do this currently to set the default host file path. If the implementation starts to get messy with an if block that's a good hint that the build flag is a better approach.

I'd naturally think a build-flag is better for this case. Considering the potential complexity here, you'd be far better off than me in doing a platform-specific change here. Having briefly looked at the code, I can imagine that changing the formatter for Windows is perhaps not as simple as a one-line-amendment...

[tdaniely]

This worked for me and shouldn't erase the file if the process crashes, assuming truncates actually calls SetEndOfFile, which I can't tell, because I'm only seeing posix calls in go.
I'm not sure, but as I understand it's the append flag in windows that has special requirements on the sharemode.
This kind of e_access_denied in windows is usually not a real lock.

func (h *Hostfile) Save() error {
	file, err := os.OpenFile(h.Path, os.O_RDWR, 0644)
	if err != nil {
		return err
	}
	defer file.Close()

	err = file.Truncate(0)
	if err != nil {
		return err
	}

	_, err = file.Write(h.Format())

	return err
}

[cbednarski]

I took a slightly different approach and rewrote Save() to use an atomic update strategy instead of truncating the file. I have not had a chance to test it in various environments yet but theoretically this is the best approach.

https://github.com/cbednarski/hostess/compare/atomic-update

If you want to try this out yourself, let me know your results!

go get -u github.com/cbednarski/hostess/...
cd $GOPATH/src/github.com/cbednarski/hostess
git checkout atomic-update
go test .
go install .
$GOPATH/bin/hostess fix

[cbednarski]

Where multiple hosts are defined on a single line (127.0.0.1 host1 host2), only host1 would load. Because I was in a rush to fix something on a dev-site, I reverted to the original one-host-per-line format. Unable to check now as I'm on a different PC without my dev-stack.

Btw I also looked into this (did some simple editing with Notepad.exe) and multiple hostnames on the same line were picked up OK. I tested with ping. If you have more info about this let me know!

[tdaniely]

I tried the atomic-update branch.

When the file is replaced the permissions change and the whole thing breaks.
To make the hosts file work again I went to the advanced security tab on the file. Enabled inherited permissions, removed the files own permissions, and then flush the dns cache to trigger an update.

Probably replacing the file breaks the watch on the file. and without the correct permission set, whatever is watching the file doesn't watch the new file.

I don't think replacing the file will work because the system has a watcher with an open handle, so even if the permissions are set correctly it will probably still loose the watcher handle until the next flush or cache timeout.

Edit
This post https://serverfault.com/a/452269/448794 suggests that copy instead of move will work.

[cbednarski]

@tdaniely Thanks for testing and following up! Your feedback is super helpful. The permissions thing is something I had not considered so I will likely need to add some platform-specific code here for Unix vs. Windows. I will try to get to this in the next few days.

[cbednarski]

Thanks all for the reports and suggestions. This fix is now live on master and tagged in 0.4.1. Sorry this was open for so long. 😬