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

Legacy PerRPCCredentials with RequireTransportSecurity don't consider TransportCredentials SecurityLevel #3974

Closed
dfawley opened this issue Oct 21, 2020 · 4 comments
Assignees
Labels
P1 Type: Security A bug or other problem affecting security

Comments

@dfawley
Copy link
Member

dfawley commented Oct 21, 2020

No description provided.

@dfawley
Copy link
Member Author

dfawley commented Oct 21, 2020

From #3964 (comment):

Yes, that all sounds right to me. But note that the check in clientconn.go can't be improved since the TransportCredentials don't report their security level until after the connection is established. So the only place we can do this is in http2_client.go, and it will have to be a connection failure and not a channel-creation failure.

Actually -- would this be a per-call failure? I believe we can check at connection time or call time (or both).

@yihuazhang
Copy link
Contributor

You are right. We should do checks at both connection and call times as it is always preferred to reject early if possible. For per-connection check, I believe you should add a similar check in DialContext() API in clientconn.go.

@yihuazhang
Copy link
Contributor

Sorry, I missed your previous comment on "not being able to improve the check in clientconn.go", and I agree with it. And if PerRPCCredentials.RequireTransportSecurity()==true, I believe we should require ==PrivacyAndIntegrity.

@easwars
Copy link
Contributor

easwars commented Nov 12, 2020

Can this be closed now?

@easwars easwars closed this as completed Nov 12, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P1 Type: Security A bug or other problem affecting security
Projects
None yet
Development

No branches or pull requests

3 participants