Merge pull request #101 from Venafi/fix-thread-lock-issue

Fix thread lock issue - removed mutex and added retry attribute

CHANGELOG.md

@@ -1,3 +1,6 @@
+# v.0.10.4 (May 27, 2022)
+Fixed a thread locking bug
+
 # v0.10.3 (May 12, 2022)
 Fixed a bug about storing private keys behavior and validation of certificate mismatch
 

go.mod

@@ -4,7 +4,7 @@ go 1.13
 
 require (
 	github.com/Venafi/vcert v3.18.4+incompatible
-	github.com/Venafi/vcert/v4 v4.19.0
+	github.com/Venafi/vcert/v4 v4.20.1
 	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.19.0 h1:/zIl9+s6uIjtI/LazPplrcSgThbwJkUx1XbyET3u8Iw=
-github.com/Venafi/vcert/v4 v4.19.0/go.mod h1:VcojF47VAzBnYHSRrb0SwOCmMpWJczajTuPiZNDJJSo=
+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/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/path_venafi_cert_enroll.go

@@ -59,6 +59,10 @@ func pathVenafiCertEnroll(b *backend) *framework.Path {
 				Description: `The requested Time To Live for the certificate; sets the expiration date.
 If not specified the role default is used. Cannot be larger than the role max TTL.`,
 			},
+			"retry": {
+				Type:        framework.TypeBool,
+				Description: "Used to specify to retry (once) issuance of certificate if any error occurred",
+			},
 		},
 		Operations: map[logical.Operation]framework.OperationHandler{
 			logical.UpdateOperation: &framework.PathOperation{
@@ -93,6 +97,10 @@ If not specified the role default is used. Cannot be larger than the role max TT
 				Type:        framework.TypeCommaStringSlice,
 				Description: "Use to specify custom fields in format 'key=value'. Use comma to separate multiple values: 'key1=value1,key2=value2'",
 			},
+			"retry": {
+				Type:        framework.TypeBool,
+				Description: "Used to specify to retry (once) issuance of certificate if any error occurred",
+			},
 		},
 		Operations: map[logical.Operation]framework.OperationHandler{
 			logical.UpdateOperation: &framework.PathOperation{
@@ -144,8 +152,6 @@ func (b *backend) pathVenafiSign(ctx context.Context, req *logical.Request, data
 
 func (b *backend) pathVenafiCertObtain(ctx context.Context, req *logical.Request, data *framework.FieldData, role *roleEntry, signCSR bool) (
 	*logical.Response, error) {
-	b.mux.Lock()
-	defer b.mux.Unlock()
 	// When utilizing performance standbys in Vault Enterprise, this forces the call to be redirected to the primary since
 	// a storage call is made after the API calls to issue the certificate.  This prevents the certificate from being
 	// issued twice in this scenario.
@@ -154,8 +160,6 @@ func (b *backend) pathVenafiCertObtain(ctx context.Context, req *logical.Request
 		return nil, logical.ErrReadOnly
 	}
 
-	keyPass := fmt.Sprintf("t%d-%s.tem.pwd", time.Now().Unix(), randRunes(4))
-
 	b.Logger().Debug("Getting the role\n")
 	roleName := data.Get("role").(string)
 
@@ -268,24 +272,26 @@ func (b *backend) pathVenafiCertObtain(ctx context.Context, req *logical.Request
 	}
 
 	b.Logger().Debug("Running enroll request")
-
-	requestID, err := cl.RequestCertificate(certReq)
-	if err != nil {
-		return logical.ErrorResponse(err.Error()), nil
-	}
-
-	pickupReq := &certificate.Request{
-		PickupID: requestID,
-		Timeout:  timeout,
+	format := ""
+	privateKeyFormat, ok := data.GetOk("private_key_format")
+	if ok {
+		if privateKeyFormat == LEGACY_PEM {
+			format = "legacy-pem"
+		}
 	}
 
-	if role.ServiceGenerated {
-		pickupReq.FetchPrivateKey = true
-		pickupReq.KeyPassword = keyPass
-	}
+	keyPass := fmt.Sprintf("t%d-%s.tem.pwd", time.Now().Unix(), randRunes(4))
+	retry, ok := data.GetOk("retry")
 
-	pcc, err := cl.RetrieveCertificate(pickupReq)
+	var pcc *certificate.PEMCollection
+	pcc, err = issueCertificate(certReq, keyPass, cl, timeout, role, format, privateKeyFormat, signCSR)
 	if err != nil {
+		if retry == true {
+			pcc, err = issueCertificate(certReq, keyPass, cl, timeout, role, format, privateKeyFormat, signCSR)
+			if err != nil {
+				return logical.ErrorResponse(err.Error()), nil
+			}
+		}
 		return logical.ErrorResponse(err.Error()), nil
 	}
 
@@ -303,51 +309,6 @@ func (b *backend) pathVenafiCertObtain(ctx context.Context, req *logical.Request
 	chain := strings.Join(append([]string{pcc.Certificate}, pcc.Chain...), "\n")
 	b.Logger().Debug("cert Chain: " + strings.Join(pcc.Chain, ", "))
 
-	format := ""
-	privateKeyFormat, ok := data.GetOk("private_key_format")
-	if ok {
-		if privateKeyFormat == LEGACY_PEM {
-			format = "legacy-pem"
-		}
-	}
-
-	// Local generated
-	if !signCSR && !role.ServiceGenerated {
-		privateKeyPemBytes, err := certificate.GetPrivateKeyPEMBock(certReq.PrivateKey, format)
-		if err != nil {
-			return nil, err
-		}
-		privateKeyPem := string(pem.EncodeToMemory(privateKeyPemBytes))
-		pcc.PrivateKey = privateKeyPem
-	} else if role.ServiceGenerated {
-		// Service generated
-		privateKey, err := DecryptPkcs8PrivateKey(pcc.PrivateKey, keyPass)
-		if err != nil {
-			return nil, err
-		}
-		block, _ := pem.Decode([]byte(privateKey))
-		if privateKeyFormat == LEGACY_PEM {
-			encrypted, err := util.X509EncryptPEMBlock(rand.Reader, "RSA PRIVATE KEY", block.Bytes, []byte(keyPass), util.PEMCipherAES256)
-			if err != nil {
-				return nil, err
-			}
-			encryptedPem := pem.EncodeToMemory(encrypted)
-			privateKeyBytes, err := getPrivateKey(encryptedPem, keyPass)
-			if err != nil {
-				return nil, err
-			}
-			privateKey = string(privateKeyBytes)
-		}
-		pcc.PrivateKey = privateKey
-	} else {
-		b.Logger().Debug("CSR is being provided, not processing private key")
-	}
-
-	_, err = tls.X509KeyPair([]byte(pcc.Certificate), []byte(pcc.PrivateKey))
-	if err != nil {
-		fmt.Errorf("the certificate returned by Venafi did not contain the requested private key," +
-			" key pair has been discarded")
-	}
 	if role.StorePrivateKey && !signCSR {
 		entry, err = logical.StorageEntryJSON("", VenafiCert{
 			Certificate:      pcc.Certificate,
@@ -455,6 +416,68 @@ type requestData struct {
 	ttl          time.Duration
 }
 
+func issueCertificate(certReq *certificate.Request, keyPass string, cl endpoint.Connector, timeout time.Duration, role *roleEntry, format string, privateKeyFormat interface{}, signCSR bool) (pcc *certificate.PEMCollection, err error) {
+	requestID, err := cl.RequestCertificate(certReq)
+	if err != nil {
+		return nil, err
+	}
+
+	pickupReq := &certificate.Request{
+		PickupID: requestID,
+		Timeout:  timeout,
+	}
+
+	if role.ServiceGenerated {
+		pickupReq.FetchPrivateKey = true
+		pickupReq.KeyPassword = keyPass
+	}
+
+	var pemCollection *certificate.PEMCollection
+	pemCollection, err = cl.RetrieveCertificate(pickupReq)
+	if err != nil {
+		return nil, err
+	}
+
+	// Local generated
+	if !signCSR && !role.ServiceGenerated {
+		privateKeyPemBytes, err := certificate.GetPrivateKeyPEMBock(certReq.PrivateKey, format)
+		if err != nil {
+			return nil, err
+		}
+		privateKeyPem := string(pem.EncodeToMemory(privateKeyPemBytes))
+		pemCollection.PrivateKey = privateKeyPem
+	} else if role.ServiceGenerated {
+		// Service generated
+		privateKey, err := DecryptPkcs8PrivateKey(pemCollection.PrivateKey, keyPass)
+		if err != nil {
+			return nil, err
+		}
+		block, _ := pem.Decode([]byte(privateKey))
+		if privateKeyFormat == LEGACY_PEM {
+			encrypted, err := util.X509EncryptPEMBlock(rand.Reader, "RSA PRIVATE KEY", block.Bytes, []byte(keyPass), util.PEMCipherAES256)
+			if err != nil {
+				return nil, err
+			}
+			encryptedPem := pem.EncodeToMemory(encrypted)
+			privateKeyBytes, err := getPrivateKey(encryptedPem, keyPass)
+			if err != nil {
+				return nil, err
+			}
+			privateKey = string(privateKeyBytes)
+		}
+		pemCollection.PrivateKey = privateKey
+	}
+
+	if !signCSR {
+		_, err = tls.X509KeyPair([]byte(pemCollection.Certificate), []byte(pemCollection.PrivateKey))
+		if err != nil {
+			return nil, fmt.Errorf("the certificate returned by Venafi did not contain the requested private key," +
+				" key pair has been discarded")
+		}
+	}
+	return pemCollection, nil
+}
+
 func formRequest(reqData requestData, role *roleEntry, signCSR bool, logger hclog.Logger) (certReq *certificate.Request, err error) {
 	if !signCSR {
 		if len(reqData.commonName) == 0 && len(reqData.altNames) == 0 {