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.

Sign up for GitHub

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

Jump to bottom

Update grpc, and update credentials_manager to use FileWatcherCertificateProvider #850

Merged
merged 36 commits into from
Dec 16, 2021
Merged

Update grpc, and update credentials_manager to use FileWatcherCertificateProvider #850

merged 36 commits into from
Dec 16, 2021

Conversation

koconnor29
Copy link
Contributor

@koconnor29 koconnor29 commented Nov 5, 2021 •
edited by bocon13
Loading

Also include a few other small things:

  • bump gRPC version from 1.33.2 to 1.35.0
  • add a test for credentials_manager
  • update go toolchain
  • add a missing include statement to admin_service.cc

Fixes: #844

@koconnor29
Update grpc, and update credentials_manager to use FileWatcherCertifi…
bde6d5a 
…cateProvider.

Also include a few other small things:

- add a test for credentials_manager
- update go toolchain
- add a missing include statement to admin_service.cc
@koconnor29
Update to ensure reuse checks pass
d1327c3
bocon13
bocon13 previously approved these changes Nov 5, 2021
View reviewed changes
Copy link
Member

@bocon13 bocon13 left a comment

Choose a reason for hiding this comment

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

LGTM

bocon13
bocon13 previously approved these changes Nov 5, 2021
View reviewed changes
@bocon13 bocon13 mentioned this pull request Nov 5, 2021
Merged
@bocon13 bocon13 changed the base branch from main to bocon/signal-include November 5, 2021 08:16
@bocon13 bocon13 changed the base branch from bocon/signal-include to main November 5, 2021 08:23
@bocon13 bocon13 changed the base branch from main to bocon/signal-include November 5, 2021 08:23
@bocon13 bocon13 requested a review from Yi-Tseng November 5, 2021 08:34
@cholve
Copy link
Contributor

cholve commented Nov 5, 2021

A family that codes together, stays together.

@pudelkoM
Copy link
Member

pudelkoM commented Nov 5, 2021

I'd split the dependency update and credential_manager in two PRs.

bocon13
bocon13 reviewed Nov 17, 2021
View reviewed changes
.reuse/dep5 Outdated Show resolved Hide resolved
koconnor29 and others added 2 commits November 17, 2021 00:45
@koconnor29 @bocon13
Update stratum/lib/security/credentials_manager_test.cc
5c4da40 
Co-authored-by: Brian O'Connor <bocon@opennetworking.org>
@koconnor29 @bocon13
Update stratum/lib/security/credentials_manager_test.cc
cd3e5d6 
Co-authored-by: Brian O'Connor <bocon@opennetworking.org>
@bocon13 bocon13 deleted the branch stratum:main November 22, 2021 10:14
@bocon13 bocon13 closed this Nov 22, 2021
@bocon13 bocon13 reopened this Nov 22, 2021
@pudelkoM pudelkoM deleted the branch stratum:main November 22, 2021 18:47
@pudelkoM pudelkoM closed this Nov 22, 2021
@pudelkoM pudelkoM reopened this Nov 22, 2021
@pudelkoM
Copy link
Member

Just reset the bcm_sdk_wrapper files. They're not formatted.

@bocon13 bocon13 removed this from the 2021-12 Release milestone Dec 7, 2021
@bocon13 bocon13 changed the base branch from bocon/signal-include to main December 14, 2021 02:23
@bocon13
Switch clients to new gRPC classes
883df7d 
* FileWatcherCertificateProvider
* TlsChannelCredentialsOptions
@codecov
Copy link

codecov bot commented Dec 14, 2021 •
edited
Loading

Codecov Report

Merging #850 (938bed5) into main (8b5eff6) will increase coverage by 0.25%.
The diff coverage is 90.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #850      +/-   ##
==========================================
+ Coverage   78.55%   78.81%   +0.25%     
==========================================
  Files         334      336       +2     
  Lines       30077    30133      +56     
==========================================
+ Hits        23628    23750     +122     
+ Misses       6449     6383      -66
Impacted Files Coverage Δ
stratum/lib/security/cert_utils.cc 89.41% <89.41%> (ø)
stratum/lib/security/cert_utils.h 100.00% <100.00%> (ø)
stratum/lib/security/credentials_manager.cc 85.71% <100.00%> (+79.76%) ⬆️
stratum/lib/security/test_main.cc 95.00% <0.00%> (ø)
stratum/p4c_backends/fpm/parser_field_mapper.cc 92.02% <0.00%> (+0.26%) ⬆️

@bocon13
Copy link
Member

bocon13 commented Dec 14, 2021 •
edited
Loading

Tested on bmv2:

brian@menlo-pdp-lotta-nics:/stratum$ sudo bazel-bin/stratum/hal/bin/bmv2/stratum_bmv2 \
     -persistent_config_dir=/tmp/ \
     -chassis_config_file=${BMV2_DIR}/chassis_config.pb.txt \
     -initial_pipeline=${BMV2_DIR}/dummy.json \
     -forwarding_pipeline_configs_file=/tmp/bmv2_pipeline_cfg \
     -bmv2_log_level=debug \
     -ca_cert tools/tls/certs/ca.crt \
     -server_cert tools/tls/certs/stratum.crt \
     -server_key tools/tls/certs/stratum.key

Stratum starts cleanly when providing a certificate.

Stratum Log 
I20211214 05:44:10.424633 86066 logging.cc:63] Stratum version 0 built at 1970-01-01T00:00:00+00:00 on host redacted by user redacted.
E20211214 05:44:10.426285 86066 main.cc:124] Starting bmv2 simple_switch and waiting for P4 pipeline
[05:44:10.430] [bmv2] [D] [thread 86066] Set default default entry for table 't_drop': a_drop - 
I20211214 05:44:10.432965 86066 hal.cc:127] Setting up HAL in COLDBOOT mode...
I20211214 05:44:10.433064 86066 config_monitoring_service.cc:94] Pushing the saved chassis config read from /stratum/stratum/hal/bin/bmv2/chassis_config.pb.txt...
I20211214 05:44:10.441839 86066 bmv2_chassis_manager.cc:519] Registered port status callbacks successfully for node 1.
I20211214 05:44:10.441881 86066 bmv2_chassis_manager.cc:61] Adding port 1 to node 1
[05:44:10.441] [bmv2] [D] [thread 86066] Adding interface veth0 as port 1
I20211214 05:44:10.536839 86066 bmv2_chassis_manager.cc:61] Adding port 2 to node 1
[05:44:10.536] [bmv2] [D] [thread 86066] Adding interface veth2 as port 2
I20211214 05:44:10.596419 86066 p4_service.cc:121] Pushing the saved forwarding pipeline configs read from /tmp/bmv2_pipeline_cfg...
E20211214 05:44:10.596537 86066 utils.cc:112] StratumErrorSpace::ERR_FILE_NOT_FOUND: /tmp/bmv2_pipeline_cfg not found.
E20211214 05:44:10.596948 86066 utils.cc:68] Return Error: ReadFileToString(filename, &text) failed with StratumErrorSpace::ERR_FILE_NOT_FOUND: /tmp/bmv2_pipeline_cfg not found.
W20211214 05:44:10.596966 86066 p4_service.cc:130] No saved forwarding pipeline config found at /tmp/bmv2_pipeline_cfg. This is normal when the switch is just installed and no master controller is connected yet.
E20211214 05:44:10.606514 86066 hal.cc:220] Stratum external facing services are listening to 0.0.0.0:9339, 0.0.0.0:9559, localhost:9559...
I20211214 05:44:10.632382 86084 bmv2_chassis_manager.cc:453] State of port 1 in node 1: UP.
I20211214 05:44:10.632576 86084 bmv2_chassis_manager.cc:453] State of port 2 in node 1: UP.

brian@menlo-pdp-lotta-nics:/stratum$ bazel run //stratum/tools/gnmi:gnmi_cli --config asan -- \
  get /interfaces/interface[name=*]/ \
    --ca-cert /stratum/tools/tls/certs/ca.crt \
    --client-cert /stratum/tools/tls/certs/client.crt \
    --client-key /stratum/tools/tls/certs/client.key \
    --grpc_addr stratum.local:9339

gNMI CLI successfully connects to Stratum, including certificate validation.

gNMI CLI Log 
REQUEST
path {
  elem {
    name: "interfaces"
  }
  elem {
    name: "interface"
    key {
      key: "name"
      value: "*"
    }
  }
}
encoding: PROTO

RESPONSE

notification {

timestamp: 1639460866134566466

update {

path {

elem {

name: "interfaces"

}

elem {

name: "interface"

key {

key: "name"

value: "veth0"

}

}

elem {

name: "config"

}

elem {

name: "enabled"

}

}

val {

bool_val: true

}

}

}

...



brian@menlo-pdp-lotta-nics:/stratum$ openssl s_client -showcerts -connect stratum.local:9339

OpenSSL connects and successfully validates certs.

openssl log 
CONNECTED(00000003)
depth=1 C = US, ST = CA, L = Menlo Park, O = Open Networking Foundation, OU = Stratum, CN = Stratum CA
verify return:1
depth=0 C = US, ST = CA, L = Menlo Park, O = Open Networking Foundation, OU = Stratum, CN = stratum.local
verify return:1
---
Certificate chain
 0 s:/C=US/ST=CA/L=Menlo Park/O=Open Networking Foundation/OU=Stratum/CN=stratum.local
   i:/C=US/ST=CA/L=Menlo Park/O=Open Networking Foundation/OU=Stratum/CN=Stratum CA
-----BEGIN CERTIFICATE-----
MIIDdTCCAl0CCQCGHe92uBWKHDANBgkqhkiG9w0BAQsFADB7MQswCQYDVQQGEwJV
UzELMAkGA1UECAwCQ0ExEzARBgNVBAcMCk1lbmxvIFBhcmsxIzAhBgNVBAoMGk9w
ZW4gTmV0d29ya2luZyBGb3VuZGF0aW9uMRAwDgYDVQQLDAdTdHJhdHVtMRMwEQYD
VQQDDApTdHJhdHVtIENBMB4XDTIxMTIxNDAzMTUwN1oXDTIyMDExMzAzMTUwN1ow
fjELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNBMRMwEQYDVQQHDApNZW5sbyBQYXJr
MSMwIQYDVQQKDBpPcGVuIE5ldHdvcmtpbmcgRm91bmRhdGlvbjEQMA4GA1UECwwH
U3RyYXR1bTEWMBQGA1UEAwwNc3RyYXR1bS5sb2NhbDCCASIwDQYJKoZIhvcNAQEB
BQADggEPADCCAQoCggEBAMw9iIIGyHYTXwbHwlcQ+Cj9Ho5CEkUZfWfIElPPlse8
hvsiqskP6a6sAtirWYqnPok7bCOTu1iTygEJrBNTBxD4lRXZqy8dHE6ciV+ZUqeA
Q+ftpc53F6Mn9PhNOemhGVWbButF4mnrQcn9HfJ5mI1E85jDyQH2DSYLNjciAKur
5Ik02sl08fLBS3psohhRyN2fJP729I+vMfM+k7Lib9QCmCPdajcXCRNj2cVXZ9MA
dzKkOtEIuhL2dFOf8nsFFfFZOuh8kJU7ykuZBuYzuRbz/aGdlPOS9mKS8joZBbwf
NlMK+O8Kndwq/mnBpvX+51cpvdJ5JihD4CjGJW3IJSsCAwEAATANBgkqhkiG9w0B
AQsFAAOCAQEAOpM4hHEImJ8zZaApJYDfnH7KDkOjmWzE6rAfsrB1Jo6RRItALDak
O9VUt8Cx/EOH6oj+7Ejs21gnzcZVIaEpCz8U1Ej4wZJ6DBLfzh2sG15wooh3Ch6+
K+LtgfsxjgP2/wLJ+RKFRg9OryQErArd8/0fB3gr0mmKT01xafqTg+FCcW11Ao1p
/ZDx8vU46bwFUZk9BkhEMJlWQSOhHtZYEDSqNHMBhpjkVVrIQcY2EPKEuSrcPArF
HVNdZILGOwCdqQfmVBM6mKw6Z5IixT2LXtWfIH0yQ/06mQboUXuwDRL9WS4II4pV
CPs2eYaY1O56Y57zmoDj7ky/UEzaUyTfvQ==
-----END CERTIFICATE-----
---
Server certificate
subject=/C=US/ST=CA/L=Menlo Park/O=Open Networking Foundation/OU=Stratum/CN=stratum.local
issuer=/C=US/ST=CA/L=Menlo Park/O=Open Networking Foundation/OU=Stratum/CN=Stratum CA
---
No client certificate CA names sent
Peer signing digest: SHA256
Server Temp Key: ECDH, P-256, 256 bits
---
SSL handshake has read 1561 bytes and written 302 bytes
Verification: OK
---
New, TLSv1.2, Cipher is ECDHE-RSA-AES256-GCM-SHA384
Server public key is 2048 bit
Secure Renegotiation IS supported
Compression: NONE
Expansion: NONE
No ALPN negotiated
SSL-Session:
    Protocol  : TLSv1.2
    Cipher    : ECDHE-RSA-AES256-GCM-SHA384
    Session-ID: 8ED40A52F2C71BA5B8A82B3847A97F08F64F087E16D7695D5BF819B8A9041E73
    Session-ID-ctx: 
    Master-Key: FFEEE6CB0DBF6551A981BB21042C655986BE1FB190D84D42F4339A5BB83D6E324CDBAF8EEBCE1767DC347FCEEAA8F5B0
    PSK identity: None
    PSK identity hint: None
    SRP username: None
    TLS session ticket lifetime hint: 7200 (seconds)
    TLS session ticket:
    0000 - 47 fc 6f a3 c5 cf d4 b8-54 5e 6d 08 73 dc 4b e8   G.o.....T^m.s.K.
    0010 - 45 c3 6f 84 51 d5 8b 34-58 77 50 19 3e 2c 7f 01   E.o.Q..4XwP.>,..
    0020 - 21 2a 91 76 96 4a 1e e8-9b a8 91 32 35 0f ab 2d   !*.v.J.....25..-
    0030 - 75 6d 55 1f 35 72 59 6e-55 6f 1c dd 6f 16 6a c8   umU.5rYnUo..o.j.
    0040 - e4 66 27 61 b3 b4 9c 6b-17 8e e3 8a 29 f1 2c e2   .f'a...k....).,.
    0050 - b9 7a 07 01 c8 d0 bd 46-05 57 63 e9 77 2a 26 17   .z.....F.Wc.w*&.
    0060 - be 66 8e c7 de 3f 78 03-4d f5 4d 9e 95 6f e8 65   .f...?x.M.M..o.e
    0070 - 5e 02 59 4a d4 ef 90 dc-79 8e 4f e5 20 d2 71 9f   ^.YJ....y.O. .q.
    0080 - 18 8e a2 cb 8a 17 d4 aa-fb 3d 29 51 6f 96 ca 40   .........=)Qo..@
    0090 - 2d 86 d7 9c 84 1d b4 6d-a8 9a a9 7b 76 b8 0e 3d   -......m...{v..=
    00a0 - fb 9e f7 eb d5 1e c1 d7-cc 98 a6 38 f1 f7 c9 04   ...........8....

Start Time: 1639462075
Timeout   : 7200 (sec)
Verify return code: 0 (ok)
Extended master secret: yes

@bocon13 bocon13 added this to the 2022-03 Release milestone Dec 14, 2021
@bocon13 bocon13 merged commit a553f1d into stratum:main Dec 16, 2021
bocon13 added a commit that referenced this pull request Dec 16, 2021
@koconnor29 @bocon13
Update gRPC version and use new FileWatcherCertificateProvider (#850)
7e84e6b 
* Bump gRPC version from 1.33.2 to 1.35.0
    * Bump go toolchain from 0.20.3 to 0.24.11 (for gRPC)
* Update CredentialsManager to use FileWatcherCertificateProvider
* Add a test for CredentialsManager
* Add cert_utils to generate X509 certs for testing
* Update gnmi_cli and stratum_replay to use new APIs:
    * TlsChannelCredentialsOptions
    * FileWatcherCertificateProvider

Co-authored-by: Brian O'Connor <bocon@opennetworking.org>
@bocon13 bocon13 deleted the koconnor-grpc-credentials-manager-updates branch December 16, 2021 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pudelkoM pudelkoM pudelkoM left review comments

@bocon13 bocon13 bocon13 approved these changes

@Yi-Tseng Yi-Tseng Awaiting requested review from Yi-Tseng

Assignees
No one assigned
Labels
Merge After Release
Projects
None yet
Milestone
2022-03 Release
Development

Successfully merging this pull request may close these issues.

Replace CredentialsReloadInterface with FileWatcherCertificateProvider
4 participants
@koconnor29 @cholve @pudelkoM @bocon13