search

splunk / splunk-sdk-python

Splunk Software Development Kit for Python
http://dev.splunk.com
Apache License 2.0
698 stars 370 forks source link

Set cookie issue 438 #449

Closed ashah-splunk closed 2 years ago

ashah-splunk commented 2 years ago

Changes in response to the issue #438

bendikro commented 2 years ago

This fix now ensures that cookies only will be used when there is an auth cookie splunkd_<XXXX> availlable in self.http._cookies. However, it introduces a new bug which is that if there is a cookie available, e.g. from a Set-Cookie from a middleware, when there is not an auth cookie availlable, this cookie will not be included in the requests.

fantavlik commented 2 years ago

This fix now ensures that cookies only will be used when there is an auth cookie splunkd_<XXXX> availlable in self.http._cookies. However, it introduces a new bug which is that if there is a cookie available, e.g. from a Set-Cookie from a middleware, when there is not an auth cookie availlable, this cookie will not be included in the requests.

Hi @bendikro - the fact that any/all cookies were being sent if detected was an unintentional side-effect of the bug that was addressed in this change. By design the SDK should not include cookies unrelated to Splunk - although any additional headers including cookies can be set by users to be sent by the SDK client here: https://github.com/splunk/splunk-sdk-python/blob/19acb9a3944d09f52374b7c9fb829ccff277dc22/splunklib/binding.py#L454-L455 does this address your concern here? I hope I'm capturing your middleware use case.

bendikro commented 2 years ago

Hi @bendikro - the fact that any/all cookies were being sent if detected was an unintentional side-effect of the bug that was addressed in this change. By design the SDK should not include cookies unrelated to Splunk - although any additional headers including cookies can be set by users to be sent by the SDK client here:

https://github.com/splunk/splunk-sdk-python/blob/19acb9a3944d09f52374b7c9fb829ccff277dc22/splunklib/binding.py#L454-L455 does this address your concern here? I hope I'm capturing your middleware use case.

Hi @fantavlik

What is the rationale for not including cookies in the requests when a Set-Cookie has been included in a response? What is gained by ignoring these?

With "cookies unrelated to Splunk" you mean Set-Cookies that are not set by the Splunk server? I challenge your definition of "unrelated to Splunk", and for our case I'd argue the Set-Cookie (BIGipServer<MyPoolName>) from the F5 load balancer is very much related to Splunk even though it's not the Splunk server that generates the Set-Cookie. And by ignoring this Set-Cookie (set by the F5 middleware in our case), the Splunk SDK doesn't work properly when the network is configured with any load balancer that utilizes this technique.

The official Splunk docs even has a section on Use a load balancer with search head clustering:

Splunk recommends that you run a third-party hardware or software load balancer in front of your set of clustered search heads. That way, users can access the set of search heads through a single interface, without needing to specify a particular one.

There are a variety of third-party load balancers available that you can use for this purpose. Select a load balancer that employs layer-7 (application-level) processing.

Configure the load balancer so that user sessions are "sticky" or "persistent." This ensures that the user remains on a single search head throughout their session.

From this we find:

The F5 doc has a page HTTP Load Balancing with Cookie Persistence, where "Cookie Persistence" is the "sticky" or "persistent" sessions referenced in the Splunk docs above.

The solution to include additional headers manually in the client is how we worked around the issue in the first place, but it shouldn't be necessary to manually extract the BIGip cookie (after initial requests to Splunk) and then create a new client with this cookie as an additional header. It is the responsibility of the Splunk SDK client to handle this.

If this behavior of the SDK truly is by design, I think the design should be reconsidered.

fantavlik commented 2 years ago

Hi @bendikro thanks for giving us a detailed breakdown of your use case. I believe we've addressed your request in this PR: https://github.com/splunk/splunk-sdk-python/pull/463 which will be part of our next release in the coming days.

bendikro commented 2 years ago

@fantavlik Thanks! That fix has a bug though (https://github.com/splunk/splunk-sdk-python/issues/438#issuecomment-1163103666) but after resolving that it seems to be working properly.

Trek333 commented 2 years ago

@fantavlik and @bendikro The latest version of the SOAR Splunk app is impacted by this issue also. I would suggest that the next release of the Splunk Enterprise Python SDK features list should mention support of Load Balancer "sticky sessions" (persistent cookies) to make it more clear which software package that handles this feature.

The Splunk Enterprise Python SDK is included with the SOAR Splunk app to connect to a Splunk Enterprise Search Head (SH) server and/or Search Head Cluster (SHC). Using the proxy feature of the Burp Suite app to observe the behavior of the http requests & responses by the SOAR Splunk app via the Splunk Enterprise Python SDK to a Splunk Enterprise SHC with a F5 Load Balancer in front of it, the BIGipServer cookie used for persistence ("sticking sessions") is not included in subsequent http requests after receiving the BIGipServer cookie in the response of the first http request from the Splunk SOAR app. However as a comparison using a Firefox browser instead of the SOAR Splunk app as client to the Splunk Enterprise SHC, a Firefox browser does send the BIGipServer cookie in subsequent http requests after receiving the BIGipServer cookie in the response of the first http request from the Firefox browser.

bendikro commented 2 years ago

@Trek333 I agree the feature should be listed, and the unit tests should be updated with a test to avoid any future regressions.

ashah-splunk commented 2 years ago

@bendikro we did include unit tests where if the Auth Cookie is not available, authentication is being done with username and password. Test cases were updated as part of this PR itself, would request you to refer them. Also there were multiple PRs for the fix of the issue #438 and the fix was delivered in 2 different releases. As for mentioning this feature it was mentioned in the CHANGELOG.md but under Bug Fix(version 1.6.20) and Minor Changes(version 1.7.0) - Reference. I hope I have answered your concern, please do let me know if anything I have missed. Thanks

Trek333 commented 2 years ago

@ashah-splunk and @bendikro #485 pull request has been submitted to add a unit test that is specific to regression testing the persistence of 3rd party cookies by the Splunk Enterprise Python SDK. More specifically, this unit test is targeted at the use case of the Splunk Enterprise Python SDK supporting "sticky sessions" of a Load Balancer for a Splunk Enterprise SHC (Search Head Cluster). Please let me know your feedback and feel to free to add more unit tests for 3rd party cookie persistence. Thanks