dragonflyoss / Dragonfly2

Dragonfly is an open source P2P-based file distribution and image acceleration system. It is hosted by the Cloud Native Computing Foundation (CNCF) as an Incubating Level Project.
https://d7y.io
Apache License 2.0
2.22k stars 280 forks source link

AutoIssueCert loses control when opt.Security. CACert is not empty #3422

Open karlhjm opened 1 month ago

karlhjm commented 1 month ago

Bug report:

https://github.com/dragonflyoss/Dragonfly2/blob/505d53b83c3000eff62cfda00a062c11b2b77bae/client/daemon/daemon.go#L154

The meaning of AutoIssueCert is as follows, but when CACert is not empty, tls is still used for peer to connect to the manager

type GlobalSecurityOption struct { // AutoIssueCert indicates to issue client certificates for all grpc call // if AutoIssueCert is false, any other option in Security will be ignored AutoIssueCert bool mapstructure:"autoIssueCert" yaml:"autoIssueCert" // CACert is the root CA certificate for all grpc tls handshake, it can be path or PEM format string CACert types.PEMContent mapstructure:"caCert" yaml:"caCert" // TLSVerify indicates to verify client certificates. TLSVerify bool mapstructure:"tlsVerify" yaml:"tlsVerify" // TLSPolicy controls the grpc shandshake behaviors: // force: both ClientHandshake and ServerHandshake are only support tls // prefer: ServerHandshake supports tls and insecure (non-tls), ClientHandshake will only support tls // default: ServerHandshake supports tls and insecure (non-tls), ClientHandshake will only support insecure (non-tls) TLSPolicy string mapstructure:"tlsPolicy" yaml:"tlsPolicy" // CertSpec is the desired state of certificate. CertSpec *CertSpec mapstructure:"certSpec" yaml:"certSpec" }

Expected behavior:

when AutoIssueCert=false and CACert="a ca cert", dfdaemon still use non-tls to connect the manager

How to reproduce it:

set AutoIssueCert=false and CACert="a ca cert" in dfdaemon's global security config

karlhjm commented 1 month ago

Why is it necessary to hard code "withTLS" to disable TLS for peerListener? Shouldn't withTLS be configured through TLSVerify or TLSPolicy?

https://github.com/dragonflyoss/Dragonfly2/blob/505d53b83c3000eff62cfda00a062c11b2b77bae/client/daemon/daemon.go#L512

https://github.com/dragonflyoss/Dragonfly2/blob/505d53b83c3000eff62cfda00a062c11b2b77bae/client/daemon/daemon.go#L608