-
Notifications
You must be signed in to change notification settings - Fork 21
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
Add support for UPN SANs #333
Conversation
This PR adds a new type otherName SAN UPN with OID 1.3.6.1.4.1.311.20.2.3. This change also allows the use of the HardwareModuleName and DirectoryName SANs in the template. The previous version only allowed the use of those using code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! Many MDMs support setting UPNs in their web interfaces.
x509util/extensions.go
Outdated
@@ -66,6 +66,7 @@ const ( | |||
RegisteredIDType = "registeredID" | |||
PermanentIdentifierType = "permanentIdentifier" | |||
HardwareModuleNameType = "hardwareModuleName" | |||
UPNType = "upn" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would propose to use UserPrincipalNameType
internally instead of UPNType
, but ain't blocking this from being used. There are others acronyms, like DNSType
, but those are generally more well known.
Besides that, maybe it should be "userPrincipalName"
as the JSON property? That's more in line with the rest of the properties. Curious if it would be possible to use two names for the same, like an alias?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed with b724aa2
x509util/extensions.go
Outdated
@@ -389,6 +412,22 @@ func (s SubjectAlternativeName) RawValue() (asn1.RawValue, error) { | |||
IsCompound: true, | |||
Bytes: rdn, | |||
}, nil | |||
case UPNType: | |||
if s.Value == "" { | |||
return zero, errors.New("error parsing UserPrincipalName SAN: empty Value is not allowed") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor, but I think more consistent with the error message below
return zero, errors.New("error parsing UserPrincipalName SAN: empty Value is not allowed") | |
return zero, errors.New("error parsing UserPrincipalName SAN: empty value is not allowed") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed with b724aa2
x509util/extensions.go
Outdated
Value: asn1.RawValue{FullBytes: rawBytes}, | ||
}, "tag:0") | ||
if err != nil { | ||
return zero, errors.Wrap(err, "unable to Marshal UserPrincipalName SAN") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In line with other (un)marshaling errors:
return zero, errors.Wrap(err, "unable to Marshal UserPrincipalName SAN") | |
return zero, errors.Wrap(err, "error marshaling UserPrincipalName SAN") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed with b724aa2
Description
This PR adds a new type otherName SAN UPN with OID 1.3.6.1.4.1.311.20.2.3. This change also allows the use of the HardwareModuleName and DirectoryName SANs in the template. The previous version only allowed the use of those using code.