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

Error accessing authenticated endpoint with plain HTTP endpoint [HTTPError: Only https is supported] #5486

Open
thelazydogsback opened this issue Sep 28, 2024 · 10 comments
Labels
Python Status: No Recent Activity status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close type:question An issue that's a question

Comments

@thelazydogsback
Copy link

thelazydogsback commented Sep 28, 2024

What are you generating using Kiota, clients or plugins?

API Client/SDK

In what context or format are you using Kiota?

Windows executable

Client library/SDK language

Python

Describe the bug

I am trying to access an authenticated HTTP endpoint - not HTTPS.
However it appears that authentication must take place over HTTPS.

Expected behavior

Both the authentication call and the final api call should succeed

How to reproduce

The following code using only httpx works fine when the baseUri is "http://mySvc".
Note this is an authenticated service-to-service call within a single Azure Container vnet - only an HTTP endpoint is exposed, not HTTPS.

def get_my_foo(baseUri, id):
    scope = "api://xxxxx/.default"
    credential = ChainedTokenCredential(ManagedIdentityCredential(), DefaultAzureCredential())
    token = credential.get_token(scope).token
    with httpx.Client() as client: 
        headers = { "Authorization": f"Bearer {token}" }
        response = client.get(f"{baseUri}/foos/{id}", headers=headers)
        response.raise_for_status()  # This will raise an exception for 4xx/5xx errors
        return response.json()

However, using this kiota generated client:

        credential = ChainedTokenCredential(ManagedIdentityCredential(), DefaultAzureCredential()  )
        # Note that no combination of these params makes any difference here
        http_client = httpx.AsyncClient(verify = False,  follow_redirects = True)
        auth_provider = AzureIdentityAuthenticationProvider(credential, scopes=adt_api_scope) 
        request_adapter = HttpxRequestAdapter(
            http_client = http_client,
            authentication_provider = auth_provider,
            base_url = self.base_url)
        client = MyClientKiota(request_adapter)

Does not work when calling client.foos.by_id(id). The error I get is:

 File "/usr/local/lib/python3.12/site-packages/kiota_authentication_azure/azure_identity_access_token_provider.py", line 78, in get_authorization_token
    raise exc
kiota_authentication_azure._exceptions.HTTPError: Only https is supported

This appears to be because that while my actual internal service call needs to be plain HTTP, then auth call needs to be HTTPS.
How do I get the kiota client to behave like the httx code, (apparently) still using HTTPS to get the token, but using HTTP to access my endpoint?

Open API description file

No response

Kiota Version

1.17.0+1eb16cd65853c17179e2dde3ae6098135deacf55

Latest Kiota version known to work for scenario above?(Not required)

No response

Known Workarounds

No response

Configuration

No response

Debug output

Click to expand log ```
</details>


### Other information

_No response_
@thelazydogsback thelazydogsback added status:waiting-for-triage An issue that is yet to be reviewed or assigned type:bug A broken experience labels Sep 28, 2024
@msgraph-bot msgraph-bot bot added the Python label Sep 28, 2024
@baywet
Copy link
Member

baywet commented Sep 29, 2024

Hi @thelazydogsback ,
Thank you for using Kiota and for reaching out.
This is a design principle to avoid compromising credentials.
Transmitting jwts over clear HTTP makes the application extremely vulnerable to a monster in the middle attack.
While this vulnerability is negated in loopback scenarios, we don't want to give people an easy way out so they don't accidentally build unsecure applications.

Your best way forward is probably to create a new "DangerousAzureIdentityAcccesTokenProvider" that derives from the original one, replaces http by https, and calls the base method.
Then you can also create a "DangerousAzureIdentityAuthenticationProvider" by copying the code we have in the original one but changing the access token provider by the one you've just provided.

Let us know if you have additional questions.

@baywet baywet added status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close type:question An issue that's a question and removed type:bug A broken experience status:waiting-for-triage An issue that is yet to be reviewed or assigned labels Sep 29, 2024
@SilverioMiranda
Copy link

@baywet i think that approach a pain in the ass, i use kiota for in cluster communications inside k8s using bff architecture, so there is no security problem, there is no access to external urls, i realy think that must be a better way around that, an easy one

@baywet
Copy link
Member

baywet commented Sep 30, 2024

Thank you for the additional information.

On the technical side, the change "is easy". Simply add another setting the the authentication/access token providers to drive the condition which enforces the use of HTTPS.

What I'm questioning is whether we should make it easy, at the risk of making it easy to build unsecured applications and compromising customers.

Maybe if we named the setting "DangerousDisableHttpsVerification" or something along those lines it make this distinction clear?
We could also add an open telemetry tracing attribute when this is disabled, or trigger an event (the later option being much noisier).

I'd like to get the input of our architect @darrelmiller before we more forward with this one.

@thelazydogsback
Copy link
Author

thelazydogsback commented Sep 30, 2024

Thanks @SilverioMiranda and @baywet.
Yes with the vnet setup the container ecosystem is closed to the outside world (except for public BFF containers where ispublic=true) and there should be no security issue. (Not "loopback" to single host but set of containers with local DNS resolving internal services.)
I don't see this as being uncommon these days, so ISTM that a "best practice" that only addresses a particular scenario could be the default but should be changeable via a setting as you suggest.
(I could be wrong, but ISTM that the onus of deciding what is a secure application is the responsibility of the target service design/author, and not the consumer/client - but I guess defense in depth is best...)

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs: Attention 👋 and removed status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close labels Sep 30, 2024
@thelazydogsback
Copy link
Author

thelazydogsback commented Sep 30, 2024

Thanks @baywet - I was able to implement your suggestion by overriding AzureIdentityAcccesTokenProvider which has a copy/paste implementation of get_authorization_token with this code disabled:

                if parsed_url.scheme != "https" and (parsed_url.hostname not in self.LOCALHOST_STRINGS):
                    span.set_attribute(self.IS_VALID_URL, False)
                    exc = HTTPError("Only https is supported")
                    span.record_exception(exc)
                    raise exe

and providing a subclass of BaseBearerTokenAuthenticationProvider to provide it.

So I'm good for now, but will remove this code if there is a configurable option in the future.

@baywet
Copy link
Member

baywet commented Oct 1, 2024

Thank you for the additional information.

An alternative (and arguably better) design here would be to make the localhost_strings configurable with a better name like "HostsToAllowHttp" or something like that. It could reuse the entirety of the ValidHosts infrastructure, leading to minimal duplication in the implementation.

But again, I'll like a sign off from @darrelmiller before we proceed here.

@darrelmiller
Copy link
Member

There is ongoing discussion in the IETF about sending credentials to APIs over HTTP(S) https://www.ietf.org/archive/id/draft-rsalz-httpapi-privacy-00.html following the blog post a few months ago about the risks of redirecting HTTP to HTTPS.

Some people are of the opinion that APIs should only be exposed via HTTPs. richsalz/draft-rsalz-httpapi-privacy#7

I don't have a strong objection to making it possible to turn off HTTPS as long as it is clear that it is not a recommended approach.

@thelazydogsback
Copy link
Author

thelazydogsback commented Oct 2, 2024

Thanks @darrelmiller . That makes sense to me for APIs exposed to the outside world, but I'm not sure I see the reasoning if none of the endpoints are even reachable at all except from other internal services that are in the same vnet, and also check that the caller is whitelisted, etc. HTTPS is pretty fast, but it's not free. (A few hundred ms for handshakes and 10-20% overhead thereafter?)

@baywet
Copy link
Member

baywet commented Oct 2, 2024

Thanks for chiming in.

On the spirit of the ask: while there's probably no risk for a loopback scenario, there's probably still some risk on a microservices scenario, even running on a single host. All those containers are relying on IPs, names, and DNS to communicate with each other. A compromised container could in theory start sending new IPs, change the DNS resolution and start observing clear HTTP traffic. Hopefully the network stack for the container engine guards against that through firewalls preventing containers from doing DHCP, but that's not a guarantee. So yes, it should be clearly called out.

But let's experiment with what it'd mean to open this up a little more. @SilverioMiranda @thelazydogsback would one of you be willing to submit a draft PR with:

  1. an additional optional parameter "dangerous_allow_http_hosts" in the authentication provider (pass it down to the access token provider, initialize with this value)
  2. a reflection of that optional parameter in the azure identity access token provider
  3. initialize a new allowed host provider with the value
  4. use the new allowed host provider property for the validation

@baywet baywet added status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close and removed Needs: Attention 👋 labels Oct 2, 2024
Copy link
Contributor

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Python Status: No Recent Activity status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close type:question An issue that's a question
Projects
Status: Waits for author 🔁
Development

No branches or pull requests

4 participants