AuthOptions should support OS_KEY/OS_CERT/OS_CACERT

[majewsky]

We have a regulatory requirement to enable multi-factor auth in our OpenStack deployment, so we want to require users to supply client certificates in addition to their regular password-based credentials. In the upstream clients based on python-keystoneclient, this usecase is supported via the OS_CERT and OS_KEY environment variables (or respective clouds.yaml entries).

The gophercloud/utils API somewhat supports OS_CERT/OS_KEY and their sibling OS_CACERT , but only in clientconfig.NewServiceClient, not in the much more important clientconfig.AuthOptions and clientconfig.AuthenticatedClient calls. For us, that means that the only choice in the short term is to patch client certs into the ProviderClient.HTTPClient manually, like in https://github.com/sapcc/limesctl/pull/122. This feels like something that Gophercloud should cover.

The main issue is that both clientconfig.AuthOptions and clientconfig.AuthenticatedClient are constrained by the capabilities of the gophercloud.AuthOptions type, which does not have any API surface for TLS certificates. In clientconfig.AuthOptions, the constraint is visible from the outside since gophercloud.AuthOptions is the return value for this function. In clientconfig.AuthenticatedClient, the constraint is less obvious: Its implementation defers most of the work to openstack.AuthenticatedClient which takes a single gophercloud.AuthOptions argument.

Proposal

Right now, as far as I can see, the best that we can do is to change Gophercloud to:

• add new fields to gophercloud.AuthOptions that cover TLS client certs and CA certs
  • option 1: in the same way that the env vars as structured (ClientCertFile, ClientKeyFile, CACertFile string) -> advantage: clear semantics
  • option 2: allow a custom http.Client to be injected (HTTPClient *http.Client) -> advantage: same pattern as in clientconfig.ClientOpts, so we would have the chance to also fix this field to work properly all the time
• have openstack.AuthenticatedClient respect those fields (i.e. set up the ProviderClient.HTTPClient thusly before moving on into openstack.Authenticate)

Once Gophercloud has cut a new release after these changes, gophercloud/utils can be changed to:

• have clientconfig.AuthOptions fill the new fields in its return value appropriately from env vars and clouds.yaml
• have clientconfig.AuthOptions carry forward the ClientOpts.HTTPClient field into its return value (if option 2 was chosen above)

The ugly part here is that there are several intended paths to obtain a gophercloud.ProviderClient, but we can only fix some of them (specifically, all that go through openstack.AuthenticatedClient). To support the new AuthOptions fields in all possible paths, openstack.NewClient would have to be amended, but its API is too narrow.

When Gophercloud breaks API compatibility next time, I propose to:

• change openstack.NewClient() from taking a single endpoint string argument to taking a struct of at least endpoint string plus the TLS-related options
• change gophercloud.AuthOptions to have that struct type as a field in it (thus grouping all the options together that are needed for the NewClient step)

I'm willing to contribute dev time on our team to implement these changes, but I would like to align with you first before starting on the implementation.

cc @talal @databus23

[majewsky]

Friendly reminder, in case this issue got lost over the holiday break.

[majewsky]

Ping. Please advise what else we can do for you to get this moving.

[pierreprinetti]

Hi @majewsky, thank you for reporting this issue. Our team also needed to use a custom transport at some point, and we did exactly as in the code you link: the HTTP client being an exposed property of ProviderClient, we replaced it right after calling openstack.AuthenticatedClient.

Did you fully cover your use case by replacing the http.Client in ProviderClient, or is there some missing functionality that your proposed change would bring?

I am asking this because if the issue you report is purely a UX one, then maybe better docs, or an additional convenience function in utils, may be a painless acceptable solution.

[majewsky]

This is a fundamental problem in the sense that every application that wants to authenticate to OpenStack needs to be patched if certificate auth is required. This is usually only a concern in specific OpenStack installations, so tool authors are most likely not aware of it. We're now going through the motions of hand-patching dozens of OpenStack client applications, and all that is time I'd rather have invested in fixing the problem once from the get-go.

or an additional convenience function in utils, may be a painless acceptable solution.

An additional convenience function does not solve the fundamental issue:

The main issue is that both clientconfig.AuthOptions and clientconfig.AuthenticatedClient are constrained by the capabilities of the gophercloud.AuthOptions type, which does not have any API surface for TLS certificates.

Sure, we could add an additional function that deals correctly with certificate auth. But then you still have dozens of existing functions that are broken when confronted with certificate auth. Should all of these be deprecated? That does not seem like the right solution. What I'm proposing is the minimal change to gophercloud/gophercloud to allow fixing gophercloud/utils in a way that does not result in a self-contradicting API.

[EmilienM]

@majewsky could you please propose a PR and some tests? I'm not sure we have reached consensus here, but maybe if you propose some code we could directly discuss in the PR.

[majewsky]

Well, it depends on whether you would prefer option 1 or option 2 as outlined above. I don't think I'll have the time to build out both options separately.

• add new fields to gophercloud.AuthOptions that cover TLS client certs and CA certs
  • option 1: in the same way that the env vars as structured (ClientCertFile, ClientKeyFile, CACertFile string) -> advantage: clear semantics
  • option 2: allow a custom http.Client to be injected (HTTPClient *http.Client) -> advantage: same pattern as in clientconfig.ClientOpts, so we would have the chance to also fix this field to work properly all the time

Also, to clarify. @pierreprinetti said:

Our team also needed to use a custom transport at some point, and we did exactly as in the code you link: the HTTP client being an exposed property of ProviderClient, we replaced it right after calling openstack.AuthenticatedClient.

This does not work here, because at that point authentication has already been done. Injection after AuthenticatedClient is entirely too late.

[Nuckal777]

For reference 🧐 : The prometheus API allows to inject a custom HTTPClient. This handy as connection related options are configurable. If unset it is more or less set to http.DefaultClient.