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

Add cert authorization with common name support. #241

Merged
merged 18 commits into from
Jun 13, 2024

Conversation

liuh-80
Copy link
Contributor

@liuh-80 liuh-80 commented May 28, 2024

Add cert authorization with common name support.

Why I did it

Support cert authorization with common name.

How I did it

Load trusted cert common name from config DB and check cert common name.

How to verify it

Manually test.
Add new UT.

Work item tracking

Microsoft ADO (number only): 25226269

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111

Description for the changelog

Add cert authorization with common name support.

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@liuh-80 liuh-80 requested review from ganglyu and qiluo-msft June 5, 2024 02:44
@liuh-80 liuh-80 marked this pull request as ready for review June 5, 2024 02:45
@liuh-80
Copy link
Contributor Author

liuh-80 commented Jun 5, 2024

The yang model and service start script change in this PR: sonic-net/sonic-buildimage#18709
Will create test case PR after this PR merged.

@@ -22,6 +22,7 @@ var (
serverKey = flag.String("server_key", "", "TLS server private key")
insecure = flag.Bool("insecure", false, "Skip providing TLS cert and key, for testing only!")
allowNoClientCert = flag.Bool("allow_no_client_auth", false, "When set, telemetry server will request but not require a client certificate.")
clientCrtCname = flag.String("client_crt_cname", "", "Client cert common name")
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need cname for dialout server?

Copy link
Contributor Author

@liuh-80 liuh-80 Jun 5, 2024

Choose a reason for hiding this comment

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

Reverted, confirmed with Zain, dialout not use in prod.

t.Errorf("CommonNameMatch with empty config table should success: %v", err)
}

cancel()
Copy link
Contributor

Choose a reason for hiding this comment

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

Not aligned.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed this and similar case

@liuh-80 liuh-80 merged commit f0d0959 into sonic-net:master Jun 13, 2024
5 checks passed
@liuh-80
Copy link
Contributor Author

liuh-80 commented Nov 7, 2024

PR for 202311: #323
PR for 202405 #322

@bingwang-ms
Copy link

Cherry-pick to 202405 is done by PR #322

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

Successfully merging this pull request may close these issues.

4 participants