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

destination: add UriLikeIdentity and server_name #285

Conversation

zaharidichev
Copy link
Member

This PR changes the TlsIdentity type in the destination API such that:

  • we add an extra UriLikeIdentity identity type that should contain identities that are in URI format (e.g. SPIFFE)
  • we add a server_name to the TlsIdentity type. This allows us to differentiate between an SNI value and a TLS Id value. This is mainly needed because in certain identity systems (SPIFFE/SPIRE) the TLS SAN can be in URI form. A URI cannot be used as a SNI extension in a ClientHello, so an alternative SNI value needs to be provided. This brings the need to distinguish between these two concepts.

For context:
linkerd/linkerd2-proxy#2506

Signed-off-by: Zahari Dichev <zaharidichev@gmail.com>
Copy link
Member

@mateiidavid mateiidavid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me after playing with this in the prototype. Should we also add a small comment in

message Identity { string name = 1; }
to say identity can be either uri or dns?

oneof strategy { DnsLikeIdentity dns_like_identity = 1; }
oneof strategy {
DnsLikeIdentity dns_like_identity = 1;
UriLikeIdentity uri_like_identity = 3;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I recall correctly, the name DnsLikeIdentity was chosen to indicate that this isn't actually a DNS name:

  1. We don't expect it to be resolvable via DNS.
  2. It doesn't actually support all DNS names (i.e., trailing dots are forbidden).

I think in this case, we can just call it UriIdentity.

UriLikeIdentity uri_like_identity = 3;
}

ServerName server_name = 4;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems appropriate to use DnsLikeIdentity for the server_name type to me. The server name has to be a DNS-like string.

Signed-off-by: Zahari Dichev <zaharidichev@gmail.com>
@zaharidichev zaharidichev requested a review from olix0r January 3, 2024 12:35
Signed-off-by: Zahari Dichev <zaharidichev@gmail.com>
@zaharidichev
Copy link
Member Author

@olix0r renamed UriIdentity back to UriLikeIdentity as discussed earlier. Are we fine merging that now?

@zaharidichev
Copy link
Member Author

Will merge that, we can do a follow up if needed.

@zaharidichev zaharidichev merged commit 2da43c5 into linkerd:main Jan 5, 2024
8 checks passed
zaharidichev added a commit to linkerd/linkerd2-proxy that referenced this pull request Jan 15, 2024
This change is a follow-up to the work to split the concepts of
`ServerId` and `ServerName`. To do that we consume the
changes to the protobuf API introduced in: linkerd/linkerd2-proxy-api#285.
while keeping things backward compatible.

 Signed-off-by: Zahari Dichev <zaharidichev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants