[Unprivileged] Windows enablement #4264

blakerouse · 2024-02-14T20:54:05Z

What does this PR do?

Enables the installation mode of --unprivileged for Windows. During installation the elastic-agent-user user and elastic-agent group is created, permissions are set correctly on the copied files, service is created running as the elastic-agent-user. While running in unprivileged mode the daemon creates the vault with the correct permissions and gives users in the elastic-agent group the ability to communicate with the running Elastic Agent daemon.

Why is it important?

Allowing users to install the Elastic Agent with less permissions.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
~~[ ] I have made corresponding changes to the documentation~~
~~[ ] I have made corresponding change to the default configuration files~~
~~[ ] I have added tests that prove my fix is effective or that my feature works~~ (cannot test with unit tests)
~~[ ] I have added an entry in ./changelog/fragments using the changelog tool~~ (skipped as we don't want to expose this feature until ready)
I have added an integration test or an E2E test

Author's Checklist

elastic-agent-user is created
elastic-agent group is created
elastic-agent-user is added to elastic-agent group
file permission/owner is set for elastic-agent-user and elastic-agent group
elastic-agent runs correctly

How to test this PR locally

$ .\elastic-agent.exe install -f --unprivileged

Add your local user to elastic-agent group. Be sure to re-open command prompt. Run C:\Program Files\Elastic\Agent\elastic-agent.exe status.

Related issues

Closes [Unprivileged] Enable --unprivileged on Windows #3868

mergify · 2024-02-14T20:54:52Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b unprivileged-windows upstream/unprivileged-windows
git merge upstream/main
git push upstream unprivileged-windows

mergify · 2024-02-22T20:47:52Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b unprivileged-windows upstream/unprivileged-windows
git merge upstream/main
git push upstream unprivileged-windows

elasticmachine · 2024-03-21T12:55:43Z

Pinging @elastic/elastic-agent (Team:Elastic-Agent)

control_v2.proto

leehinman

just documentation requests.

leehinman · 2024-03-21T21:37:29Z

internal/pkg/agent/application/info/svc_windows.go


-	t, err := windows.OpenCurrentProcessToken()
+	token := windows.Token(0)


can you add a comment on why the switch to Token(0) ? We have had so many issues with detecting "admin" I'd just like to be sure we have this documented if things pop up later.

windows.Token(0) is used because its passed into checkTokenMembership that when the token is zero that check uses the current process token. It is a simpler way of saying this processes current token, without the need to grab a handle that needs to be closed for the current process.

What do you want documented here? That windows.Token(0) means the current process token?

What do you want documented here? That windows.Token(0) means the current process token?

Probably a few links to MSDN documentation, and just a comment that windows.Token(0) is current process. I'm just thinking in 3 years if we have a bug around this code in some new Azure environment it is nice to know what we were thinking and hopefully pointers to full documentation which might give clues if it has been deprecated etc.

Added some more comments and a link for the windows.Token(0).

internal/pkg/agent/cmd/reexec_windows.go

leehinman · 2024-03-21T22:04:06Z

internal/pkg/agent/install/svc.go

+		"OnFailureResetPeriod":   10,
+	}
+	if opts.Password != "" {
+		option["Password"] = opts.Password


I'm pretty sure this password is stored encrypted in the Registry. We need to document how the user would go about changing it. It's not uncommon for password policies to require changing static passwords like this on a regular basis.

I don't know if that is true in this case. The user cannot login at all, it can only be used as a service and the password is setup to never expire.

I don't know if that is true in this case.

See things like: https://www.stigviewer.com/stig/windows_server_2019/2020-06-15/finding/V-93209

That is interesting finding, but even in this case the installing administrator doesn't even know the password. Being the finding is speaking about an administrator knowing the password, and in this case the password is completely unknown.

We can definitely add documentation about it for unprivileged installation on Windows. If they choose to change the user account password they will also have to update the password for the service.

pchila

Code looks good, no major blockers.
Left a few comments that are mostly for readability/documentation, those don't fundamentally change the implementation

internal/pkg/agent/cmd/reexec_windows.go

internal/pkg/agent/install/install.go

internal/pkg/agent/install/install_windows.go

internal/pkg/agent/install/user_windows.go

pkg/utils/perm_windows.go

testing/integration/install_unix_test.go

leehinman

need to use crypto/rand not math/rand for generating random passwords.

Other than that, LGTM.

blakerouse · 2024-04-03T19:49:16Z

@leehinman Okay I have updated to crypto/rand.

elastic-sonarqube · 2024-04-03T20:17:38Z

Quality Gate failed

Failed conditions

7.7% 7.7% Coverage on New Code (required ≥ 40%)

See analysis details on SonarQube

pierrehilbert · 2024-04-04T12:55:36Z

What can be tested is tested and additional tests will require this to be merged first.
I'm forcing the merge even if the SonarQube requirement is red.

blakerouse added 11 commits February 8, 2024 20:52

Work on creating user/group on windows.

a74cf31

Allow install on Windows.

5b59d62

Set correct permissions on install.

cd0de99

Remove the FreeSID causing crash.

a72e02a

Return correct errors for FindGID and FindUID.

322c363

Add SeRestorePrivilege.

5523eff

Set the password for the service.

da71060

Un-comment the uninstall on error.

685f3d3

Set the local security policy for the created user.

6dd742b

Fix set password to use a specific struct.

f65da0d

Fix vault and listener permissions.

4aab797

blakerouse added Team:Elastic-Agent backport-skip labels Feb 14, 2024

blakerouse self-assigned this Feb 14, 2024

blakerouse added 4 commits February 14, 2024 15:57

Update notice.

6ec63f2

Merge branch 'main' into unprivileged-windows

94cf61f

Fix deps.

c57d4fa

Merge branch 'main' into unprivileged-windows

df4e2fb

blakerouse added the skip-changelog label Feb 14, 2024

blakerouse added 5 commits February 15, 2024 08:56

Fix lint.

6aaebc3

Fix windows lint.

b4c3e6d

Fix windows lint.

8c7ed65

Adjust integration tests for allowing unprivileged on Windows.

733c443

Adjust more tests.

0840e4c

blakerouse added 4 commits March 7, 2024 09:19

Merge branch 'main' into unprivileged-windows

c3943e2

Fix test.

83af91c

Fix user create.

909d48a

More work on Windows.

de92fb7

blakerouse requested review from michalpristas and pchila March 21, 2024 12:55

pchila reviewed Mar 21, 2024

View reviewed changes

control_v2.proto Outdated Show resolved Hide resolved

blakerouse added 2 commits March 21, 2024 11:23

Switch from privileged to unprivileged in control protocol.

131b24f

Merge branch 'main' into unprivileged-windows

60a9835

pierrehilbert requested review from leehinman and removed request for michalpristas March 21, 2024 16:41

leehinman requested changes Mar 21, 2024

View reviewed changes

pchila approved these changes Mar 22, 2024

View reviewed changes

blakerouse added 9 commits April 1, 2024 09:03

Improve logging on start.

39bcb29

Merge branch 'main' into unprivileged-windows

bb665a7

Fix TestMetricsMonitoringCorrectBinaries.

f9e69d5

Fix more issues and test.

ee6a4ab

Some code cleanup.

f989d44

Fix logging of context cancelled.

a5ad1e8

Fix restart delay.

d05c552

Add comment for windows.Token(0), and more context.

12f7b16

Merge remote-tracking branch 'upstream/main' into unprivileged-windows

9e30596

pierrehilbert requested a review from leehinman April 3, 2024 08:40

leehinman requested changes Apr 3, 2024

View reviewed changes

Switch to crypto/rand.

af2cbe7

leehinman approved these changes Apr 3, 2024

View reviewed changes

pierrehilbert merged commit c923f2e into elastic:main Apr 4, 2024
8 of 9 checks passed

blakerouse deleted the unprivileged-windows branch April 4, 2024 12:57

cmacknz mentioned this pull request Apr 16, 2024

[Unprivileged] Enable --unprivileged on Windows #3868

Closed

ycombinator mentioned this pull request May 9, 2024

Document pre-requisites for running Elastic Agent in unprivileged mode #4705

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Unprivileged] Windows enablement #4264

[Unprivileged] Windows enablement #4264

blakerouse commented Feb 14, 2024 •

edited

Loading

mergify bot commented Feb 14, 2024

mergify bot commented Feb 22, 2024

elasticmachine commented Mar 21, 2024

leehinman left a comment

leehinman Mar 21, 2024

blakerouse Apr 2, 2024

leehinman Apr 2, 2024

blakerouse Apr 2, 2024

leehinman Mar 21, 2024

blakerouse Apr 2, 2024

leehinman Apr 2, 2024

blakerouse Apr 2, 2024

pchila left a comment

leehinman left a comment

blakerouse commented Apr 3, 2024

elastic-sonarqube bot commented Apr 3, 2024

pierrehilbert commented Apr 4, 2024


		t, err := windows.OpenCurrentProcessToken()
		token := windows.Token(0)

[Unprivileged] Windows enablement #4264

[Unprivileged] Windows enablement #4264

Conversation

blakerouse commented Feb 14, 2024 • edited Loading

What does this PR do?

Why is it important?

Checklist

Author's Checklist

How to test this PR locally

Related issues

mergify bot commented Feb 14, 2024

mergify bot commented Feb 22, 2024

elasticmachine commented Mar 21, 2024

leehinman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pchila left a comment

Choose a reason for hiding this comment

leehinman left a comment

Choose a reason for hiding this comment

blakerouse commented Apr 3, 2024

elastic-sonarqube bot commented Apr 3, 2024

Quality Gate failed

Failed conditions

pierrehilbert commented Apr 4, 2024

blakerouse commented Feb 14, 2024 •

edited

Loading