Skip to content
This repository has been archived by the owner on Dec 10, 2023. It is now read-only.

Commit

Permalink
Allow empty or absent subject block (hashicorp#209)
Browse files Browse the repository at this point in the history
* Update schema to allow `subject` to be optional or empty in `tls_cert_request` and `tls_self_signed_cert`

* Centralised the logic to handle `subject` detection and parsing

The only case to handle are:
1. is absent
2. is empty

in case it appear multiple times instead, we let the schema handle that

* Updated testing

* Regenerated doc

* Linter caught issues

* Updated CHANGELOG

* Tweaking negative tests error regexp to accomodate TF 0.12

* Better follow https://datatracker.ietf.org/doc/html/rfc5280#section-4.1.2.6

If the certificate we are creating is a CA, subject can't be empty

* Raise a warning when trying to create a `tls_locally_signed_cert` using a CA that is not a CA

The certificate will STILL be created, but the provider should signal that the resulting certificate could fail validation.

* PR review

* Parallelise run for GolangCI-Lint(er)

* Update CHANGELOG: mention warning now raised by `tls_locally_signed_cert` resource when providing a CA that is not a CA

* Apply suggestions from code review

Co-authored-by: Brian Flad <bflad417@gmail.com>

Co-authored-by: Brian Flad <bflad417@gmail.com>
  • Loading branch information
2 people authored and jackivanov committed Aug 4, 2022
1 parent 3ff0b88 commit d168eb4
Show file tree
Hide file tree
Showing 13 changed files with 463 additions and 102 deletions.
3 changes: 3 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,6 @@ linters:
- tenv
- unconvert
- unparam

run:
allow-parallel-runners: true
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ NEW FEATURES:
* data-source/tls_certificate: New attribute `content` that can be used in alternative to `url`, to provide the certificate in PEM format ([#189](https://github.com/hashicorp/terraform-provider-tls/pull/189)).
* data-source/tls_certificate: Objects in the `certificates` chain attribute expose a new attribute `cert_pem` (PEM format) ([#208](https://github.com/hashicorp/terraform-provider-tls/pull/208)).

ENHANCEMENTS:

* resource/tls_locally_signed_cert: If CA provided via `ca_cert_pem` is not an actual CA, a warning will be raised, but the certificate will still be created ([#209](https://github.com/hashicorp/terraform-provider-tls/pull/209)).

NOTES:

* data-source/tls_certificate: The `id` attribute has changed to the hashing of all certificates information in the chain. The first apply of this updated data source may show this difference ([#189](https://github.com/hashicorp/terraform-provider-tls/pull/189)).
Expand All @@ -13,6 +17,10 @@ BUG FIXES:

* data-source/tls_certificate: Prevent plan differences with the `id` attribute ([#79](https://github.com/hashicorp/terraform-provider-tls/issues/79), [#189](https://github.com/hashicorp/terraform-provider-tls/pull/189)).

* resource/tls_cert_request: Allow for absent or empty `subject` block ([#209](https://github.com/hashicorp/terraform-provider-tls/pull/209)).

* resource/tls_self_signed_cert: Allow for absent or empty `subject` block ([#209](https://github.com/hashicorp/terraform-provider-tls/pull/209)).

## 3.3.0 (April 07, 2022)

NEW FEATURES:
Expand Down
1 change: 1 addition & 0 deletions GNUmakefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ install: build

# See https://golangci-lint.run/
lint:
golangci-lint cache clean
golangci-lint run

generate:
Expand Down
2 changes: 1 addition & 1 deletion docs/resources/cert_request.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ resource "tls_cert_request" "example" {
### Required

- `private_key_pem` (String, Sensitive) Private key in [PEM (RFC 1421)](https://datatracker.ietf.org/doc/html/rfc1421) format, that the certificate will belong to. This can be read from a separate file using the [`file`](https://www.terraform.io/language/functions/file) interpolation function. Only an irreversible secure hash of the private key will be stored in the Terraform state.
- `subject` (Block List, Min: 1) The subject for which a certificate is being requested. The acceptable arguments are all optional and their naming is based upon [Issuer Distinguished Names (RFC5280)](https://tools.ietf.org/html/rfc5280#section-4.1.2.4) section. (see [below for nested schema](#nestedblock--subject))

### Optional

- `dns_names` (List of String) List of DNS names for which a certificate is being requested (i.e. certificate subjects).
- `ip_addresses` (List of String) List of IP addresses for which a certificate is being requested (i.e. certificate subjects).
- `key_algorithm` (String, Deprecated) Name of the algorithm used when generating the private key provided in `private_key_pem`. **NOTE**: this is deprecated and ignored, as the key algorithm is now inferred from the key.
- `subject` (Block List, Max: 1) The subject for which a certificate is being requested. The acceptable arguments are all optional and their naming is based upon [Issuer Distinguished Names (RFC5280)](https://tools.ietf.org/html/rfc5280#section-4.1.2.4) section. (see [below for nested schema](#nestedblock--subject))
- `uris` (List of String) List of URIs for which a certificate is being requested (i.e. certificate subjects).

### Read-Only
Expand Down
2 changes: 1 addition & 1 deletion docs/resources/self_signed_cert.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ resource "tls_self_signed_cert" "example" {

- `allowed_uses` (List of String) List of key usages allowed for the issued certificate. Values are defined in [RFC 5280](https://datatracker.ietf.org/doc/html/rfc5280) and combine flags defined by both [Key Usages](https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.3) and [Extended Key Usages](https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.12). Accepted values: `any_extended`, `cert_signing`, `client_auth`, `code_signing`, `content_commitment`, `crl_signing`, `data_encipherment`, `decipher_only`, `digital_signature`, `email_protection`, `encipher_only`, `ipsec_end_system`, `ipsec_tunnel`, `ipsec_user`, `key_agreement`, `key_encipherment`, `microsoft_commercial_code_signing`, `microsoft_kernel_code_signing`, `microsoft_server_gated_crypto`, `netscape_server_gated_crypto`, `ocsp_signing`, `server_auth`, `timestamping`.
- `private_key_pem` (String, Sensitive) Private key in [PEM (RFC 1421)](https://datatracker.ietf.org/doc/html/rfc1421) format, that the certificate will belong to. This can be read from a separate file using the [`file`](https://www.terraform.io/language/functions/file) interpolation function. Only an irreversible secure hash of the private key will be stored in the Terraform state.
- `subject` (Block List, Min: 1) The subject for which a certificate is being requested. The acceptable arguments are all optional and their naming is based upon [Issuer Distinguished Names (RFC5280)](https://tools.ietf.org/html/rfc5280#section-4.1.2.4) section. (see [below for nested schema](#nestedblock--subject))
- `validity_period_hours` (Number) Number of hours, after initial issuing, that the certificate will remain valid for.

### Optional
Expand All @@ -54,6 +53,7 @@ resource "tls_self_signed_cert" "example" {
- `is_ca_certificate` (Boolean) Is the generated certificate representing a Certificate Authority (CA) (default: `false`).
- `key_algorithm` (String, Deprecated) Name of the algorithm used when generating the private key provided in `private_key_pem`. **NOTE**: this is deprecated and ignored, as the key algorithm is now inferred from the key.
- `set_subject_key_id` (Boolean) Should the generated certificate include a [subject key identifier](https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.2) (default: `false`).
- `subject` (Block List, Max: 1) The subject for which a certificate is being requested. The acceptable arguments are all optional and their naming is based upon [Issuer Distinguished Names (RFC5280)](https://tools.ietf.org/html/rfc5280#section-4.1.2.4) section. (see [below for nested schema](#nestedblock--subject))
- `uris` (List of String) List of URIs for which a certificate is being requested (i.e. certificate subjects).

### Read-Only
Expand Down
45 changes: 32 additions & 13 deletions internal/provider/common_cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"strings"
"time"

"github.com/google/go-cmp/cmp"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
Expand Down Expand Up @@ -164,8 +165,9 @@ func setCertificateSubjectSchema(s map[string]*schema.Schema) {

s["subject"] = &schema.Schema{
Type: schema.TypeList,
Required: true,
Optional: true,
ForceNew: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"organization": {
Expand Down Expand Up @@ -353,6 +355,12 @@ func createCertificate(d *schema.ResourceData, template, parent *x509.Certificat
}

if d.Get("is_ca_certificate").(bool) {
// NOTE: if the Certificate we are trying to create is a Certificate Authority,
// then https://datatracker.ietf.org/doc/html/rfc5280#section-4.1.2.6 requires
// the `.Subject` to contain at least 1 Distinguished Name.
if cmp.Equal(template.Subject, pkix.Name{}) {
return diag.Errorf("Certificate Subject must contain at least one Distinguished Name when creating Certificate Authority (CA)")
}
template.IsCA = true

template.SubjectKeyId, err = generateSubjectKeyID(pub)
Expand Down Expand Up @@ -448,39 +456,50 @@ func updateCertificate(_ context.Context, _ *schema.ResourceData, _ interface{})
return nil
}

// distinguishedNamesFromSubjectAttributes it takes a map subject attributes and
// converts it to a pkix.Name (X.509 distinguished names).
func distinguishedNamesFromSubjectAttributes(nameMap map[string]interface{}) *pkix.Name {
// createSubjectDistinguishedNames return a *pkix.Name (i.e. a "Certificate Subject") if the non-empty.
// This used for creating x509.Certificate or x509.CertificateRequest.
func createSubjectDistinguishedNames(subjectList []interface{}) *pkix.Name {
if len(subjectList) == 0 {
// No subject block was provided
return nil
}

subject, ok := subjectList[0].(map[string]interface{})
if !ok {
// Empty subject block was provided
return nil
}

result := &pkix.Name{}

if value := nameMap["common_name"]; value != "" {
if value := subject["common_name"]; value != "" {
result.CommonName = value.(string)
}
if value := nameMap["organization"]; value != "" {
if value := subject["organization"]; value != "" {
result.Organization = []string{value.(string)}
}
if value := nameMap["organizational_unit"]; value != "" {
if value := subject["organizational_unit"]; value != "" {
result.OrganizationalUnit = []string{value.(string)}
}
if value := nameMap["street_address"].([]interface{}); len(value) > 0 {
if value := subject["street_address"].([]interface{}); len(value) > 0 {
result.StreetAddress = make([]string, len(value))
for i, vi := range value {
result.StreetAddress[i] = vi.(string)
}
}
if value := nameMap["locality"]; value != "" {
if value := subject["locality"]; value != "" {
result.Locality = []string{value.(string)}
}
if value := nameMap["province"]; value != "" {
if value := subject["province"]; value != "" {
result.Province = []string{value.(string)}
}
if value := nameMap["country"]; value != "" {
if value := subject["country"]; value != "" {
result.Country = []string{value.(string)}
}
if value := nameMap["postal_code"]; value != "" {
if value := subject["postal_code"]; value != "" {
result.PostalCode = []string{value.(string)}
}
if value := nameMap["serial_number"]; value != "" {
if value := subject["serial_number"]; value != "" {
result.SerialNumber = value.(string)
}

Expand Down
17 changes: 6 additions & 11 deletions internal/provider/resource_cert_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,13 @@ func createCertRequest(_ context.Context, d *schema.ResourceData, _ interface{})
return diag.Errorf("error setting value on key 'key_algorithm': %s", err)
}

subjectConfs := d.Get("subject").([]interface{})
if len(subjectConfs) != 1 {
return diag.Errorf("must have exactly one 'subject' block")
}
subjectConf, ok := subjectConfs[0].(map[string]interface{})
if !ok {
return diag.Errorf("subject block cannot be empty")
}
subject := distinguishedNamesFromSubjectAttributes(subjectConf)
// Look for a 'subject' block
subject := createSubjectDistinguishedNames(d.Get("subject").([]interface{}))

certReq := x509.CertificateRequest{
Subject: *subject,
// Add a `Subject` to the `Certificate` only if it was provided
certReq := x509.CertificateRequest{}
if subject != nil {
certReq.Subject = *subject
}

dnsNamesI := d.Get("dns_names").([]interface{})
Expand Down
Loading

0 comments on commit d168eb4

Please sign in to comment.