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

Allow OtherName type in SAN matching #34358

Closed
arulthileeban opened this issue May 24, 2024 · 5 comments · Fixed by #34471
Closed

Allow OtherName type in SAN matching #34358

arulthileeban opened this issue May 24, 2024 · 5 comments · Fixed by #34471
Labels
area/listener area/tls enhancement Feature requests. Not bugs or questions.

Comments

@arulthileeban
Copy link
Contributor

Title: Allow OtherName type in SAN matching

Description:
In the current state, Envoy allows only 4 types(EMAIL, DNS, URI, IP_ADDRESS) of SAN against which SAN verification will be performed against. This restricts our ability to match against OtherName based SANs. For user/device certificates with non-standard SANs (not Email/URI), "OtherName" SAN is preferred. CN matching isn't ideal and not supported by Envoy as well.

OtherName (UPN) is also the suggested SAN in MDMs like Jamf/Intune for these usecases. With this in mind, could the list of types be expanded to allow matching against OtherName type?

@arulthileeban arulthileeban added enhancement Feature requests. Not bugs or questions. triage Issue requires triage labels May 24, 2024
@arulthileeban
Copy link
Contributor Author

arulthileeban commented May 24, 2024

I looked at the implementation for the typed SAN matching and it looks like OtherName idea was dropped due to inability to generalize it #18628 . That issue still stands.

I'd be happy to raise a PR to resolve this, but I wasn't clear on how to go forward. We need the OID of the type of OtherName to be defined(Eg: UPN). Either we can pick a few OtherName types which are popular like UPN and only support them or ask users to mention the OID if we want to generalize it (which is not very user friendly, but generalizable), unless there's a better way I'm missing

@zuercher zuercher added area/tls area/listener and removed triage Issue requires triage labels May 28, 2024
@zuercher
Copy link
Member

cc @ggreenway

@ggreenway
Copy link
Contributor

@arulthileeban that would be great if you made a PR to address this. I prefer making it more general, with the OID, but maybe post what the config proto would look like for both alternatives and we can decide based on that.

@arulthileeban
Copy link
Contributor Author

Generalized format:

// Matcher for subject alternative names, to match both type and value of the SAN.
message SubjectAltNameMatcher {
  // Indicates the choice of GeneralName as defined in section 4.2.1.5 of RFC 5280 to match
  // against.
  enum SanType {
    SAN_TYPE_UNSPECIFIED = 0;
    EMAIL = 1;
    DNS = 2;
    URI = 3;
    IP_ADDRESS = 4;
    OTHER = 5;
  }

  // Specification of type of SAN. Note that the default enum value is an invalid choice.
  SanType san_type = 1 [(validate.rules).enum = {defined_only: true not_in: 0}];

  // Matcher for SAN value.
  type.matcher.v3.StringMatcher matcher = 2 [(validate.rules).message = {required: true}];

  // OID for Other SAN
  string OID = 3;
}

Specific OtherType format:

// Matcher for subject alternative names, to match both type and value of the SAN.
message SubjectAltNameMatcher {
  // Indicates the choice of GeneralName as defined in section 4.2.1.5 of RFC 5280 to match
  // against.
  enum SanType {
    SAN_TYPE_UNSPECIFIED = 0;
    EMAIL = 1;
    DNS = 2;
    URI = 3;
    IP_ADDRESS = 4;
    OTHER = 5;
  }

  enum OtherSanType {
    OTHER_SAN_TYPE_UNSPECIFIED = 0;
    UPN = 1; // OID - 1.3.6.1.4.1.311.20.2.3
    SRVNAME = 2;  // OID - 1.3.6.1.5.5.7.8.7
    PERMANENT_IDENTIFIER = 3; // OID - 1.3.6.1.5.5.7.8.3
    NT_PRINCIPAL_NAME = 4; // OID - 1.3.6.1.4.1.311.20.2.1 
    KRB5PRINCIPALNAME = 5; // OID - 1.3.6.1.5.2.2
  }

  // Specification of type of SAN. Note that the default enum value is an invalid choice.
  SanType san_type = 1 [(validate.rules).enum = {defined_only: true not_in: 0}];

  // Matcher for SAN value.
  type.matcher.v3.StringMatcher matcher = 2 [(validate.rules).message = {required: true}];

  // Type for Other SAN
  OtherSanType other_san_type = 3 [(validate.rules).enum = {defined_only: true not_in: 0}];
}

As I'm thinking more about this, it makes more sense to me as well to generalize it with getting an OID as input. Code changes shouldn't be required each time someone needs support for a new OtherName type. @ggreenway Thoughts on how we can proceed ?

@ggreenway
Copy link
Contributor

Agreed, the generalized version looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/listener area/tls enhancement Feature requests. Not bugs or questions.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants