openconfig / gnmic

gNMIc is a gNMI CLI client and collector
https://gnmic.openconfig.net
Apache License 2.0
170 stars 55 forks source link

explicitly set default TLS cipher suite #371

Closed karimra closed 7 months ago

karimra commented 7 months ago

367

vincentbernat commented 7 months ago

Are you sure this is is equivalent to before? This is enabling ciphers that are deemed insecure since a long time. There is tls.CipherSuites() which has more ciphers but is still avoiding the insecure ones. 3DES and RC4 should never be used anymore. Which cipher SROS was using that was put on the insecure list? Maybe an AES CBC one?

karimra commented 7 months ago

This is just to solve #367, working on making the cipher suites configurable. SROS only supports RSA as key exchange algorithm.

vincentbernat commented 7 months ago

tls.CipherSuites() also includes RSA key exchange. That was the default value before I think. The one you took are only used for ordering. Nobody keeps RC4 in the default list of ciphers.

karimra commented 7 months ago

I understand, like I said this was a fix for a specific issue to unblock someone. There will be another PR where the default cipher suites chosen will better mimic what the stdlib does. tls.CipherSuites() does not return what the stdlib selects by default (it includes both TLS1.2 and 1.3 and does not follow the preferred order), it's a bit more complicated than that, see here