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

Enables timeout for enroll operations for TLSPC and TLSPDC in VCert SDK #428

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion pkg/venafi/cloud/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,13 @@ func (c *Connector) RequestCertificate(req *certificate.Request) (requestID stri
return "", err
}

// We set timeout for the http client
if req.Timeout != 0 {
if c.client == nil {
c.client = &http.Client{}
}
c.client.Timeout = req.Timeout
}
statusCode, status, body, err := c.request("POST", url, cloudReq)

if err != nil {
Expand Down Expand Up @@ -895,7 +902,13 @@ func (c *Connector) SupportSynchronousRequestCertificate() bool {

// RetrieveCertificate retrieves the certificate for the specified ID
func (c *Connector) RetrieveCertificate(req *certificate.Request) (certificates *certificate.PEMCollection, err error) {

// We set timeout for the http client
if req.Timeout != 0 {
if c.client == nil {
c.client = &http.Client{}
}
c.client.Timeout = req.Timeout
}
if req.PickupID == "" && req.CertID == "" && req.Thumbprint != "" {
// search cert by Thumbprint and fill pickupID
var certificateRequestId string
Expand Down Expand Up @@ -1184,6 +1197,7 @@ func (c *Connector) waitForCertificate(url string, request *certificate.Request)
if statusCode == http.StatusOK {
return
}

if request.Timeout == 0 {
err = endpoint.ErrCertificatePending{CertificateID: request.PickupID, Status: status}
return
Expand Down
95 changes: 95 additions & 0 deletions pkg/venafi/cloud/connector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,33 @@ func TestRequestCertificate(t *testing.T) {
}
}

func TestRequestCertificateWithExtendTime(t *testing.T) {
t.Skip("Skipping as we cannot make VaaS to hold the amount of time we want to properly test this")
conn := getTestConnector(ctx.CloudZone)
conn.verbose = true
err := conn.Authenticate(&endpoint.Authentication{APIKey: ctx.CloudAPIkey})
if err != nil {
t.Fatalf("%s", err)
}
zoneConfig, err := conn.ReadZoneConfiguration()
if err != nil {
t.Fatalf("%s", err)
}
req := certificate.Request{}
req.Subject.CommonName = test.RandCN()
req.Subject.Organization = []string{"Venafi, Inc."}
req.Subject.OrganizationalUnit = []string{"Automated Tests"}
req.Timeout, _ = time.ParseDuration("45s")
err = conn.GenerateRequest(zoneConfig, &req)
if err != nil {
t.Fatalf("%s", err)
}
_, err = conn.RequestCertificate(&req)
if err != nil {
t.Fatalf("%s", err)
}
}

func TestRequestCertificateED25519WithValidation(t *testing.T) {
conn := getTestConnector(ctx.VAASzoneEC)
conn.verbose = true
Expand Down Expand Up @@ -469,6 +496,74 @@ func TestRetrieveCertificate(t *testing.T) {
}
}

func TestRetrieveCertificateWithExtendedTime(t *testing.T) {
t.Skip("Skipping as we cannot make VaaS to hold the amount of time we want to properly test this")
conn := getTestConnector(ctx.CloudZone)
err := conn.Authenticate(&endpoint.Authentication{APIKey: ctx.CloudAPIkey})
if err != nil {
t.Fatalf("%s", err)
}
zoneConfig, err := conn.ReadZoneConfiguration()
if err != nil {
t.Fatalf("%s", err)
}
req := &certificate.Request{}
req.Subject.CommonName = test.RandCN()
req.Subject.Organization = []string{"Venafi, Inc."}
req.Subject.OrganizationalUnit = []string{"Automated Tests"}
req.Timeout, _ = time.ParseDuration("45s")
err = conn.GenerateRequest(zoneConfig, req)
if err != nil {
t.Fatalf("%s", err)
}
pickupID, err := conn.RequestCertificate(req)
if err != nil {
t.Fatalf("%s", err)
}
req.PickupID = pickupID
req.ChainOption = certificate.ChainOptionRootLast

pcc, _ := certificate.NewPEMCollection(nil, nil, nil)
startTime := time.Now()
for {

pcc, err = conn.RetrieveCertificate(req)
if err != nil {
_, ok := err.(endpoint.ErrCertificatePending)
if ok {
if time.Now().After(startTime.Add(time.Duration(600) * time.Second)) {
err = endpoint.ErrRetrieveCertificateTimeout{CertificateID: pickupID}
break
}
time.Sleep(time.Duration(10) * time.Second)
continue
}
break
}
break
}
if err != nil {
t.Fatalf("%s", err)
}
p, _ := pem.Decode([]byte(pcc.Certificate))
cert, err := x509.ParseCertificate(p.Bytes)
if err != nil {
t.Fatalf("%s", err)
}
if req.Subject.CommonName != cert.Subject.CommonName {
t.Fatalf("Retrieved certificate did not contain expected CN. Expected: %s -- Actual: %s", req.Subject.CommonName, cert.Subject.CommonName)
}

p, _ = pem.Decode([]byte(pcc.Chain[0]))
cert, err = x509.ParseCertificate(p.Bytes)
if err != nil {
t.Fatalf("%s", err)
}
if !cert.IsCA || fmt.Sprintf("%v", cert.Subject) == fmt.Sprintf("%v", cert.Issuer) {
t.Fatalf("Expected Intermediate Root Certificate first, instead got Subject: %v -- Issuer %v", cert.Subject, cert.Issuer)
}
}

func TestRetrieveCertificateRootFirst(t *testing.T) {
conn := getTestConnector(ctx.CloudZone)
err := conn.Authenticate(&endpoint.Authentication{APIKey: ctx.CloudAPIkey})
Expand Down
32 changes: 32 additions & 0 deletions pkg/venafi/tpp/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,14 @@ func (c *Connector) prepareRequest(req *certificate.Request, zone string) (tppRe
// - true: Clear the Disabled attribute, reenable, and then renew the certificate (in this request). Reuse the same CertificateDN, that is also known as a Certificate object.
tppReq.Reenable = true

// We set timeout at 2 levels:
// - for the platform request, in this case TPP
// - for the Go HTTP client
// In the following we set the timeout for TPP
if req.Timeout != 0 {
tppReq.WorkToDoTimeout = strconv.FormatFloat(req.Timeout.Seconds(), 'f', 0, 64)
}

return tppReq, err
}

Expand Down Expand Up @@ -715,6 +723,17 @@ func (c *Connector) RequestCertificate(req *certificate.Request) (requestID stri
return "", err
}

// We set timeout at 2 levels:
// - for the platform request, in this case TPP
// - for the Go HTTP client
// In the following we will add for the http client
if req.Timeout != 0 {
if c.client == nil {
c.client = &http.Client{}
}
c.client.Timeout = req.Timeout
}

statusCode, status, body, err := c.request("POST", urlResourceCertificateRequest, tppCertificateRequest)
if err != nil {
return "", err
Expand Down Expand Up @@ -1328,6 +1347,18 @@ func (c *Connector) RetrieveCertificate(req *certificate.Request) (certificates
includeChain := req.ChainOption != certificate.ChainOptionIgnore
rootFirstOrder := includeChain && req.ChainOption == certificate.ChainOptionRootFirst

// We set timeout at 2 levels:
// - for the platform request, in this case TPP
// - for the Go HTTP client
// In the following we will add for the http client

if req.Timeout != 0 {
if c.client == nil {
c.client = &http.Client{}
}
c.client.Timeout = req.Timeout
}

if req.PickupID == "" && req.Thumbprint != "" {
// search cert by Thumbprint and fill pickupID
searchResult, err := c.searchCertificatesByFingerprint(req.Thumbprint)
Expand Down Expand Up @@ -1360,6 +1391,7 @@ func (c *Connector) RetrieveCertificate(req *certificate.Request) (certificates
startTime := time.Now()
for {
var retrieveResponse *certificateRetrieveResponse

retrieveResponse, err = c.retrieveCertificateOnce(certReq)
if err != nil {
return nil, fmt.Errorf("unable to retrieve: %s", err)
Expand Down
39 changes: 25 additions & 14 deletions pkg/venafi/tpp/connector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,9 @@ func TestNewConnectorURLErrors(t *testing.T) {
"trailing_other": "https://example.com/foo/",
"nested_vedsdk": "https://example.com/foo/vedsdk",
}
for label, url := range tests {
for label, testUrl := range tests {
t.Run(label, func(t *testing.T) {
c, err := NewConnector(url, "", false, nil)
c, err := NewConnector(testUrl, "", false, nil)
if err == nil {
t.Error("expected an error")
}
Expand Down Expand Up @@ -500,7 +500,7 @@ func TestRequestCertificateUserPassword(t *testing.T) {
t.Fatalf("err is not nil, err: %s", err)
}
}
DoRequestCertificate(t, tpp)
DoRequestCertificate(t, tpp, 0)
}

func TestRequestCertificateToken(t *testing.T) {
Expand All @@ -515,7 +515,24 @@ func TestRequestCertificateToken(t *testing.T) {
t.Fatalf("err is not nil, err: %s", err)
}
}
DoRequestCertificate(t, tpp)
DoRequestCertificate(t, tpp, 0)
}

func TestRequestCertificateTokenWithExtendedTimeout(t *testing.T) {
t.Skip("Skipping as we cannot make TPP to hold the amount of time we want to properly test this")
tpp, err := getTestConnector(ctx.TPPurl, ctx.TPPZone)
if err != nil {
t.Fatalf("err is not nil, err: %s url: %s", err, expectedURL)
}

if tpp.apiKey == "" {
err = tpp.Authenticate(&endpoint.Authentication{AccessToken: ctx.TPPaccessToken})
if err != nil {
t.Fatalf("err is not nil, err: %s", err)
}
}
timeout, _ := time.ParseDuration("45s")
DoRequestCertificate(t, tpp, timeout)
}

func TestRequestCertificateWithValidityHours(t *testing.T) {
Expand Down Expand Up @@ -884,15 +901,6 @@ func TestRetrieveCertificate(t *testing.T) {
givenTimeout: 3 * time.Second,
expectErr: "unable to retrieve: Unexpected status code on TPP Certificate Retrieval. Status: 500 Certificate \\VED\\Policy\\TLS/SSL\\aexample.com has encountered an error while processing, Status: Post CSR failed with error: Cannot connect to the certificate authority (CA), Stage: 500.",
},
{
name: "should fail when timeout too small while waiting for the cert",
mockRetrieve: []mockResp{
{`202 Certificate \VED\Policy\TLS/SSL\aexample.com being processed, Status: Post CSR, Stage: 500.`,
`{"Stage": 500, "Status": "Post CSR"}`},
},
givenTimeout: 1 * time.Millisecond,
expectErr: "Operation timed out. You may try retrieving the certificate later using Pickup ID: \\VED\\Policy\\Test\\bexample.com",
},
}

serverWith := func(mockRetrieve []mockResp) (_ *httptest.Server, retrieveCount *int32) {
Expand Down Expand Up @@ -1106,7 +1114,7 @@ func DoRequestCertificateWithValidityDuration(t *testing.T, tpp *Connector) {

}

func DoRequestCertificate(t *testing.T, tpp *Connector) {
func DoRequestCertificate(t *testing.T, tpp *Connector, timeout time.Duration) {
config, err := tpp.ReadZoneConfiguration()
if err != nil {
t.Fatalf("err is not nil, err: %s", err)
Expand All @@ -1126,6 +1134,9 @@ func DoRequestCertificate(t *testing.T, tpp *Connector) {
req.CustomFields = []certificate.CustomField{
{Name: "custom", Value: "2019-10-10"},
}
if timeout != 0 {
req.Timeout = timeout
}
err = tpp.GenerateRequest(config, req)
if err != nil {
t.Fatalf("err is not nil, err: %s", err)
Expand Down
1 change: 1 addition & 0 deletions pkg/venafi/tpp/tpp.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ type certificateRequest struct {
Devices []device `json:",omitempty"`
CertificateType string `json:",omitempty"`
Reenable bool `json:",omitempty"`
WorkToDoTimeout string `json:",omitempty"`
}

type certificateRetrieveRequest struct {
Expand Down