Merge pull request #103 from Venafi/prevent-reissuance

Prevent reissuance

CHANGELOG.md

@@ -1,3 +1,6 @@
+# v0.10.5 (August 30, 2022)
+Added feature in order to prevent an issuance of the certificate if it is already inside Vault storage 
+
 # v.0.10.4 (May 27, 2022)
 Fixed a thread locking bug
 

README.md

@@ -343,6 +343,33 @@ reload_id    d8180af4-01e0-d4d8-10ce-0daf69fbb6ed
 with the same version of the plugin to avoid inconsistent, unexpected, and
 possibly erroneous results.
 
+## Prevent Re-issue
+
+In order to prevent an issuance of a new certificate if current certificate exists in Vault's storage, we added a capability
+to return that certificate instead. To issue this feature you must set:
+
+- `min_cert_time_left`: Golang's duration format string (e.g. 24h, 23h5m20s, 10000s, etc.)
+- `store_by="serial"`
+- `store_pkey=true`
+
+If certificate was successfully loaded from Vault storage, you will encounter `Loading certificate from storage` message
+in logs when `[DEBUG]` mode is set:
+
+```
+2022-08-30T13:41:49.007-0500 [DEBUG] secrets.venafi-pki-backend.venafi-pki-backend_5df77702.venafi-pki-backend.venafi-pki-backend: Loading certificate from storage: timestamp=2022-08-30T13:41:49.006-0500
+2022-08-30T13:41:49.008-0500 [DEBUG] secrets.venafi-pki-backend.venafi-pki-backend_5df77702.venafi-pki-backend.venafi-pki-backend: Getting venafi certificate: timestamp=2022-08-30T13:41:49.008-0500
+2022-08-30T13:41:49.010-0500 [DEBUG] secrets.venafi-pki-backend.venafi-pki-backend_5df77702.venafi-pki-backend.venafi-pki-backend: certificate is:-----BEGIN CERTIFICATE-----
+MIIHvjCCBaagAwIBAgITbQCpUfV8kBfjsOaP8QAAAKlR9TANBgkqhkiG9w0BAQsF
+ADBbMRMwEQYKCZImiZPyLGQBGRYDY29tMRYwFAYKCZImiZPyLGQBGRYGdmVuYWZp
+MRUwEwYKCZImiZPyLGQBGRYFdmVucWExFTATBgNVBAMTDFFBIFZlbmFmaSBDQTAe
+Fw0yMjA4MzAxODMxNDNaFw0yNDA4MjkxODMxNDNaMIHAMQswCQYDVQQGEwJVUzEN
+MAsGA1UECBMEVXRhaDEXMBUGA1UEBxMOU2FsdCBMYWtlIENpdHkxFDASBgNVBAoT
+C1ZlbmFmaSBJbmMuMRQwEgYDVQQLEwtFbmdpbmVlcmluZzEbMBkGA1UECxMSUHJv
+ZHVjdCBNYW5hZ2VtZW50MRowGAYDVQQLExFRdWFsaXR5IEFzc3VyYW5jZTEkMCIG
+A1UEAxMbbm9wcml2YXRla2V5LnZlbmFmaS5leGFtcGxlMIIBIjANBgkqhkiG9w0B
+```
+
+
 ## License
 
 Copyright © Venafi, Inc. All rights reserved.

go.mod

@@ -4,7 +4,7 @@ go 1.13
 
 require (
 	github.com/Venafi/vcert v3.18.4+incompatible
-	github.com/Venafi/vcert/v4 v4.20.1
+	github.com/Venafi/vcert/v4 v4.22.0
 	github.com/hashicorp/go-hclog v0.14.1
 	github.com/hashicorp/vault/api v1.0.4
 	github.com/hashicorp/vault/sdk v0.1.13

go.sum

@@ -16,8 +16,8 @@ github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym
 github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU=
 github.com/Venafi/vcert v3.18.4+incompatible h1:mDXSjd+EpXa8YEkEo9Oad19E270aiPJJMhjoKs63b+8=
 github.com/Venafi/vcert v3.18.4+incompatible/go.mod h1:3dpfrCI+31cDZosD+1UX8GFziVFORaegByXtzT1dwNo=
-github.com/Venafi/vcert/v4 v4.20.1 h1:dPUJ7XZW7SXxM9ZA4qBw535mYdU5YlapOgrhFmByqoE=
-github.com/Venafi/vcert/v4 v4.20.1/go.mod h1:4Nec3twWisOdS1unpDZ93sfau9eVSDS8Ot+Ry/gg0es=
+github.com/Venafi/vcert/v4 v4.22.0 h1:trH5eftOQ3cKgGFenMGFZ62yfITeunOSF9zx2xpZ1g8=
+github.com/Venafi/vcert/v4 v4.22.0/go.mod h1:4Nec3twWisOdS1unpDZ93sfau9eVSDS8Ot+Ry/gg0es=
 github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
 github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
 github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e/go.mod h1:3U/XgcO3hCbHZ8TKRvWD2dDTCfh9M9ya+I9JpbB7O8o=

plugin/pki/env_test.go

@@ -7,7 +7,9 @@ import (
 	"crypto/tls"
 	"crypto/x509"
 	"encoding/pem"
+	"errors"
 	"fmt"
+	"github.com/Venafi/vault-pki-backend-venafi/plugin/pki/vpkierror"
 	"github.com/Venafi/vcert/v4/pkg/util"
 	"log"
 	"math/rand"
@@ -911,20 +913,19 @@ func (e *testEnv) ReadCertificate(t *testing.T, data testData, configString vena
 
 func (e *testEnv) CheckThatThereIsNoCertificate(t *testing.T, certId string) {
 
+	path := "cert/" + certId
 	_, err := e.Backend.HandleRequest(e.Context, &logical.Request{
 		Operation: logical.ReadOperation,
-		Path:      "cert/" + certId,
+		Path:      path,
 		Storage:   e.Storage,
 	})
 
 	if err == nil {
 		t.Fatal("should be no entry error if there is no certificate")
 	}
 
-	const noCertError = "no entry found in path"
-	certContain := strings.Contains(err.Error(), noCertError)
-	if !certContain {
-		t.Fatalf("error should contain %s substring but it is %s", noCertError, err)
+	if !errors.As(err, &vpkierror.CertEntryNotFound{}) {
+		t.Fatalf("error should contain %s substring but it is %s", vpkierror.CertEntryNotFound{EntryPath: path}, err.Error())
 	}
 
 }

plugin/pki/path_roles.go

@@ -114,7 +114,7 @@ attached to them. Defaults to "false".`,
 			},
 			"server_timeout": {
 				Type:        framework.TypeInt,
-				Description: "Timeout of waiting certificate",
+				Description: "Timeout of waiting certificate (seconds)",
 				Default:     180,
 			},
 			"venafi_secret": {
@@ -127,6 +127,10 @@ attached to them. Defaults to "false".`,
 				Description: `When true, settings of an existing role will be retained unless they are specified in the update.
                               By default unspecified settings are returned to their default values`,
 			},
+			"min_cert_time_left": {
+				Type:        framework.TypeDurationSecond,
+				Description: `When set, is used to determinate if certificate issuance is needed comparing certificate validity against desired remaining validity`,
+			},
 		},
 
 		Operations: map[logical.Operation]framework.OperationHandler{
@@ -322,6 +326,12 @@ func (b *backend) pathRoleUpdate(ctx context.Context, req *logical.Request, data
 		entry.Zone = zone
 	}
 
+	_, isSet = data.GetOk("min_cert_time_left")
+	minCertTimeLeft := time.Duration(data.Get("min_cert_time_left").(int)) * time.Second
+	if isSet && (entry.MinCertTimeLeft != minCertTimeLeft) {
+		entry.MinCertTimeLeft = minCertTimeLeft
+	}
+
 	err = validateEntry(entry)
 	if err != nil {
 		return nil, err
@@ -363,6 +373,7 @@ func (b *backend) pathRoleCreate(ctx context.Context, req *logical.Request, data
 			ServerTimeout:    time.Duration(data.Get("server_timeout").(int)) * time.Second,
 			VenafiSecret:     data.Get("venafi_secret").(string),
 			Zone:             data.Get("zone").(string),
+			MinCertTimeLeft:  time.Duration(data.Get("min_cert_time_left").(int)) * time.Second,
 		}
 	}
 
@@ -480,6 +491,7 @@ type roleEntry struct {
 	ServerTimeout    time.Duration `json:"server_timeout"`
 	VenafiSecret     string        `json:"venafi_secret"`
 	Zone             string        `json:"zone"`
+	MinCertTimeLeft  time.Duration `json:"min_cert_time_left"`
 }
 
 func (r *roleEntry) ToResponseData() map[string]interface{} {
@@ -495,6 +507,7 @@ func (r *roleEntry) ToResponseData() map[string]interface{} {
 		"max_ttl":                int64(r.MaxTTL.Seconds()),
 		"generate_lease":         r.GenerateLease,
 		"chain_option":           r.ChainOption,
+		"min_cert_time_left":     shortDurationString(r.MinCertTimeLeft),
 	}
 	return responseData
 }

plugin/pki/path_venafi_cert_enroll.go

@@ -7,9 +7,12 @@ import (
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/pem"
+	"errors"
 	"fmt"
+	"github.com/Venafi/vault-pki-backend-venafi/plugin/pki/vpkierror"
 	"github.com/Venafi/vcert/v4/pkg/endpoint"
 	"github.com/Venafi/vcert/v4/pkg/util"
+	"github.com/Venafi/vcert/v4/pkg/verror"
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/vault/sdk/helper/consts"
 	"net"
@@ -221,7 +224,16 @@ func (b *backend) pathVenafiCertObtain(ctx context.Context, req *logical.Request
 
 	}
 
-	certReq, err = formRequest(reqData, role, signCSR, b.Logger())
+	if &role.MinCertTimeLeft != nil && role.MinCertTimeLeft != time.Duration(0) && role.StorePrivateKey == true && (role.StoreBy == storeBySerialString || role.StoreBySerial == true) {
+		// if we don't receive a logic response, whenever is an error or the actual certificate found in storage
+		// means we need to issue a new one
+		logicalResp := preventReissue(b, ctx, req, &reqData, &cl, role, roleName)
+		if logicalResp != nil {
+			return logicalResp, nil
+		}
+	}
+
+	certReq, err = formRequest(reqData, role, &cl, signCSR, b.Logger())
 	if err != nil {
 		return logical.ErrorResponse(err.Error()), nil
 	}
@@ -478,26 +490,96 @@ func issueCertificate(certReq *certificate.Request, keyPass string, cl endpoint.
 	return pemCollection, nil
 }
 
-func formRequest(reqData requestData, role *roleEntry, signCSR bool, logger hclog.Logger) (certReq *certificate.Request, err error) {
+func preventReissue(b *backend, ctx context.Context, req *logical.Request, reqData *requestData, cl *endpoint.Connector, role *roleEntry, roleName string) *logical.Response {
+	b.Logger().Debug("Preventing re-issuance if certificate is already stored")
+	cfg, err := b.getConfig(ctx, req, roleName, false)
+	if err != nil {
+		return logical.ErrorResponse(err.Error())
+	}
+	b.Logger().Debug("Looking if certificate exist in the platform")
+	var certInfo *certificate.CertificateInfo
+	// creating new variables, so we don't mess up with reqData values since we may send that during request or modify them during issuing operation
+	commonName := reqData.commonName
+	sans := &certificate.Sans{
+		DNS: reqData.altNames,
+	}
+	// During search, if VaaS doesn't provide the CN and the CIT restricts the CN, then we will return an error since it's not supported.
+	if (*cl).GetType() == endpoint.ConnectorTypeTPP {
+		// if the CN is not inside the SAN DNS List, we added in order to search for since this functionality
+		// is also added formRequest function of this package
+		if !sliceContains(sans.DNS, commonName) && commonName != "" { // Go can compare if en empty string exist in the slice, so we omit that case
+			b.Logger().Debug(fmt.Sprintf("Adding CN %s to SAN %s because it wasn't included.", reqData.commonName, reqData.altNames))
+			sans.DNS = append(sans.DNS, commonName)
+		}
+	}
+
+	certInfo, err = (*cl).SearchCertificate(cfg.Zone, commonName, sans, role.MinCertTimeLeft)
+	if err != nil && !(err == verror.NoCertificateFoundError || err == verror.NoCertificateWithMatchingZoneFoundError) {
+		return logical.ErrorResponse(err.Error())
+	}
+	if certInfo != nil {
+		b.Logger().Debug("Looking for certificate in storage")
+		serialNumber, err := addSeparatorToHexFormattedString(certInfo.Serial, ":")
+		if err != nil {
+			return logical.ErrorResponse(err.Error())
+		}
+		serialNormalized := normalizeSerial(serialNumber)
+		b.Logger().Debug("Loading certificate from storage")
+		cert, err := loadCertificateFromStorage(b, ctx, req, serialNormalized, reqData.keyPassword)
+		// We want to ignore error from plugin that is related to the certificate not being in storage. If it is
+		// we would like to issue a new one instead
+		if err != nil && !(errors.As(err, &vpkierror.CertEntryNotFound{})) {
+			return logical.ErrorResponse(err.Error())
+		}
+		if cert != nil && cert.PrivateKey != "" {
+			respData := map[string]interface{}{
+				"certificate_uid":   serialNormalized,
+				"serial_number":     cert.SerialNumber,
+				"certificate_chain": cert.CertificateChain,
+				"certificate":       cert.Certificate,
+				"private_key":       cert.PrivateKey,
+			}
+			var logResp *logical.Response
+			logResp = b.Secret(SecretCertsType).Response(
+				respData,
+				map[string]interface{}{
+					"serial_number": serialNumber,
+				})
+			return logResp
+		}
+		// if we arrive here it means that we could NOT find a certificate in storage, so we ignored previous error
+		// and we are going to exit the prevent-reissue code block and we will try to issue a new certificate
+	}
+	// if certInfo is equal to nil but we arrived here, means we skipped the error (since VCert returns error if certificate is not found,
+	// so we won't try to open storage and we will issue a new certificate
+	return nil
+}
+
+func formRequest(reqData requestData, role *roleEntry, cl *endpoint.Connector, signCSR bool, logger hclog.Logger) (certReq *certificate.Request, err error) {
 	if !signCSR {
-		if len(reqData.commonName) == 0 && len(reqData.altNames) == 0 {
+		if len(reqData.altNames) == 0 && reqData.commonName == "" {
 			return certReq, fmt.Errorf("no domains specified on certificate")
 		}
-		if len(reqData.commonName) == 0 && len(reqData.altNames) > 0 {
-			reqData.commonName = reqData.altNames[0]
-		}
-		if !sliceContains(reqData.altNames, reqData.commonName) {
+		if !sliceContains(reqData.altNames, reqData.commonName) && reqData.commonName != "" { // Go can compare if en empty string exist in the slice, so we omit that case
 			logger.Debug(fmt.Sprintf("Adding CN %s to SAN %s because it wasn't included.", reqData.commonName, reqData.altNames))
 			reqData.altNames = append(reqData.altNames, reqData.commonName)
 		}
+
 		certReq = &certificate.Request{
-			Subject: pkix.Name{
-				CommonName: reqData.commonName,
-			},
 			CsrOrigin:   certificate.LocalGeneratedCSR,
 			KeyPassword: reqData.keyPassword,
 		}
 
+		if reqData.commonName != "" {
+			certReq.Subject = pkix.Name{
+				CommonName: reqData.commonName,
+			}
+		}
+
+		if len(reqData.commonName) == 0 && len(reqData.altNames) > 0 && (*cl).GetType() == endpoint.ConnectorTypeTPP {
+			certReq.FriendlyName = reqData.altNames[0]
+		}
+
 		if role.ServiceGenerated {
 			certReq.CsrOrigin = certificate.ServiceGeneratedCSR
 		}

plugin/pki/path_venafi_cert_enroll_test.go

@@ -1,6 +1,7 @@
 package pki
 
 import (
+	"github.com/Venafi/vcert/v4"
 	"testing"
 )
 
@@ -18,7 +19,11 @@ func TestOriginInRequest(t *testing.T) {
 	role.KeyType = "rsa"
 	role.ChainOption = "first"
 
-	certReq, err := formRequest(data, &role, signCSR, integrationTestEnv.Backend.Logger())
+	// The purpose of this test is to verify the customField Utility, regardless of connector Type
+	cfg := &vcert.Config{}
+	client, err := vcert.NewClient(cfg)
+
+	certReq, err := formRequest(data, &role, &client, signCSR, integrationTestEnv.Backend.Logger())
 	if certReq.CustomFields[0].Value != utilityName {
 		t.Fatalf("Expected %s in request custom fields origin", utilityName)
 	}

plugin/pki/path_venafi_cert_read.go

@@ -2,7 +2,6 @@ package pki
 
 import (
 	"context"
-	"fmt"
 	"github.com/hashicorp/vault/sdk/framework"
 	"github.com/hashicorp/vault/sdk/logical"
 )
@@ -37,26 +36,12 @@ func (b *backend) pathVenafiCertRead(ctx context.Context, req *logical.Request,
 		return logical.ErrorResponse("no common name specified on certificate"), nil
 	}
 
-	path := "certs/" + certUID
+	keyPassword := data.Get("key_password").(string)
 
-	entry, err := req.Storage.Get(ctx, path)
+	cert, err := loadCertificateFromStorage(b, ctx, req, certUID, keyPassword)
 	if err != nil {
-		return nil, fmt.Errorf("failed to read Venafi certificate: %s", err)
-	}
-
-	if entry == nil {
-		return nil, fmt.Errorf("no entry found in path %s", path)
-	}
-
-	var cert VenafiCert
-	b.Logger().Debug("Getting venafi certificate")
-
-	if err := entry.DecodeJSON(&cert); err != nil {
-		b.Logger().Error("error reading venafi configuration: %s", err)
 		return nil, err
 	}
-	b.Logger().Debug("certificate is:" + cert.Certificate)
-	b.Logger().Debug("chain is:" + cert.CertificateChain)
 
 	respData := map[string]interface{}{
 		"certificate_uid":   certUID,
@@ -65,7 +50,7 @@ func (b *backend) pathVenafiCertRead(ctx context.Context, req *logical.Request,
 		"certificate":       cert.Certificate,
 		"private_key":       cert.PrivateKey,
 	}
-	keyPassword := data.Get("key_password").(string)
+
 	if keyPassword != "" {
 		encryptedPrivateKeyPem, err := encryptPrivateKey(cert.PrivateKey, keyPassword)
 		if err != nil {