opensearch-py icon indicating copy to clipboard operation
opensearch-py copied to clipboard

[FEATURE] Optionally disable default certifi certificates

Open nokados opened this issue 1 year ago • 6 comments

Is your feature request related to a problem?

I dislike certifi because it makes managing self-signed certificates difficult. I would prefer to use the system certificate store instead.

What solution would you like?

I believe the most straightforward solution would be to use ca_certs=None if it is explicitly set. Here's what I mean: change this

            # Convert all sentinel values to their actual default
            # values if not using an SSLContext.
            if verify_certs is VERIFY_CERTS_DEFAULT:
                verify_certs = True
            if ssl_show_warn is SSL_SHOW_WARN_DEFAULT:
                ssl_show_warn = True

            ca_certs = self.default_ca_certs() if ca_certs is None else ca_certs

to this

```python
            # Convert all sentinel values to their actual default
            # values if not using an SSLContext.
            if verify_certs is VERIFY_CERTS_DEFAULT:
                verify_certs = True
            if ssl_show_warn is SSL_SHOW_WARN_DEFAULT:
                ssl_show_warn = True

            ca_certs = self.default_ca_certs() if ca_certs is CA_CERTS_DEFAULT else ca_certs

What alternatives have you considered?

I can't think of any other viable solutions.

nokados avatar Dec 23 '24 11:12 nokados

Maybe we could add ca_certs as an input parameter, introduce ca_certs = AUTO and default to it?

Either way appreciate a PR!

dblock avatar Dec 23 '24 12:12 dblock

I think I can make some time this week

nokados avatar Dec 24 '24 07:12 nokados

Maybe we could add ca_certs as an input parameter, introduce ca_certs = AUTO and default to it?

Yes, your option is probably better, as it is more explicit, but it deviates from the pattern of automatically determining default values used for verify_certs and ssl_show_warn. On the other hand, these are boolean variables, while we are changing str | Path | certifi | system_default. Alternatively, we could define constants like 'system' and 'certifi' to make it even clearer which paths will be checked. Please select the most preferable option. Also, are there any other suggestions for the Pull Request?

nokados avatar Dec 24 '24 08:12 nokados

Thanks @nokados. My biggest concern is backwards compatibility, we cannot change the way existing parameters and their values work in the current interface without a major version bump, and a lesser concern future adaptability so we can keep changing it. We can always add of course. Give all these options a try and keep that in mind?

dblock avatar Dec 24 '24 11:12 dblock

I've encountered problems: with tests (https://github.com/opensearch-project/opensearch-py/pull/879) and with requests support, so the PR will drag on for a while.

Some new details about the implementation:

  • I want to add a new constant value, "system", for verify_certs. I believe it is intuitive enough and fully backward-compatible.
  • For requests, some unavoidable complications will arise. I believe that using a custom adapter is the most straightforward and reliable approach.

nokados avatar Jan 10 '25 13:01 nokados

Is there any reason this wouldn't work ? https://github.com/opensearch-project/opensearch-py/pull/196

nufissime avatar Jul 29 '25 15:07 nufissime