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

Implement Concurrency and Timeout in Policy Verification Process #57

Open
naveensrinivasan opened this issue Oct 23, 2023 · 0 comments
Open

Comments

@naveensrinivasan
Copy link
Contributor

https://github.com/testifysec/go-witness/blob/5e567f0823cff91d879d7f4f0691b648af1d27a1/policy/policy.go#L201-L223

Description:

In the current implementation of the Verify function in policy.go, the Search function is called synchronously for each step in the policy. This could potentially block for a significant amount of time, especially if there are many attestations to search through because it will be i/o call.

Proposed Solution:

To improve the performance and responsiveness of the policy verification process, we propose to make the Search function call non-blocking by using Go's built-in concurrency with goroutines and channels. Additionally, we suggest adding a timeout for each search to prevent the verification process from hanging indefinitely in case of slow or unresponsive searches.

Here's a rough example of how the code could be modified:

var wg sync.WaitGroup
results := make(chan PolicyResult)
errors := make(chan error)

for stepName, step := range p.Steps {
	wg.Add(1)
	go func(stepName string, step Step) {
		defer wg.Done()
		ctxWithTimeout, cancel := context.WithTimeout(ctx, 5*time.Second)
		defer cancel()

		statements, err := vo.verifiedSource.Search(ctxWithTimeout, stepName, vo.subjectDigests, attestationsByStep[stepName])
		if err != nil {
			errors <- err
			return
		}

		approvedCollections := step.checkFunctionaries(statements, trustBundles)
		stepResults := step.validateAttestations(approvedCollections)
		results <- PolicyResult{EvidenceByStep: map[string][]source.VerifiedCollection{stepName: stepResults.Passed}}
	}(stepName, step)
}

Benefits:

  1. Performance: By making the Search function call non-blocking, the policy verification process can continue with other tasks while the search is being performed, potentially improving the overall performance.

  2. Responsiveness: Adding a timeout for each search ensures that the verification process doesn't hang indefinitely in case of slow or unresponsive searches, improving the system's responsiveness.

  3. Resource Usage: By limiting the time spent on each search, we can also control the resource usage of the policy verification process, which can be particularly beneficial in resource-constrained environments.

  4. Scalability: With the proposed changes, we could potentially increase the searchDepth in the policy verification process. This would allow us to perform a more thorough search over many iterations, potentially uncovering more attestations that satisfy the policy. This can be particularly beneficial for complex policies with many steps or dependencies between steps. By implementing concurrency and timeouts, we can scale the depth of our search without significantly impacting performance or responsiveness.

I wanted to try this code change before proposing it but didn't have the infra or the tests to run it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant