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

Add per-issuer AIA URI information to PKI secrets engine #16563

Merged
merged 11 commits into from
Aug 19, 2022
75 changes: 75 additions & 0 deletions builtin/logical/pki/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4922,6 +4922,81 @@ AwEHoUQDQgAE57NX8bR/nDoW8yRgLswoXBQcjHrdyfuHS0gPwki6BNnfunUzryVb
require.Equal(t, len(importedIssuers), 0)
}

func TestPerIssuerAIA(t *testing.T) {
t.Parallel()
b, s := createBackendWithStorage(t)

// Generating a root without anything should not have AIAs.
resp, err := CBWrite(b, s, "root/generate/internal", map[string]interface{}{
"common_name": "root example.com",
"issuer_name": "root",
"key_type": "ec",
})
require.NoError(t, err)
require.NotNil(t, resp)
rootCert := parseCert(t, resp.Data["certificate"].(string))
require.Empty(t, rootCert.OCSPServer)
require.Empty(t, rootCert.IssuingCertificateURL)
require.Empty(t, rootCert.CRLDistributionPoints)

// Set some local URLs on the issuer.
_, err = CBWrite(b, s, "issuer/default", map[string]interface{}{
"issuing_certificates": []string{"https://google.com"},
})
require.NoError(t, err)

_, err = CBWrite(b, s, "roles/testing", map[string]interface{}{
"allow_any_name": true,
"ttl": "85s",
"key_type": "ec",
})
require.NoError(t, err)

// Issue something with this re-configured issuer.
resp, err = CBWrite(b, s, "issuer/default/issue/testing", map[string]interface{}{
"common_name": "localhost.com",
})
require.NoError(t, err)
require.NotNil(t, resp)
leafCert := parseCert(t, resp.Data["certificate"].(string))
require.Empty(t, leafCert.OCSPServer)
require.Equal(t, leafCert.IssuingCertificateURL, []string{"https://google.com"})
require.Empty(t, leafCert.CRLDistributionPoints)

// Set global URLs and ensure they don't appear on this issuer's leaf.
_, err = CBWrite(b, s, "config/urls", map[string]interface{}{
"issuing_certificates": []string{"https://example.com/ca", "https://backup.example.com/ca"},
"crl_distribution_points": []string{"https://example.com/crl", "https://backup.example.com/crl"},
"ocsp_servers": []string{"https://example.com/ocsp", "https://backup.example.com/ocsp"},
})
require.NoError(t, err)
resp, err = CBWrite(b, s, "issuer/default/issue/testing", map[string]interface{}{
"common_name": "localhost.com",
})
require.NoError(t, err)
require.NotNil(t, resp)
leafCert = parseCert(t, resp.Data["certificate"].(string))
require.Empty(t, leafCert.OCSPServer)
require.Equal(t, leafCert.IssuingCertificateURL, []string{"https://google.com"})
require.Empty(t, leafCert.CRLDistributionPoints)

// Now come back and remove the local modifications and ensure we get
// the defaults again.
_, err = CBPatch(b, s, "issuer/default", map[string]interface{}{
"issuing_certificates": []string{},
})
require.NoError(t, err)
resp, err = CBWrite(b, s, "issuer/default/issue/testing", map[string]interface{}{
"common_name": "localhost.com",
})
require.NoError(t, err)
require.NotNil(t, resp)
leafCert = parseCert(t, resp.Data["certificate"].(string))
require.Equal(t, leafCert.IssuingCertificateURL, []string{"https://example.com/ca", "https://backup.example.com/ca"})
require.Equal(t, leafCert.OCSPServer, []string{"https://example.com/ocsp", "https://backup.example.com/ocsp"})
require.Equal(t, leafCert.CRLDistributionPoints, []string{"https://example.com/crl", "https://backup.example.com/crl"})
}

var (
initTest sync.Once
rsaCAKey string
Expand Down
9 changes: 5 additions & 4 deletions builtin/logical/pki/cert_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func (sc *storageContext) fetchCAInfoByIssuerId(issuerId issuerID, usage issuerU
RevocationSigAlg: entry.RevocationSigAlg,
}

entries, err := getURLs(sc.Context, sc.Storage)
entries, err := entry.GetAIAURLs(sc)
if err != nil {
return nil, errutil.InternalError{Err: fmt.Sprintf("unable to fetch URL information: %v", err)}
}
Expand Down Expand Up @@ -674,8 +674,9 @@ func generateCert(sc *storageContext,
data.Params.PermittedDNSDomains = input.apiData.Get("permitted_dns_domains").([]string)

if data.SigningBundle == nil {
// Generating a self-signed root certificate
entries, err := getURLs(ctx, sc.Storage)
// Generating a self-signed root certificate. Since we have no
// issuer entry yet, we default to the global URLs.
entries, err := getGlobalAIAURLs(ctx, sc.Storage)
if err != nil {
return nil, errutil.InternalError{Err: fmt.Sprintf("unable to fetch URL information: %v", err)}
}
Expand Down Expand Up @@ -1395,7 +1396,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn
return creation, nil
}

// This will have been read in from the getURLs function
// This will have been read in from the getGlobalAIAURLs function
creation.Params.URLs = caSign.URLs

// If the max path length in the role is not nil, it was specified at
Expand Down
12 changes: 6 additions & 6 deletions builtin/logical/pki/path_config_urls.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func validateURLs(urls []string) string {
return ""
}

func getURLs(ctx context.Context, storage logical.Storage) (*certutil.URLEntries, error) {
func getGlobalAIAURLs(ctx context.Context, storage logical.Storage) (*certutil.URLEntries, error) {
entry, err := storage.Get(ctx, "urls")
if err != nil {
return nil, err
Expand Down Expand Up @@ -98,7 +98,7 @@ func writeURLs(ctx context.Context, storage logical.Storage, entries *certutil.U
}

func (b *backend) pathReadURL(ctx context.Context, req *logical.Request, _ *framework.FieldData) (*logical.Response, error) {
entries, err := getURLs(ctx, req.Storage)
entries, err := getGlobalAIAURLs(ctx, req.Storage)
if err != nil {
return nil, err
}
Expand All @@ -115,7 +115,7 @@ func (b *backend) pathReadURL(ctx context.Context, req *logical.Request, _ *fram
}

func (b *backend) pathWriteURL(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
entries, err := getURLs(ctx, req.Storage)
entries, err := getGlobalAIAURLs(ctx, req.Storage)
if err != nil {
return nil, err
}
Expand All @@ -124,21 +124,21 @@ func (b *backend) pathWriteURL(ctx context.Context, req *logical.Request, data *
entries.IssuingCertificates = urlsInt.([]string)
if badURL := validateURLs(entries.IssuingCertificates); badURL != "" {
return logical.ErrorResponse(fmt.Sprintf(
"invalid URL found in issuing certificates: %s", badURL)), nil
"invalid URL found in Authority Information Access (AIA) parameter issuing_certificates: %s", badURL)), nil
}
}
if urlsInt, ok := data.GetOk("crl_distribution_points"); ok {
entries.CRLDistributionPoints = urlsInt.([]string)
if badURL := validateURLs(entries.CRLDistributionPoints); badURL != "" {
return logical.ErrorResponse(fmt.Sprintf(
"invalid URL found in CRL distribution points: %s", badURL)), nil
"invalid URL found in Authority Information Access (AIA) parameter crl_distribution_points: %s", badURL)), nil
}
}
if urlsInt, ok := data.GetOk("ocsp_servers"); ok {
entries.OCSPServers = urlsInt.([]string)
if badURL := validateURLs(entries.OCSPServers); badURL != "" {
return logical.ErrorResponse(fmt.Sprintf(
"invalid URL found in OCSP servers: %s", badURL)), nil
"invalid URL found in Authority Information Access (AIA) parameter ocsp_servers: %s", badURL)), nil
}
}

Expand Down
131 changes: 131 additions & 0 deletions builtin/logical/pki/path_fetch_issuers.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,21 @@ which may not be known at modification time (such as with PKCS#11 managed
RSA keys).`,
Default: "",
}
fields["issuing_certificates"] = &framework.FieldSchema{
Type: framework.TypeCommaStringSlice,
Description: `Comma-separated list of URLs to be used
for the issuing certificate attribute. See also RFC 5280 Section 4.2.2.1.`,
}
fields["crl_distribution_points"] = &framework.FieldSchema{
Type: framework.TypeCommaStringSlice,
Description: `Comma-separated list of URLs to be used
for the CRL distribution points attribute. See also RFC 5280 Section 4.2.1.13.`,
}
fields["ocsp_servers"] = &framework.FieldSchema{
Type: framework.TypeCommaStringSlice,
Description: `Comma-separated list of URLs to be used
for the OCSP servers attribute. See also RFC 5280 Section 4.2.2.1.`,
}

return &framework.Path{
// Returns a JSON entry.
Expand Down Expand Up @@ -208,13 +223,22 @@ func respondReadIssuer(issuer *issuerEntry) (*logical.Response, error) {
"usage": issuer.Usage.Names(),
"revocation_signature_algorithm": revSigAlgStr,
"revoked": issuer.Revoked,
"issuing_certificates": []string{},
"crl_distribution_points": []string{},
"ocsp_servers": []string{},
}

if issuer.Revoked {
data["revocation_time"] = issuer.RevocationTime
data["revocation_time_rfc3339"] = issuer.RevocationTimeUTC.Format(time.RFC3339Nano)
}

if issuer.AIAURIs != nil {
data["issuing_certificates"] = issuer.AIAURIs.IssuingCertificates
data["crl_distribution_points"] = issuer.AIAURIs.CRLDistributionPoints
data["ocsp_servers"] = issuer.AIAURIs.OCSPServers
}

return &logical.Response{
Data: data,
}, nil
Expand Down Expand Up @@ -302,6 +326,20 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da
return nil, err
}

// AIA access changes
issuerCertificates := data.Get("issuing_certificates").([]string)
if badURL := validateURLs(issuerCertificates); badURL != "" {
return logical.ErrorResponse(fmt.Sprintf("invalid URL found in Authority Information Access (AIA) parameter issuing_certificates: %s", badURL)), nil
}
crlDistributionPoints := data.Get("crl_distribution_points").([]string)
if badURL := validateURLs(crlDistributionPoints); badURL != "" {
return logical.ErrorResponse(fmt.Sprintf("invalid URL found in Authority Information Access (AIA) parameter crl_distribution_points: %s", badURL)), nil
}
ocspServers := data.Get("ocsp_servers").([]string)
if badURL := validateURLs(ocspServers); badURL != "" {
return logical.ErrorResponse(fmt.Sprintf("invalid URL found in Authority Information Access (AIA) parameter ocsp_servers: %s", badURL)), nil
}

modified := false

var oldName string
Expand Down Expand Up @@ -331,6 +369,49 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da
modified = true
}

if issuer.AIAURIs == nil && (len(issuerCertificates) > 0 || len(crlDistributionPoints) > 0 || len(ocspServers) > 0) {
issuer.AIAURIs = &certutil.URLEntries{}
}
if issuer.AIAURIs != nil {
// Associative mapping from data source to destination on the
// backing issuer object.
type aiaPair struct {
Source *[]string
Dest *[]string
}
pairs := []aiaPair{
{
Source: &issuerCertificates,
Dest: &issuer.AIAURIs.IssuingCertificates,
},
{
Source: &crlDistributionPoints,
Dest: &issuer.AIAURIs.CRLDistributionPoints,
},
{
Source: &ocspServers,
Dest: &issuer.AIAURIs.OCSPServers,
},
}

// For each pair, if it is different on the object, update it.
for _, pair := range pairs {
if isStringArrayDifferent(*pair.Source, *pair.Dest) {
*pair.Dest = *pair.Source
modified = true
}
}

// If no AIA URLs exist on the issuer, set the AIA URLs entry to nil
// to ease usage later.
if len(issuer.AIAURIs.IssuingCertificates) == 0 && len(issuer.AIAURIs.CRLDistributionPoints) == 0 && len(issuer.AIAURIs.OCSPServers) == 0 {
issuer.AIAURIs = nil
}
}

// Updating the chain should be the last modification as there's a chance
// it'll write it out to disk for us. We'd hate to then modify the issuer
// again and write it a second time.
var updateChain bool
var constructedChain []issuerID
for index, newPathRef := range newPath {
Expand Down Expand Up @@ -511,6 +592,56 @@ func (b *backend) pathPatchIssuer(ctx context.Context, req *logical.Request, dat
}
}

// AIA access changes.
if issuer.AIAURIs == nil {
issuer.AIAURIs = &certutil.URLEntries{}
}

// Associative mapping from data source to destination on the
// backing issuer object. For PATCH requests, we use the source
// data parameter as we still need to validate them and process
// it into a string list.
type aiaPair struct {
Source string
Dest *[]string
}
pairs := []aiaPair{
{
Source: "issuing_certificates",
Dest: &issuer.AIAURIs.IssuingCertificates,
},
{
Source: "crl_distribution_points",
Dest: &issuer.AIAURIs.CRLDistributionPoints,
},
{
Source: "ocsp_servers",
Dest: &issuer.AIAURIs.OCSPServers,
},
}

// For each pair, if it is different on the object, update it.
for _, pair := range pairs {
rawURLsValue, ok := data.GetOk(pair.Source)
if ok {
urlsValue := rawURLsValue.([]string)
if badURL := validateURLs(urlsValue); badURL != "" {
return logical.ErrorResponse(fmt.Sprintf("invalid URL found in Authority Information Access (AIA) parameter %v: %s", pair.Source, badURL)), nil
}

if isStringArrayDifferent(urlsValue, *pair.Dest) {
modified = true
*pair.Dest = urlsValue
}
}
}

// If no AIA URLs exist on the issuer, set the AIA URLs entry to nil to
// ease usage later.
if len(issuer.AIAURIs.IssuingCertificates) == 0 && len(issuer.AIAURIs.CRLDistributionPoints) == 0 && len(issuer.AIAURIs.OCSPServers) == 0 {
issuer.AIAURIs = nil
}

// Manual Chain Changes
newPathData, ok := data.GetOk("manual_chain")
if ok {
Expand Down
4 changes: 2 additions & 2 deletions builtin/logical/pki/path_intermediate.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,12 @@ func (b *backend) pathGenerateIntermediate(ctx context.Context, req *logical.Req
Data: map[string]interface{}{},
}

entries, err := getURLs(ctx, req.Storage)
entries, err := getGlobalAIAURLs(ctx, req.Storage)
if err == nil && len(entries.OCSPServers) == 0 && len(entries.IssuingCertificates) == 0 && len(entries.CRLDistributionPoints) == 0 {
// If the operator hasn't configured any of the URLs prior to
// generating this issuer, we should add a warning to the response,
// informing them they might want to do so and re-generate the issuer.
resp.AddWarning("This mount hasn't configured any authority access information fields; this may make it harder for systems to find missing certificates in the chain or to validate revocation status of certificates. Consider updating /config/urls with this information.")
resp.AddWarning("This mount hasn't configured any authority information access (AIA) fields; this may make it harder for systems to find missing certificates in the chain or to validate revocation status of certificates. Consider updating /config/urls or the newly generated issuer with this information. Since this certificate is an intermediate, it might be useful to regenerate this certificate after fixing this problem for the root mount.")
}

switch format {
Expand Down
4 changes: 2 additions & 2 deletions builtin/logical/pki/path_manage_issuers.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,8 @@ func (b *backend) pathImportIssuers(ctx context.Context, req *logical.Request, d
// Also while we're here, we should let the user know the next steps.
// In particular, if there's no default AIA URLs configuration, we should
// tell the user that's probably next.
if entries, err := getURLs(ctx, req.Storage); err == nil && len(entries.IssuingCertificates) == 0 && len(entries.CRLDistributionPoints) == 0 && len(entries.OCSPServers) == 0 {
response.AddWarning("This mount hasn't configured any authority access information fields; this may make it harder for systems to find missing certificates in the chain or to validate revocation status of certificates. Consider updating /config/urls with this information.")
if entries, err := getGlobalAIAURLs(ctx, req.Storage); err == nil && len(entries.IssuingCertificates) == 0 && len(entries.CRLDistributionPoints) == 0 && len(entries.OCSPServers) == 0 {
response.AddWarning("This mount hasn't configured any authority information access (AIA) fields; this may make it harder for systems to find missing certificates in the chain or to validate revocation status of certificates. Consider updating /config/urls or the newly generated issuer with this information.")
}

return response, nil
Expand Down
6 changes: 3 additions & 3 deletions builtin/logical/pki/path_root.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request,
// If the operator hasn't configured any of the URLs prior to
// generating this issuer, we should add a warning to the response,
// informing them they might want to do so prior to issuing leaves.
resp.AddWarning("This mount hasn't configured any authority access information fields; this may make it harder for systems to find missing certificates in the chain or to validate revocation status of certificates. Consider updating /config/urls with this information.")
resp.AddWarning("This mount hasn't configured any authority information access (AIA) fields; this may make it harder for systems to find missing certificates in the chain or to validate revocation status of certificates. Consider updating /config/urls or the newly generated issuer with this information.")
}

switch format {
Expand Down Expand Up @@ -407,8 +407,8 @@ func (b *backend) pathIssuerSignIntermediate(ctx context.Context, req *logical.R
if len(parsedBundle.Certificate.OCSPServer) == 0 && len(parsedBundle.Certificate.IssuingCertificateURL) == 0 && len(parsedBundle.Certificate.CRLDistributionPoints) == 0 {
// If the operator hasn't configured any of the URLs prior to
// generating this issuer, we should add a warning to the response,
// informing them they might want to do so and re-generate the issuer.
resp.AddWarning("This mount hasn't configured any authority access information fields; this may make it harder for systems to find missing certificates in the chain or to validate revocation status of certificates. Consider updating /config/urls with this information.")
// informing them they might want to do so prior to issuing leaves.
resp.AddWarning("This mount hasn't configured any authority information access (AIA) fields; this may make it harder for systems to find missing certificates in the chain or to validate revocation status of certificates. Consider updating /config/urls or the newly generated issuer with this information.")
}

caChain := append([]string{cb.Certificate}, cb.CAChain...)
Expand Down
Loading