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

Custom extended key usage for PKI #4667

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions builtin/logical/pki/cert_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ type creationParameters struct {
NotAfter time.Time
KeyUsage x509.KeyUsage
ExtKeyUsage certExtKeyUsage
ExtKeyUsageOIDs []string
PolicyIdentifiers []string
BasicConstraintsValidForNonCA bool

Expand Down Expand Up @@ -918,6 +919,7 @@ func generateCreationBundle(b *backend, data *dataBundle) error {
NotAfter: notAfter,
KeyUsage: x509.KeyUsage(parseKeyUsages(data.role.KeyUsage)),
ExtKeyUsage: extUsage,
ExtKeyUsageOIDs: data.role.ExtKeyUsageOIDs,
PolicyIdentifiers: data.role.PolicyIdentifiers,
BasicConstraintsValidForNonCA: data.role.BasicConstraintsValidForNonCA,
}
Expand Down Expand Up @@ -989,6 +991,16 @@ func addPolicyIdentifiers(data *dataBundle, certTemplate *x509.Certificate) {
}
}

// addExtKeyUsageOids adds custom extended key usage OIDs to certificate
func addExtKeyUsageOids(data *dataBundle, certTemplate *x509.Certificate) {
for _, oidstr := range data.params.ExtKeyUsageOIDs {
oid, err := stringToOid(oidstr)
if err == nil {
certTemplate.UnknownExtKeyUsage = append(certTemplate.UnknownExtKeyUsage, oid)
}
}
}

// Performs the heavy lifting of creating a certificate. Returns
// a fully-filled-in ParsedCertBundle.
func createCertificate(data *dataBundle) (*certutil.ParsedCertBundle, error) {
Expand Down Expand Up @@ -1045,6 +1057,8 @@ func createCertificate(data *dataBundle) (*certutil.ParsedCertBundle, error) {

addKeyUsages(data, certTemplate)

addExtKeyUsageOids(data, certTemplate)
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be in signCertificate too


certTemplate.IssuingCertificateURL = data.params.URLs.IssuingCertificates
certTemplate.CRLDistributionPoints = data.params.URLs.CRLDistributionPoints
certTemplate.OCSPServer = data.params.URLs.OCSPServers
Expand Down Expand Up @@ -1254,6 +1268,8 @@ func signCertificate(data *dataBundle) (*certutil.ParsedCertBundle, error) {

addKeyUsages(data, certTemplate)

addExtKeyUsageOids(data, certTemplate)

var certBytes []byte

certTemplate.IssuingCertificateURL = data.params.URLs.IssuingCertificates
Expand Down
18 changes: 18 additions & 0 deletions builtin/logical/pki/path_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,12 @@ To remove all key usages from being set, set
this value to an empty list.`,
},

"ext_key_usage_oids": &framework.FieldSchema{
Type: framework.TypeCommaStringSlice,
Default: []string{},
Copy link
Member

Choose a reason for hiding this comment

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

Don't set an empty default here -- it's done automatically!

Description: `A comma-separated string or list of extended key usage oids.`,
},

"use_csr_common_name": &framework.FieldSchema{
Type: framework.TypeBool,
Default: true,
Expand Down Expand Up @@ -451,6 +457,7 @@ func (b *backend) pathRoleCreate(ctx context.Context, req *logical.Request, data
UseCSRCommonName: data.Get("use_csr_common_name").(bool),
UseCSRSANs: data.Get("use_csr_sans").(bool),
KeyUsage: data.Get("key_usage").([]string),
ExtKeyUsageOIDs: data.Get("ext_key_usage_oids").([]string),
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a check on the values similar to the PolicyIdentifiers check below? That way we'll know there won't be an error when generating a cert, instead of silently ignoring it.

OU: data.Get("ou").([]string),
Organization: data.Get("organization").([]string),
Country: data.Get("country").([]string),
Expand Down Expand Up @@ -495,6 +502,15 @@ func (b *backend) pathRoleCreate(ctx context.Context, req *logical.Request, data
return errResp, nil
}

if len(entry.ExtKeyUsageOIDs) > 0 {
for _, oidstr := range entry.ExtKeyUsageOIDs {
_, err := stringToOid(oidstr)
if err != nil {
return logical.ErrorResponse(fmt.Sprintf("%q could not be parsed as a valid oid for an extended key usage", oidstr)), nil
}
}
}

if len(entry.PolicyIdentifiers) > 0 {
for _, oidstr := range entry.PolicyIdentifiers {
_, err := stringToOid(oidstr)
Expand Down Expand Up @@ -588,6 +604,7 @@ type roleEntry struct {
RequireCN bool `json:"require_cn" mapstructure:"require_cn"`
AllowedOtherSANs []string `json:"allowed_other_sans" mapstructure:"allowed_other_sans"`
PolicyIdentifiers []string `json:"policy_identifiers" mapstructure:"policy_identifiers"`
ExtKeyUsageOIDs []string `json:"ext_key_usage_oids" mapstructure:"ext_key_usage_oids"`
BasicConstraintsValidForNonCA bool `json:"basic_constraints_valid_for_non_ca" mapstructure:"basic_constraints_valid_for_non_ca"`

// Used internally for signing intermediates
Expand Down Expand Up @@ -616,6 +633,7 @@ func (r *roleEntry) ToResponseData() map[string]interface{} {
"key_type": r.KeyType,
"key_bits": r.KeyBits,
"key_usage": r.KeyUsage,
"ext_key_usage_oids": r.ExtKeyUsageOIDs,
"ou": r.OU,
"organization": r.Organization,
"country": r.Country,
Expand Down
6 changes: 5 additions & 1 deletion ui/app/models/role-pki.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ export default DS.Model.extend({
defaultValue: 'DigitalSignature,KeyAgreement,KeyEncipherment',
editType: 'stringArray',
}),
extKeyUsageOids: attr({
label: 'Custom extended key usage OIDs',
editType: 'stringArray',
}),
requireCn: attr('boolean', {
label: 'Require common name',
defaultValue: true,
Expand Down Expand Up @@ -223,7 +227,7 @@ export default DS.Model.extend({
'allowedDomains',
],
},
{ 'Extended Key Usage': ['serverFlag', 'clientFlag', 'codeSigningFlag', 'emailProtectionFlag'] },
{ 'Extended Key Usage': ['serverFlag', 'clientFlag', 'codeSigningFlag', 'emailProtectionFlag', 'extKeyUsageOids'] },
{
Advanced: ['generateLease', 'noStore', 'basicConstraintsValidForNonCA', 'policyIdentifiers'],
},
Expand Down