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

feat: support ssl + client ssl authentication #69

Merged
merged 3 commits into from
Sep 30, 2021

Conversation

shivanshs9
Copy link
Contributor

Implements Authentication and SSL support in Chaosd (as per chaos-mesh/rfcs#14)

For implementation, I referred this guide

@shivanshs9 shivanshs9 force-pushed the ft-auth-certificates branch from a0adf06 to 6b58b4f Compare May 24, 2021 17:38

Verified

This commit was signed with the committer’s verified signature.
joshmgross Josh Gross
Signed-off-by: Shivansh Saini <shivanshs9@gmail.com>
@shivanshs9 shivanshs9 force-pushed the ft-auth-certificates branch from 6b58b4f to f444fc1 Compare May 24, 2021 18:22
@WangXiangUSTC
Copy link
Collaborator

@shivanshs9 Thanks, could you please add an integration test for it, to make sure the SSL authentication can work

@shivanshs9
Copy link
Contributor Author

@WangXiangUSTC yep, I'll add one soon. Btw, I already tested it manually. If you'd like to give it a go too, you can generate certificates with this script.
Then I ran the server with: ./bin/chaosd server --cert ~/packages/nodejs-certificate-auth/server/server_cert.pem --key ~/packages/nodejs-certificate-auth/server/server_key.pem --CA ~/packages/nodejs-certificate-auth/server/server_cert.pem

And then I did bunch of requests:

  • HTTPS with no client certificate (got 401)
  • HTTPS with self-signed client certificate (got 403)
  • HTTPS with authorized client certificate (got 200)

Verified

This commit was signed with the committer’s verified signature.
joshmgross Josh Gross
- ignore output of integration tests

Signed-off-by: Shivansh Saini <shivanshs9@gmail.com>

Verified

This commit was signed with the committer’s verified signature.
joshmgross Josh Gross
Signed-off-by: Shivansh Saini <shivanshs9@gmail.com>
@shivanshs9 shivanshs9 force-pushed the ft-auth-certificates branch from 6aa3131 to e88298f Compare May 26, 2021 08:25
Copy link
Collaborator

@WangXiangUSTC WangXiangUSTC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -32,6 +32,9 @@ func NewServerCommand() *cobra.Command {

cmd.Flags().IntVarP(&conf.ListenPort, "port", "p", 31767, "listen port of the Chaosd Server")
cmd.Flags().StringVarP(&conf.ListenHost, "host", "a", "0.0.0.0", "listen host of the Chaosd Server")
cmd.Flags().StringVar(&conf.SSLCertFile, "cert", "", "path to a PEM encoded certificate file")
cmd.Flags().StringVar(&conf.SSLKeyFile, "key", "", "path to a PEM encoded private key file")
cmd.Flags().StringVar(&conf.SSLClientCAFile, "CA", "", "path to a PEM encoded CA's certificate file")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to use lowercase ca instead.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can update in another pr

Copy link
Member

@Andrewmatilde Andrewmatilde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@WangXiangUSTC WangXiangUSTC merged commit d795cf6 into chaos-mesh:main Sep 30, 2021
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

4 participants