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

Added control and rule for Kubernetes Aggregated API Server Redirection Vulnerability [CVE 2022-3172] #304

Closed
wants to merge 5 commits into from

Conversation

0xquark
Copy link

@0xquark 0xquark commented Feb 9, 2023

Summary

This PR adds control and rule for Kubernetes Aggregated API Server Redirection Vulnerability [CVE 2022-3172].

kubernetes/kubernetes#112513

0xquark and others added 2 commits February 10, 2023 00:44
…on Vulnerability [CVE 2022-3172]

Signed-off-by: Karanjot Singh <drquark@duck.com>
@slashben
Copy link
Contributor

Hey @0xquark , I think it is important to create test for every control.

We have a small test framework, @Daniel-GrunbergerCA , can you help us?

Signed-off-by: Karanjot Singh <drquark@duck.com>
@0xquark
Copy link
Author

0xquark commented Feb 13, 2023

@slashben Thank you for the review. I've added the test but i am unable to setup the test environment locally and test the changes. The kubescape documentation for testrunner redirects to same page while clicking on setting the environment
https://github.com/kubescape/regolibrary/blob/master/testrunner/README.md#environment-setup
Is there any guide to setup the env? I am not able to install the package testrunner/opaprocessor

@alegrey91
Copy link
Contributor

alegrey91 commented Feb 17, 2023

Hi @0xquark, and thanks for your contribution. This is really appreciated.
I checked the link you provided and you're right, the link is broken.
Actually you don't need to setup the environment anymore, since the git2go dependency has been removed.
So, you can test your rule easily just by running this command from the testrunner directory: go test -v -tags=static rego_test.go -run TestAllRules.
:)

@0xquark
Copy link
Author

0xquark commented Feb 17, 2023

@alegrey91 thanks! How can i install testrunner/opaprocessor package?

@alegrey91
Copy link
Contributor

@0xquark actually you should not need to install it.
Are you encountering some error message?

@0xquark
Copy link
Author

0xquark commented Feb 17, 2023

rego_test.go:7:2: cannot find package "testrunner/opaprocessor" in any of: /opt/homebrew/Cellar/go/1.19.5/libexec/src/testrunner/opaprocessor (from $GOROOT)
The opaprocessor is already present in the folder, i think the problem should be with the GOPATH
Edit : Solved it! but other dependencies are causing similar issues!

@0xquark actually you should not need to install it. Are you encountering some error message?

@alegrey91
Copy link
Contributor

alegrey91 commented Feb 17, 2023

rego_test.go:7:2: cannot find package "testrunner/opaprocessor" in any of: /opt/homebrew/Cellar/go/1.19.5/libexec/src/testrunner/opaprocessor (from $GOROOT) The opaprocessor is already present in the folder, i think the problem should be with the GOPATH Edit : Solved it! but other dependencies are causing similar issues!

Great! Can you please share how did you solve the problem?
Which are the other dependencies that are causing the problem?
Can you share the error message?

@0xquark
Copy link
Author

0xquark commented Feb 17, 2023

rego_test.go:7:2: cannot find package "testrunner/opaprocessor" in any of: /opt/homebrew/Cellar/go/1.19.5/libexec/src/testrunner/opaprocessor (from $GOROOT) The opaprocessor is already present in the folder, i think the problem should be with the GOPATH Edit : Solved it! but other dependencies are causing similar issues!

Great! Can you please share how did you solve the problem? Which are the other dependencies that are causing the problem? Can you share the error message?
I added the opaprocessor to my GOPATH and it solved it.
Now i am having these # command-line-arguments

package command-line-arguments (test)
	imports testrunner/opaprocessor
	imports github.com/kubescape/k8s-interface/workloadinterface
	imports github.com/armosec/armoapi-go/apis
	imports github.com/coreos/go-oidc: no Go files in /Users/karanjotsingh/go/src/github.com/coreos/go-oidc

The last one is maybe because go-oidc recently changed their import path ( https://github.com/coreos/go-oidc )

@alegrey91
Copy link
Contributor

Did you run go mod tidy before the test command?

@0xquark
Copy link
Author

0xquark commented Feb 17, 2023

ah! My bad! I forgot to run it that's why the missing packages
Thanks!

@alegrey91
Copy link
Contributor

You're welcome ;)

Signed-off-by: Karanjot Singh <drquark@duck.com>

- Imported future.keywords
- used .yaml input instead of .json ( doesn't make much difference )
- fixed vulnerable_version format
@0xquark
Copy link
Author

0xquark commented Feb 17, 2023

I tried running tests for the policy and it is resulting in an unmarshaling error

in parseRegoResult, 
json.Unmarshal failed. name: vulnerable_version, obj: [1.21.14 1.22.0-1.22.13 1.23.0-1.23.10 1.24.0-1.24.4 1.25.0], 
reason: json: cannot unmarshal string into Go value of type reporthandling.RuleResponse in rule: test-single-rego/input

I tried fixing this by changing the structure of vulnerable_version to be a list instead of object but it turns out to be no good. From my understanding, it indicates that json.Unmarshal cannot unmarshal the variable's value into a reporthandling.RuleResponse type. I think vulnerable_version is being interpreted as a reporthandling.RuleResponse instead of a set of strings as intended.
@alegrey91 Could you help me in here ?

Edit : Fixed using ver[_] = api_name expression assigns each version string to the variable api_name. This is a way to generate a list of Kubernetes API server names that correspond to the vulnerable versions.

Signed-off-by: Karanjot Singh <drquark@duck.com>
@0xquark
Copy link
Author

0xquark commented Feb 17, 2023

Whenever i run tests using TestSingleRego, it generates an empty struct []. This happens for all the policies i enter in testdir

@0xquark
Copy link
Author

0xquark commented Feb 18, 2023

This PR is ready for review 🚀

@dwertent
Copy link
Contributor

@alegrey91 Ping :)

@alegrey91
Copy link
Contributor

This PR is ready for review 🚀

Sorry for the late reply. We need few days to make some checks and then we can finally take a look over it.

@0xquark
Copy link
Author

0xquark commented Feb 20, 2023

This PR is ready for review 🚀

Sorry for the late reply. We need few days to make some checks and then we can finally take a look over it.

Great! Let me know if you run into any issues :)

@slashben
Copy link
Contributor

@Daniel-GrunbergerCA , what is the difference between this control and C-0089?

@0xquark
Copy link
Author

0xquark commented Feb 21, 2023

Hey @slashben, i believe this control is already implemented and i might've missed it while while looking for any previously implemented controls before implementing this. However this control also includes a rule which checks for the vulnerable versions of api servers and if it is vulnerable then it prompts the user with deny message.
I will be moving on to implementing new controls or rules. Also working on this control helped me get a better understanding of rego.

@Daniel-GrunbergerCA
Copy link
Contributor

Daniel-GrunbergerCA commented Feb 26, 2023

Hey @0xquark
First of all, sorry for the delay to respond, it's been a few busy days :)
This control is indeed implemented already, but we really appreciate the fact that you are looking into contribuiting to our project! Maybe you want to share with us the controls that you think should be implemented for next PRs?
Also I have a question about your PR: are you sure that the check ver[_] = api_name works for the ver list that you defined? For example, would it work for "1.22.0-1.22.13"?

@0xquark
Copy link
Author

0xquark commented Feb 27, 2023

Hi there! @Daniel-GrunbergerCA No worries! Thank you for letting me know about the control already being implemented. If I come across any other controls that I believe would be beneficial, I will definitely let you know.
About the check, I apologise i might've missed it while testing. You are right! It won't work as it is range of versions. I'll modify the rule and add all the version in the list.Thank you for bringing it to my attention.

@0xquark 0xquark closed this by deleting the head repository Mar 7, 2023
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

Successfully merging this pull request may close these issues.

None yet

5 participants