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.

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

add two new algos for tls connection from client side. #4

Merged
merged 1 commit into from
Jan 13, 2016

Conversation

hopkings2008
Copy link
Contributor

Signed-off-by: yuzou zouyu7@huawei.com

@hopkings2008
Copy link
Contributor Author

Dear maintainers,
Since golang 1.5.2 already supports TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 and TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, added these two algos, so that client can use them to communicate with the registry.

@thaJeztah
Copy link
Member

Will this be an issue for the ARM builds of Docker? (iirc, those still rely on Go 1.4.3 at this moment).

@calavera
Copy link
Contributor

calavera commented Jan 6, 2016

we should add them with build tags.

// +build go1.5

@hopkings2008
Copy link
Contributor Author

@thaJeztah thanks for your checking and @calavera , thanks for your good suggestion. And i have added the build tag into that config.go file and if it is go1.5, the compiler will pick up the config.go for building, and if it is go1.4 or later, the compiler will pick up config_legacy.go for building. Please check it again and welcome your suggestions.

@calavera
Copy link
Contributor

calavera commented Jan 8, 2016

Thanks @hopkings2008.

I don't think we need to copy everything in that file in for both versions do we? We could move everything that's not clientCipherSuites into a common file without the build tag, so we'd have something like directory structure:

  • tlsconfig_client_ciphers.go : with only clientCipherSuite and the 1.5 build tag.
  • tlsconfig_legacy_client_ciphers.go: with only clientCipherSuite and the !1.5 build tag.
  • tlsconfig.go: with everything else.

Does this make sense?

@hopkings2008
Copy link
Contributor Author

@calavera , good suggestion. I will make a change accordingly, thanks a lot.

Signed-off-by: yuzou <zouyu7@huawei.com>
@hopkings2008
Copy link
Contributor Author

@calavera and @thaJeztah , I have made the changes according to the suggestion, and make a test locally for the new ciphers, please check it again, thanks a lot.

@calavera
Copy link
Contributor

it looks fantastic, thanks a lot.

LGTM

@thaJeztah
Copy link
Member

lgtm (not a maintainer here), thanks!

@calavera
Copy link
Contributor

We'll need to tag this change before vendoring it in docker, but I'm going to merge this for now.

Thanks.

calavera added a commit that referenced this pull request Jan 13, 2016
add two new algos for tls connection from client side.
@calavera calavera merged commit 082e382 into docker:master Jan 13, 2016
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.

None yet

3 participants