Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[receiver/windowseventlogreceiver] Support Remote Event Log Collection via Windows API #33100

Closed
JonathanWamsley opened this issue May 16, 2024 · 8 comments
Labels

Comments

@JonathanWamsley
Copy link
Contributor

JonathanWamsley commented May 16, 2024

Component(s)

receiver/windowseventlog

Is your feature request related to a problem? Please describe.

I'm proposing to enhance the existing windowseventlogreceiver to support remote collection of Windows event logs using the Windows APIs EvtOpenSession. This feature will allow the OpenTelemetry Collector to gather event logs from remote Windows machines without needing to be installed on the host machine. This capability is especially useful in environments where direct installation on the host is not feasible.

Describe the solution you'd like

As BinaryFissionGames and pjanotti mentioned, Using EvtOpenSession can be used to enable remote event log collection. This enhancement will include:

  • Adding configuration options for specifying multiple credentials including user, and password and optional domain with server details as a list.
  • Adding remote collection using EvtOpenSession to subscribe and collect Windows Event Logs.
  • Adding a remote_sever feild on remote collection only

Single server configuration:

receivers:
    windowseventlog:
        channel: application
        remote:
            - credentials:
                username: "user"
                password: "password"
                domain: "domain"
              servers:
                - "remote-server"

Multiple servers with single credentials configuration:

receivers:
    windowseventlog:
        channel: application
        remote:
            - credentials:
                username: "user"
                password: "password"
                domain: "domain"
              servers:
                - "remote-server-1"
                - "remote-server-2"

Multiple servers with multiple credentials configuration:

receivers:
    windowseventlog:
        channel: application
        remote:
            - credentials:
                username: "user1"
                password: "password1"
                domain: "domain1"
              servers:
                - "remote-server-1"
            - credentials:
                username: "user2"
                password: "password2"
                domain: "domain2"
              servers:
                - "remote-server-2"

Describe alternatives you've considered

  1. Using go-msrpc to create a new receiver: The go-msrpc was first proposed but the library is not mature and the existing event log data is similar. After learning that the Windows API has an EVT_RPC_LOGIN, this alternative does not seem practical.

Additional context

@JonathanWamsley JonathanWamsley added enhancement New feature or request needs triage New item requiring triage labels May 16, 2024
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@djaglowski
Copy link
Member

I like that the config would only change by the addition of an optional section that implicitly indicates whether remote collection is intended, and that the outputs would be the same.

My biggest concern is with the dependency. Neither the repo or author have any public history. The repo implements a protocol which would allow the collector to make remote procedure calls to external systems. The complexity of the protocol (or at least the implementation) appear very high. All of this makes me question whether we can establish confidence that it does not introduce a security vulnerability. That said, I don't know anything about the protocol it implements so maybe someone else with a more informed opinion would be able to confidently evaluate the risk. (cc @pjanotti in case you want to look into this)

@BinaryFissionGames
Copy link
Contributor

BinaryFissionGames commented May 16, 2024

I'm curious whether it'd be easier or not to just use EvtOpenSession and just pass the returned handle into the relevant functions (e.g. as the first param in evtSubscribe).

Are there advantages to using the library over using the existing logic with a session handle from EvtOpenSession?

@pjanotti
Copy link
Contributor

(was about to hit enter @BinaryFissionGames :) saying the same)

The Event API already handles remote operations, see EvtOpenSession and EVT_RPC_LOGIN. So no need to add a new dependency.

The first question that pops on my mind is how much is this user friendly without some kind of automatic way to discover the remote servers. Is it reasonable to to start with a list that requires knowing all targeted servers before hand?

We should also think about the configuration carefully: a single set of credentials for a list of servers seems reasonable, and then multiple of such groups.

@crobert-1
Copy link
Member

Removing needs triage as code owners have responded and have a general path forward here.

@JonathanWamsley
Copy link
Contributor Author

Hey @pjanotti sorry for the delay, I have updated the issue based on feedback. I think starting with a list of targeted servers will work. The way I have it implemented allows for a single set of credentials for a list of servers and or multiple servers with multiple credentials in groups too. I used the EvtOpenSession and EVT_RPC_LOGIN as @BinaryFissionGames and you suggested and got it working 😁.

@JonathanWamsley JonathanWamsley changed the title [receiver/windowseventlogreceiver] Support Remote Event Log Collection via MSRPC [receiver/windowseventlogreceiver] Support Remote Event Log Collection via Windows API Jun 17, 2024
djaglowski added a commit that referenced this issue Aug 7, 2024
…33601)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
This PR adds remote log collection for the windows event log receiver. 
A config supports a single server configuration, multiple servers with
multiple credentials configuration and multiple servers with single
credentials configuration.

**Link to tracking Issue:** <Issue number if applicable>
#33100 

**Testing:** <Describe what testing was performed and which tests were
added.>
Added relevant test to the test files that existed.
This was tested on a single remote and with a valid / invalid remote to
ensure successful collection of logs. Gather local logs was also tested
to make sure old functionality is consistent too.

**Documentation:** <Describe the documentation added.>
Updated Read me documentation

---------

Co-authored-by: Daniel Jaglowski <[email protected]>
@JonathanWamsley
Copy link
Contributor Author

resolved with #33601

@pjanotti
Copy link
Contributor

pjanotti commented Aug 8, 2024

Thanks @JonathanWamsley!

f7o pushed a commit to f7o/opentelemetry-collector-contrib that referenced this issue Sep 12, 2024
…pen-telemetry#33601)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
This PR adds remote log collection for the windows event log receiver. 
A config supports a single server configuration, multiple servers with
multiple credentials configuration and multiple servers with single
credentials configuration.

**Link to tracking Issue:** <Issue number if applicable>
open-telemetry#33100 

**Testing:** <Describe what testing was performed and which tests were
added.>
Added relevant test to the test files that existed.
This was tested on a single remote and with a valid / invalid remote to
ensure successful collection of logs. Gather local logs was also tested
to make sure old functionality is consistent too.

**Documentation:** <Describe the documentation added.>
Updated Read me documentation

---------

Co-authored-by: Daniel Jaglowski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants