workload attestor windows: support windows service

[fbarresi]

Pull Request check list

• Commit conforms to CONTRIBUTING.md?
• Proper tests/regressions included?
• Documentation updated?

Affected functionality

Created new plugin for attesting windows services.
Similar to the systemd plugin.

Description of change

I created a new plugin that uses the SCM api for attesting a workload running as service under windows.
This plugin returns 2 selectors: name and display_name of the service.

Which issue this PR fixes

[fbarresi]

I fixed the problems and rebased on main. @amartinezfayo , would you please run the workflows again?
Thanks.

btw: please note, the doc for the built-in WorkloadAttestor "windows" was missing in the file spire_agent.md and I added in my PR.

[amartinezfayo]

Thanks for working on this, @fbarresi. I think attesting Windows services is a use case worth covering. Before going deep on the code I'd like to discuss the shape of the feature, since a new built-in plugin is a long-term commitment that is hard to walk back once released.

The existing windows attestor already reads the caller's access token on every attestation, and a service configured with a service SID (sc sidtype) already produces group selectors of the form NT SERVICE<name>, which are backed by the kernel rather than by an SCM enumeration snapshot. Did you consider extending the windows attestor with a dedicated service_name selector instead of adding a new plugin? It could be derived from the S-1-5-80 service SIDs already present in the token groups that plugin iterates today, at no extra cost and with no new configuration, and the SCM lookup could then become an explicitly opt-in fallback for services that cannot set sidtype, or be left out entirely by documenting that service attestation requires setting the service SID type.

Whichever shape this takes, I think we should pin down the intended semantics for shared-process services, where several services can share one PID and a first-match lookup is nondeterministic, and for child processes of a service, which an SCM lookup cannot attribute to the service. The token-based approach answers both, since children inherit the token and a shared process carries the SIDs of all its services, while SCM enumeration needs an explicit decision on each.

Since this is a substantial change, could we also open an issue capturing the use case and this design discussion? That will give us a better record of the reasoning behind this improvement.

Thanks again!

[davidvoit]

If I understand you right your concern is forks of services, so that they are still seen as the same service. I think we would be fine with a change, but the UX is not very userfriendly with the current way of doing stuff.

You need to be into windows internalls to know that a windows service is grouped with a internal sid.

What do you would think about the following proposal, we change the windows attestor and if a SID is a virtual sevice SID we would at a selector with the name service_name.

This will be also added to the documentation. This would mean no extra lookup and no extra attestor, but would give the userfriendliness of the pull requsest.

I hope I got your point, and this is not complete nonsense. I will discuss this with Fede on monday, and maybe we could change this pull request in this way, if everybody agrees

[fbarresi]

#7220

[fbarresi]

Hi!

Thank you for your constructive comments.
I will adapt my contribution and extend the windows plugin instead.
Best regards,

FB