Skip to content

Adds TLS support for VNC API communication#27

Open
michaelhenkel wants to merge 8 commits intoJuniper:masterfrom
michaelhenkel:master
Open

Adds TLS support for VNC API communication#27
michaelhenkel wants to merge 8 commits intoJuniper:masterfrom
michaelhenkel:master

Conversation

@michaelhenkel
Copy link

Closes-jira-task: CEM-11391

Closes-jira-task: CEM-11391
@cijohnson
Copy link

Ignatious Johnson Christopher ok, How about https://github.com/Juniper/contrail-go-api/blob/master/cli/main.go,

  1. Can we add command line options and call setEncryptor based on the api_server_use_ssl flag similar to python vncAPI?
  2. Also we need to support https scheme in keystoneClient aswell (https://github.com/Juniper/contrail-go-api/blob/master/keystone.go#L18)

Michael Henkel
sure, the cli just need call the AddEncryptor function on the client object. and yes, encryption options must be exposed
yes, needed as well
both can and should be implemented in separate pull request. Perhaps Jim can do that

port int
httpClient *http.Client
auth Authenticator
encrypt Encryptor
Copy link
Contributor

Choose a reason for hiding this comment

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

I am a bit confused about this field, didn't find the usage of it. From my understanding, the encryption setting is done by function AddEncryption, which is a member of Client, is it right?

Comment on lines +82 to +90
if certFile != "" && keyFile != "" {
cert, err := tls.LoadX509KeyPair(certFile, keyFile)
if err != nil {
return nil
}
tlsConfig.Certificates = []tls.Certificate{cert}
} else {
tlsConfig.InsecureSkipVerify = true
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Ignatious talked with me about this part. We have question about why tlsConfig.InsecureSkipVerify is set back to true when we don't have certFile or keyFile, since we already have caFile, we can verify the server already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants