-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Use TLS for certificate-free gNMI services #28915
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
base: master
Are you sure you want to change the base?
Changes from all commits
ad7a54b
12b20b6
74b73fa
334da09
d4c7053
d644062
c02431b
7f6fcc0
335786e
590c2e4
83c56a5
3fbc4c9
9b0818a
15378fc
4702de5
bda5e7c
07c9928
59c803a
5d34e7c
4c9ed38
886b49b
67c7b89
1060b68
bd2ad72
bd9a278
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,7 +27,10 @@ TELEMETRY_VARS_FILE=/usr/share/sonic/templates/telemetry_vars.j2 | |
| ESCAPE_QUOTE="'\''" | ||
|
|
||
| extract_field() { | ||
| echo $(echo $1 | jq -r $2) | ||
| if [ -z "$1" ]; then | ||
| return | ||
| fi | ||
| jq -r "$2" <<< "$1" | ||
| } | ||
|
|
||
| if [ ! -f "$TELEMETRY_VARS_FILE" ]; then | ||
|
|
@@ -39,45 +42,58 @@ fi | |
| # Use default value if no valid config exists | ||
| TELEMETRY_VARS=$(sonic-cfggen -d -t $TELEMETRY_VARS_FILE) | ||
| TELEMETRY_VARS=${TELEMETRY_VARS//[\']/\"} | ||
| X509=$(echo $TELEMETRY_VARS | jq -r '.x509') | ||
| GNMI=$(echo $TELEMETRY_VARS | jq -r '.gnmi') | ||
| CERTS=$(echo $TELEMETRY_VARS | jq -r '.certs') | ||
| X509=$(jq -r '.x509 // empty' <<< "$TELEMETRY_VARS") | ||
| GNMI=$(jq -r '.gnmi // empty' <<< "$TELEMETRY_VARS") | ||
| CERTS=$(jq -r '.certs // empty' <<< "$TELEMETRY_VARS") | ||
|
|
||
| export GRPC_GO_LOG_VERBOSITY_LEVEL=99 | ||
| export GRPC_GO_LOG_SEVERITY_LEVEL=info | ||
|
|
||
| TELEMETRY_ARGS=" -logtostderr" | ||
| USE_EPHEMERAL_TLS=false | ||
| CERTIFICATE_FREE_TLS=false | ||
| HAS_CLIENT_CA=false | ||
| export CVL_SCHEMA_PATH=/usr/sbin/schema | ||
| export GOTRACEBACK=crash | ||
|
|
||
| if [ -n "$CERTS" ]; then | ||
| SERVER_CRT=$(extract_field "$CERTS" '.server_crt') | ||
| SERVER_KEY=$(extract_field "$CERTS" '.server_key') | ||
| if [ -z $SERVER_CRT ] || [ -z $SERVER_KEY ]; then | ||
| SERVER_CRT=$(extract_field "$CERTS" '.server_crt // empty') | ||
| SERVER_KEY=$(extract_field "$CERTS" '.server_key // empty') | ||
| if [ -z "$SERVER_CRT" ] || [ -z "$SERVER_KEY" ]; then | ||
| TELEMETRY_ARGS+=" --insecure" | ||
| USE_EPHEMERAL_TLS=true | ||
| else | ||
| TELEMETRY_ARGS+=" --server_crt $SERVER_CRT --server_key $SERVER_KEY " | ||
| fi | ||
|
|
||
| CA_CRT=$(extract_field "$CERTS" '.ca_crt') | ||
| if [ ! -z $CA_CRT ]; then | ||
| CA_CRT=$(extract_field "$CERTS" '.ca_crt // empty') | ||
| if [ -n "$CA_CRT" ]; then | ||
| TELEMETRY_ARGS+=" --ca_crt $CA_CRT" | ||
| HAS_CLIENT_CA=true | ||
| elif [ "$USE_EPHEMERAL_TLS" == "true" ]; then | ||
| CERTIFICATE_FREE_TLS=true | ||
| fi | ||
| elif [ -n "$X509" ]; then | ||
| SERVER_CRT=$(extract_field "$X509" '.server_crt') | ||
| SERVER_KEY=$(extract_field "$X509" '.server_key') | ||
| if [ -z $SERVER_CRT ] || [ -z $SERVER_KEY ]; then | ||
| SERVER_CRT=$(extract_field "$X509" '.server_crt // empty') | ||
| SERVER_KEY=$(extract_field "$X509" '.server_key // empty') | ||
| if [ -z "$SERVER_CRT" ] || [ -z "$SERVER_KEY" ]; then | ||
| TELEMETRY_ARGS+=" --insecure" | ||
| USE_EPHEMERAL_TLS=true | ||
| else | ||
| TELEMETRY_ARGS+=" --server_crt $SERVER_CRT --server_key $SERVER_KEY " | ||
| fi | ||
|
|
||
| CA_CRT=$(extract_field "$X509" '.ca_crt') | ||
| if [ ! -z $CA_CRT ]; then | ||
| CA_CRT=$(extract_field "$X509" '.ca_crt // empty') | ||
| if [ -n "$CA_CRT" ]; then | ||
| TELEMETRY_ARGS+=" --ca_crt $CA_CRT" | ||
| HAS_CLIENT_CA=true | ||
| elif [ "$USE_EPHEMERAL_TLS" == "true" ]; then | ||
| CERTIFICATE_FREE_TLS=true | ||
| fi | ||
| else | ||
| TELEMETRY_ARGS+=" --noTLS --bind_address 127.0.0.1" | ||
| TELEMETRY_ARGS+=" --insecure" | ||
| USE_EPHEMERAL_TLS=true | ||
| CERTIFICATE_FREE_TLS=true | ||
| fi | ||
|
Comment on lines
93
to
97
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Current PR diff includes all three gitlinks: sonic-gnmi 835b6b4, sonic-host-services 798f9d1, and sonic-utilities 56d9bc76. The PR body now lists those integrated source changes. |
||
|
|
||
| # If no configuration entry exists for TELEMETRY, create one default port | ||
|
|
@@ -92,8 +108,8 @@ else | |
| fi | ||
| TELEMETRY_ARGS+=" --port $PORT" | ||
|
|
||
| CLIENT_AUTH=$(extract_field "$GNMI" '.client_auth') | ||
| if [ -z $CLIENT_AUTH ] || [ $CLIENT_AUTH == "false" ]; then | ||
| CLIENT_AUTH=$(extract_field "$GNMI" 'if .client_auth == null then empty else .client_auth end') | ||
| if [ "$CERTIFICATE_FREE_TLS" == "true" ] || [ "$CLIENT_AUTH" == "false" ] || { [ -z "$CLIENT_AUTH" ] && [ "$HAS_CLIENT_CA" == "false" ]; }; then | ||
| TELEMETRY_ARGS+=" --allow_no_client_auth" | ||
|
Comment on lines
+111
to
113
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Intent clarified in the PR body: CA-backed paths keep the existing mTLS default, while explicit client_auth=false remains the pre-existing opt-out that requests but does not require a client certificate. |
||
| fi | ||
|
|
||
|
|
@@ -139,16 +155,23 @@ else | |
| fi | ||
| TELEMETRY_ARGS+=" -gnmi_native_write=false" | ||
|
|
||
| USER_AUTH=$(extract_field "$GNMI" '.user_auth') | ||
| if [ ! -z "$USER_AUTH" ] && [ $USER_AUTH != "null" ]; then | ||
| USER_AUTH=$(extract_field "$GNMI" '.user_auth // empty') | ||
| USER_AUTH=$(tr -d '[:space:]' <<< "$USER_AUTH") | ||
| if [ "$CERTIFICATE_FREE_TLS" == "true" ]; then | ||
| USER_AUTH=$(tr ',' '\n' <<< "$USER_AUTH" | sed '/^[[:space:]]*cert[[:space:]]*$/d' | paste -sd, -) | ||
| if [ -z "$USER_AUTH" ]; then | ||
| USER_AUTH="none" | ||
| fi | ||
| fi | ||
| if [ -n "$USER_AUTH" ]; then | ||
| TELEMETRY_ARGS+=" --client_auth $USER_AUTH" | ||
|
|
||
| if [ $USER_AUTH == "cert" ]; then | ||
| if [[ ",$USER_AUTH," == *,cert,* ]]; then | ||
| # Reuse GNMI_CLIENT_CERT for telemetry service | ||
| TELEMETRY_ARGS+=" --config_table_name GNMI_CLIENT_CERT" | ||
|
|
||
| ENABLE_CRL=$(echo $GNMI | jq -r '.enable_crl') | ||
| if [ $ENABLE_CRL == "true" ]; then | ||
| ENABLE_CRL=$(extract_field "$GNMI" '.enable_crl // false') | ||
| if [ "$ENABLE_CRL" == "true" ]; then | ||
| TELEMETRY_ARGS+=" --enable_crl" | ||
| fi | ||
|
|
||
|
|
||
| +34 −0 | doc/grpc_telemetry.md | |
| +7 −2 | gnmi_server/clientCertAuth.go | |
| +26 −0 | gnmi_server/crl_test.go | |
| +1 −0 | go.mod | |
| +2 −0 | go.sum | |
| +10 −2 | pkg/interceptors/dpuproxy/proxy.go | |
| +47 −0 | pkg/interceptors/dpuproxy/proxy_test.go | |
| +361 −0 | pkg/interceptors/rpc_completion_log.go | |
| +729 −0 | pkg/interceptors/rpc_completion_log_test.go | |
| +9 −3 | pkg/interceptors/setup.go | |
| +24 −1 | pkg/interceptors/setup_test.go |
| +38 −11 | scripts/gnoi_shutdown_daemon.py | |
| +62 −26 | tests/gnoi_shutdown_daemon_test.py |
| +25 −0 | scripts/fast-reboot | |
| +125 −51 | scripts/reboot_smartswitch_helper | |
| +288 −0 | tests/reboot_smartswitch_helper_test.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intent clarified in the PR body: CA-backed paths keep the existing mTLS default, while explicit client_auth=false remains the pre-existing opt-out that requests but does not require a client certificate.