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

Implemented tls_x509_crl resource #73

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ services:
- docker
language: go
go:
- "1.11.x"
- "1.14.x"

env:
- GO111MODULE=on GOFLAGS=-mod=vendor
Expand Down
2 changes: 1 addition & 1 deletion GNUmakefile
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ build: fmtcheck
go install

test: fmtcheck
go test -i $(TEST) || exit 1
go test $(TEST) || exit 1
echo $(TEST) | \
xargs -t -n4 go test $(TESTARGS) -timeout=30s -parallel=4

Expand Down
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
module github.com/terraform-providers/terraform-provider-tls

go 1.14

require (
github.com/hashicorp/terraform-plugin-sdk v1.0.0
golang.org/x/crypto v0.0.0-20190820162420-60c769a6c586
Expand Down
1 change: 1 addition & 0 deletions tls/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ func Provider() terraform.ResourceProvider {
"tls_locally_signed_cert": resourceLocallySignedCert(),
"tls_self_signed_cert": resourceSelfSignedCert(),
"tls_cert_request": resourceCertRequest(),
"tls_x509_crl": resourceCertRevocationList(),
},
DataSourcesMap: map[string]*schema.Resource{
"tls_public_key": dataSourcePublicKey(),
Expand Down
20 changes: 20 additions & 0 deletions tls/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,26 @@ UI+cIbsQJcr9i0SADwhqs7XYIcEz9jDqrdezenaXdGtXYg==
-----END CERTIFICATE-----
`

var testCertificate = `
fllaca marked this conversation as resolved.
Show resolved Hide resolved
-----BEGIN CERTIFICATE-----
MIICsDCCAhkCAQIwDQYJKoZIhvcNAQELBQAwezELMAkGA1UEBhMCVVMxCzAJBgNV
BAgTAkNBMRYwFAYDVQQHEw1QaXJhdGUgSGFyYm9yMRUwEwYDVQQKEwxFeGFtcGxl
LCBJbmMxITAfBgNVBAsTGERlcGFydG1lbnQgb2YgQ0EgVGVzdGluZzENMAsGA1UE
AxMEcm9vdDAeFw0yMDAzMjkxNDM0MjFaFw0yMTAzMjkxNDM0MjFaMIHFMQswCQYD
VQQGEwJVUzELMAkGA1UECBMCQ0ExFjAUBgNVBAcTDVBpcmF0ZSBIYXJib3IxGTAX
BgNVBAkTEDU4NzkgQ290dG9uIExpbmsxEzARBgNVBBETCjk1NTU5LTEyMjcxFTAT
BgNVBAoTDEV4YW1wbGUsIEluYzEoMCYGA1UECxMfRGVwYXJ0bWVudCBvZiBUZXJy
YWZvcm0gVGVzdGluZzEUMBIGA1UEAxMLZXhhbXBsZS5jb20xCjAIBgNVBAUTATIw
gZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAM8tqrjcP0Ln3qSj3JCKlZR/YVtG
EWzXwPfAEMGpcsig/tbeublLb3b8mI09r6ZfvohgUXjXrNMEYb0nnDh3hUWtE6fw
i4xaoeB1ewJR/azM3sSclxUGwHNPHFarPgONffXm9ub88U4uUrMLQ2CziHWIOIIP
0eTYxqNm4WcmKqPVAgMBAAEwDQYJKoZIhvcNAQELBQADgYEAPIoSmOKjTu4frJpQ
wWUYrXgf63Ft7zbgPXkru8R4KswMQm3016sWibDfTkwLTp/Xli3AgVcE02cY+QQQ
/V+QZVWoZnQN7d6KPE7f1HiWAKOdP2/zWX3Ci/tvDkQUvpOUoZoRImoZKx+WwNC3
P+itMZWh3jPF4tj4WYHUnjyZqdk=
-----END CERTIFICATE-----
`

func setTimeForTest(timeStr string) func() {
return func() {
now = func() time.Time {
Expand Down
193 changes: 193 additions & 0 deletions tls/resource_cert_revocation_list.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
package tls

import (
"crypto/rand"
"crypto/x509"
"crypto/x509/pkix"
"encoding/pem"
"strings"
"time"

"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
)

func resourceCertRevocationList() *schema.Resource {
s := map[string]*schema.Schema{
"certs_to_revoke": &schema.Schema{
Type: schema.TypeList,
Elem: &schema.Schema{
Type: schema.TypeString,
},
Required: true,
Description: "PEM-encoded certificates to be revoked",
ForceNew: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

While technically correct, it would be better, if we would update the list of certificates in the CRL, rather than create the new one every time, as then the expiry date will be updated too for example.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah... but I'm quite unsure on how to handle that lifecycle 🤔 , for instance the Id of the resource is calculated as a hash of the CRL PEM content (in the same way we do for locally signed certificates and other resources in this provider). When a CRL is "updated" actually what happens is that a new version of it is created, basically a new CRL itself. I was checking https://tools.ietf.org/html/rfc5280#section-5.1 trying to find a field that we can use to identify a CRL independently of the version, but I couldn't find any. But I can miss something, do you have any idea around this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, I'm not sure if I understand the problem. If you set the ID only in Create function and then don't touch it in Update, then what's the problem? I don't think there is a need to change the ID.

Aside from that, I think this is completely fine for the initial implementation. It can always be improved later 👍

StateFunc: func(v interface{}) string {
return hashForState(strings.Join(v.([]string), ","))
},
},
"early_renewal_hours": {
Type: schema.TypeInt,
Optional: true,
Default: 0,
Description: "Number of hours before the certificates expiry when a new certificate will be generated",
fllaca marked this conversation as resolved.
Show resolved Hide resolved
},
"validity_period_hours": {
Type: schema.TypeInt,
Required: true,
Description: "Number of hours that the CRL will remain valid for",
ForceNew: true,
},
"validity_start_time": {
Type: schema.TypeString,
Computed: true,
},

"validity_end_time": {
Type: schema.TypeString,
Computed: true,
},
"ready_for_renewal": {
Type: schema.TypeBool,
Computed: true,
},
"crl_pem": {
Type: schema.TypeString,
Computed: true,
},
"ca_cert_pem": &schema.Schema{
Type: schema.TypeString,
Required: true,
Description: "PEM-encoded CA certificate",
ForceNew: true,
StateFunc: func(v interface{}) string {
return hashForState(v.(string))
},
},
"ca_private_key_pem": &schema.Schema{
Type: schema.TypeString,
Required: true,
Description: "PEM-encoded CA private key used to sign the CRL",
ForceNew: true,
Sensitive: true,
StateFunc: func(v interface{}) string {
return hashForState(v.(string))
},
},
"ca_key_algorithm": &schema.Schema{
Type: schema.TypeString,
Required: true,
Description: "Name of the algorithm used to generate the certificate's private key",
fllaca marked this conversation as resolved.
Show resolved Hide resolved
ForceNew: true,
},
}

return &schema.Resource{
Create: CreateCRL,
Delete: DeleteCRL,
Read: ReadCRL,
Update: UpdateCRL,
CustomizeDiff: CustomizeCertificateDiff,
Schema: s,
}
}

func CreateCRL(d *schema.ResourceData, meta interface{}) error {
certsToRevoke := make([]pkix.RevokedCertificate, 0)
if value := d.Get("certs_to_revoke").([]interface{}); len(value) > 0 {
fllaca marked this conversation as resolved.
Show resolved Hide resolved
for _, vi := range value {
block, err := decodePEMFromBytes([]byte(vi.(string)), "")
if err != nil {
return err
}
certificate, err := x509.ParseCertificate(block.Bytes)
fllaca marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
}
certsToRevoke = append(certsToRevoke, pkix.RevokedCertificate{
SerialNumber: certificate.SerialNumber,
})
}
}
caKey, err := parsePrivateKey(d, "ca_private_key_pem", "ca_key_algorithm")
if err != nil {
return err
fllaca marked this conversation as resolved.
Show resolved Hide resolved
}
caCert, err := parseCertificate(d, "ca_cert_pem")
if err != nil {
return err
}

notBefore := now()
notAfter := notBefore.Add(time.Duration(d.Get("validity_period_hours").(int)) * time.Hour)
validFromBytes, err := notBefore.MarshalText()
if err != nil {
return err
}
validToBytes, err := notAfter.MarshalText()
if err != nil {
return err
}

crlBytes, err := caCert.CreateCRL(rand.Reader, caKey, certsToRevoke, notBefore, notAfter)
if err != nil {
return err
}

crlPem := string(pem.EncodeToMemory(&pem.Block{Type: "X509 CRL", Bytes: crlBytes}))
if err != nil {
fllaca marked this conversation as resolved.
Show resolved Hide resolved
return err
}
d.SetId(hashForState(string(crlBytes)))
d.Set("crl_pem", crlPem)
d.Set("ready_for_renewal", false)
d.Set("validity_start_time", string(validFromBytes))
d.Set("validity_end_time", string(validToBytes))
return nil
}

func DeleteCRL(d *schema.ResourceData, meta interface{}) error {
d.SetId("")
return nil
}

func ReadCRL(d *schema.ResourceData, meta interface{}) error {
return nil
}

func UpdateCRL(d *schema.ResourceData, meta interface{}) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this function can be removed for now, if we don't support updates.

Copy link
Author

Choose a reason for hiding this comment

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

It was kept because of the early_renewal_hours field. If we make the resource non-updatable that field needs to be flagged with ForceNew: true. Does it make sense?

return nil
}

func CustomizeCrlDiff(d *schema.ResourceDiff, meta interface{}) error {
var readyForRenewal bool

endTimeStr := d.Get("validity_end_time").(string)
endTime := now()
err := endTime.UnmarshalText([]byte(endTimeStr))
if err != nil {
// If end time is invalid then we'll treat it as being at the time for renewal.
readyForRenewal = true
} else {
earlyRenewalPeriod := time.Duration(-d.Get("early_renewal_hours").(int)) * time.Hour
endTime = endTime.Add(earlyRenewalPeriod)

currentTime := now()
timeToRenewal := endTime.Sub(currentTime)
if timeToRenewal <= 0 {
readyForRenewal = true
}
}

if readyForRenewal {
fllaca marked this conversation as resolved.
Show resolved Hide resolved
err = d.SetNew("ready_for_renewal", true)
if err != nil {
return err
}
err = d.ForceNew("ready_for_renewal")
if err != nil {
return err
}
}

return nil
}
89 changes: 89 additions & 0 deletions tls/resource_cert_revocation_list_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package tls

import (
"crypto/x509"
"encoding/pem"

//"errors"
fllaca marked this conversation as resolved.
Show resolved Hide resolved
"fmt"
"strings"
"testing"

//"time"

r "github.com/hashicorp/terraform-plugin-sdk/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/terraform"
)

func TestCRL(t *testing.T) {
r.UnitTest(t, r.TestCase{
Providers: testProviders,
Steps: []r.TestStep{
{
Config: certRevocationListConfig(1, 0),
Check: func(s *terraform.State) error {
gotUntyped := s.RootModule().Outputs["crl_pem"].Value
got, ok := gotUntyped.(string)
if !ok {
return fmt.Errorf("output for \"crl_pem\" is not a string")
}
if !strings.HasPrefix(got, "-----BEGIN X509 CRL----") {
return fmt.Errorf("key is missing cert PEM preamble")
fllaca marked this conversation as resolved.
Show resolved Hide resolved
}
block, _ := pem.Decode([]byte(got))
crl, err := x509.ParseCRL(block.Bytes)
if err != nil {
return fmt.Errorf("error parsing crl: %s", err)
}

// Verify testCertificate is in the CRL serial number list
fllaca marked this conversation as resolved.
Show resolved Hide resolved
testCertBlock, _ := pem.Decode([]byte(testCertificate))
testCert, err := x509.ParseCertificate(testCertBlock.Bytes)
if err != nil {
return fmt.Errorf("error parsing test cert: %s", err)
}
if testCert.SerialNumber.Cmp(crl.TBSCertList.RevokedCertificates[0].SerialNumber) != 0 {
return fmt.Errorf("revoked certificate serial number doesn't match")
}

// Verify CRL signature with CA
fllaca marked this conversation as resolved.
Show resolved Hide resolved
caBlock, _ := pem.Decode([]byte(testCACert))
caCert, err := x509.ParseCertificate(caBlock.Bytes)
if err != nil {
return fmt.Errorf("error parsing ca cert: %s", err)
}
err = caCert.CheckCRLSignature(crl)
if err != nil {
return fmt.Errorf("Wrong CRL signature %s", err)
}
return nil
},
},
},
})
}

func certRevocationListConfig(validity uint32, earlyRenewal uint32) string {
return fmt.Sprintf(`
locals {
cert_to_revoke = <<EOT
fllaca marked this conversation as resolved.
Show resolved Hide resolved
%s
EOT
}
resource "tls_x509_crl" "test" {
certs_to_revoke = [local.cert_to_revoke]

validity_period_hours = %d
ca_cert_pem = <<EOT
%s
EOT
ca_key_algorithm = "RSA"
ca_private_key_pem = <<EOT
%s
EOT
}
output "crl_pem" {
value = "${tls_x509_crl.test.crl_pem}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto ${}.

}
`, testCertificate, validity, testCACert, testCAPrivateKey)
}
15 changes: 12 additions & 3 deletions tls/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,21 @@ import (
)

func decodePEM(d *schema.ResourceData, pemKey, pemType string) (*pem.Block, error) {
block, _ := pem.Decode([]byte(d.Get(pemKey).(string)))
block, err := decodePEMFromBytes([]byte(d.Get(pemKey).(string)), pemType)
if err != nil {
return nil, fmt.Errorf("Error decoding PEM: %s", err.Error())
}

return block, nil
}

func decodePEMFromBytes(data []byte, pemType string) (*pem.Block, error) {
block, _ := pem.Decode(data)
if block == nil {
return nil, fmt.Errorf("no PEM block found in %s", pemKey)
return nil, fmt.Errorf("no PEM block found")
}
if pemType != "" && block.Type != pemType {
return nil, fmt.Errorf("invalid PEM type in %s: %s", pemKey, block.Type)
return nil, fmt.Errorf("invalid PEM type %s", block.Type)
}

return block, nil
Expand Down
Loading