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

Make Dapr invocation work with .NET Core ASP.NET using SSL #44

Closed
philliphoff opened this issue Feb 28, 2020 · 3 comments · Fixed by #64
Closed

Make Dapr invocation work with .NET Core ASP.NET using SSL #44

philliphoff opened this issue Feb 28, 2020 · 3 comments · Fixed by #64
Assignees
Labels
enhancement Update to existing feature investigate Issues that require investigation
Milestone

Comments

@philliphoff
Copy link
Collaborator

Invocation of methods via Dapr fails with a certificate validation error when using Dapr with a .NET Core ASP.NET application configured for (forcing) SSL. As the default for newly scaffolded .NET Core ASP.NET applications is to use SSL, we should see if we can make that "just work".

@philliphoff philliphoff added the enhancement Update to existing feature label Feb 28, 2020
@philliphoff philliphoff added this to the 0.2.0 milestone Mar 6, 2020
@philliphoff philliphoff added the investigate Issues that require investigation label Mar 6, 2020
@philliphoff philliphoff self-assigned this Mar 10, 2020
@philliphoff
Copy link
Collaborator Author

So the issue is a combination of things:

  • There's an issue with .NET Core 3.1 and trusting certificates on Mac OS (Catalina) that generates some initial errors; that can be worked around (see dotnet/aspnetcore#18236).
  • daprd doesn't follow redirect responses from the underlying application. So, if the ASP.NET Core application redirects requests to the HTTP endpoint (e.g. those made by daprd) to the HTTPS endpoint (which ASP.NET Core is scaffolded by default to do), those redirections are returned to the caller (i.e. our extension) and automatically followed by the HTTP client library. As we have not specifically disabled certificate validation, the call fails. (Note that you will see the same failure with other REST clients, unless they've also disabled certificate validation.)

I've filed dapr/dapr#1242 to discuss whether daprd should manage redirects itself (which, if it did, might resolve the issue). In the meantime, or if the answer is ultimately "no", then we may need to disable TLS verification in our Dapr method invocations.

@philliphoff
Copy link
Collaborator Author

philliphoff commented Mar 10, 2020

There are several options I can think of:

  • Disable certificate validation: makes invocation appear to just work. Feels a bit like cheating, as the call the user expects to be made via the Dapr proxy is actually made directly by the extension (Dapr having only just returned a redirection).
  • Treat redirection responses themselves as errors/warnings: brings user attention to a likely mis-configuration, with a better message than "certificate verification failed". May not handle case where application intentionally wants to redirect to an arbitrary location.
  • Treat redirection responses themselves as the response: makes responses more literal, accounting for cases where a Dapr application intentionally wants to redirect to an arbitrary location. Actual method may not actually ever be invoked until user understands how to re-configure the application to stop the unwanted redirection.

@BigMorty Do you have any feedback on how best to handle these HTTPS redirects? Perhaps we can try to split the difference, and only treat redirects to the same host (perhaps with different schemas or ports) as warnings/errors. That is, a redirect to a completely different host is probably intentional on the part of the application, but redirection to the same host is probably not.

@BigMorty
Copy link
Member

I like the idea of treating redirection responses to the same host as an error, and if we can be even more specific and do this only when the schema goes from http to https that would great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Update to existing feature investigate Issues that require investigation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants