skip TLS check for detecting layer data #256
Conversation
|
@Quentin-M PTAL. Thanks! |
|
Disabling these checks should be behind a flag if we're going to support it because defaulting to this behavior is dangerous. You should be adding your certificates to the system root (and mounting that into a container). |
|
@jzelinskie Thanks for your review and advice. Yes. Users can avoid this error by adding the certificates. But if the layer data is from several different servers, this way maybe is not so convenient. At the same time, it will restrict the container to run anywhere. Adding a flag to control TLS check is a good way. I think we can add a flag |
|
@jzelinskie @Quentin-M Whether we need to add a flag to support disabling this check? |
|
@supereagle I would not be opposed to adding a flag to the binary for testing called |
jzelinskie
added
area/usability
|
@jzelinskie Is the flag |
|
@supereagle For TLS to be secure, it needs to use PKI rather than blindly trusting any certificate it sees. People should be able to run Clair without verifying certificates, but if they want run it in that mode, they should be forced to acknowledge that it's insecure. |
|
@jzelinskie Thanks. I will add this flag. |
|
@jzelinskie Have added the flag |
|
@jzelinskie Any comments about the new work in this PR? |
|
I'm not 100% convinced that this API needs enable dynamic toggling of functionality. thoughts @Quentin-M ? |
|
This needs a rebase. |
| @@ -57,6 +58,7 @@ func main() { | |||
| flagConfigPath := flag.String("config", "/etc/clair/config.yaml", "Load configuration from the specified file.") | |||
| flagCPUProfilePath := flag.String("cpu-profile", "", "Write a CPU profile to the specified file before exiting.") | |||
| flagLogLevel := flag.String("log-level", "info", "Define the logging level.") | |||
| flagInsecureTLS := flag.Bool("insecure-tls", false, "Disable TLS check when detect the data of layers.") | |||
There was a problem hiding this comment.
Disable TLS server's certificate chain and host name verification when pulling layers
| @@ -45,6 +46,10 @@ var ( | |||
|
|
|||
| // ErrCouldNotFindLayer is returned when we could not download or open the layer file. | |||
| ErrCouldNotFindLayer = cerrors.NewBadRequestError("could not find layer") | |||
|
|
|||
| insecureTLSLock sync.Mutex | |||
There was a problem hiding this comment.
I feel like the mutex is overkill.
There was a problem hiding this comment.
OK, I will remove it.
| @@ -45,6 +46,10 @@ var ( | |||
|
|
|||
| // ErrCouldNotFindLayer is returned when we could not download or open the layer file. | |||
| ErrCouldNotFindLayer = cerrors.NewBadRequestError("could not find layer") | |||
|
|
|||
| insecureTLSLock sync.Mutex | |||
| // insecureTLS controls the TLS check when detect the data of layers, enabled in default. | |||
There was a problem hiding this comment.
insecureTLS controls whether TLS server's certificate chain and host name are verified when pulling layers
| @@ -120,3 +129,10 @@ func DetectData(format, path string, headers map[string]string, toExtract []stri | |||
|
|
|||
| return nil, cerrors.NewBadRequestError(fmt.Sprintf("unsupported image format '%s'", format)) | |||
| } | |||
|
|
|||
| // SetInsecureTLS sets the insecureTLS to control the TLS check when detect the data of layers. | |||
| func SetInsecureTLS(insecure bool) { | |||
There was a problem hiding this comment.
I would not bother having a function if there is no mutex.
|
I understand the use-case and would be willing to merge this. |
|
Too many conflicts while rebasing. Create another PR #331. This PR can be closed. |
This skips TLS check for detecting layer data from Server
configured with TLS.
Fixes #251