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

Various changes from Chromium. #50

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

davidben
Copy link

Here are a bunch of patches to upstream from Chromium. As with previous ones, these fall under the "The Chromium Authors" section of the LICENSE file.

They include:

  • Minor fixes
  • FALLBACK_SCSV
  • A pile of bugs in the TLS 1.2 support
  • DHE_RSA key exchange
  • Pure-Python AES-GCM implementation. Note that the "openssl" backend is intentionally omitted here. I tried to use the M2Crypto raw AES wrappers, noticed it was completely non-functional, and then gave up.

SSL 3.0 requires minimal padding. (TLS doesn't require it but accepts it, so
apply uniformly to both.)

From Chromium.
Adapted from a Chromium patch.
Client auth now participates in signature algorithms.

From Chromium.
@tomato42
Copy link
Contributor

In general: tests for those changes?

(more comments coming later)

@tomato42
Copy link
Contributor

ok, I think that's all

BTW, I have created some unit tests for DHE key exchange here: https://github.com/tomato42/tlslite-1/tree/dhe-kex-2 you may want to reuse them.

@davidben
Copy link
Author

BTW, I have created some unit tests for DHE key exchange here: https://github.com/tomato42/tlslite-1/tree/dhe-kex-2 you may want to reuse them.

Oh, handy. It looks like you have some bits of DHE and ECDHE already? I haven't looked too closely yet. Perhaps it'd be better to pull that one out [edit: remove it from my branch that is, not yours] for the time being until we sort out how to merge them? ECDHE would be handy for Chromium's test server at some point anyway; the motivation for most of these was that we couldn't easily tighten our False Start requirements until the test server was able to simulate them.

(I'll try to get to your comments later today or this weekend. Depends how busy I end up with other stuff.)

@tomato42
Copy link
Contributor

Perhaps it'd be better to pull that one out

The DHE implementation I've made is less complete than what you have provided here, so I don't think pulling mine over yours is worth it. The ECDHE also depends on external libraries so will need a lot of changes to make it worthy of a pull request. That branch is just a rough draft to workout what tests/infrastructure are needed to support PFS. Transplanting the tests should be easier.

the motivation for most of these was that we couldn't easily tighten our False Start requirements until the test server was able to simulate them.

well, I'm working on it so that NSS and OpenSSL (among others) have better test suite (in the form of tlsfuzzer), so I'll be working to the bring tlslite to feature parity with them

@tomato42
Copy link
Contributor

one thing that @trevp suggested, is to workout merge of this pull request together with my implementation of encrypt-then-mac from pull #48, so if you could review that patch set it should be easier to arrive at a merge, especially with regards to 12bda1f

@davidben
Copy link
Author

tlsfuzzer: Neat. If you haven't seen them, we have some fairly extensive tests in BoringSSL based using Go's TLS implementation as a hackable base. It's not quite a fuzzer, but I'm quite happy with what we've been able to do with it. (It helps that it was basically already feature-complete to start with. And Go's concurrency model means it's all straight-line code with nothing funny, which is nice.)
https://boringssl.googlesource.com/boringssl/+/master/ssl/test/
https://boringssl.googlesource.com/boringssl/+/master/ssl/test/runner/runner.go

(I'd actually like to move Chromium's test server to Go someday so I don't have to maintain two different test-only SSL stacks, but that'll take more work and politics than I have the bandwidth for right now. :-) In the meantime, feature parity in tlslite is quite useful.)

@trevp
Copy link
Owner

trevp commented Feb 21, 2015

Thanks David, this is good stuff, I agree with most of Hubert's comments. Tests would be great, both client-server and unit tests (Hubert added a framework for that). Without tests, in a weakly-typed language, things will break over time. The DHE seems like it could use some refactoring too.

Once you update this I'd like to absorb these changes, and Hubert's encrypt-then-MAC, then do a release soon (week or two) so there's finally some secure ciphersuites.

@tomato42
Copy link
Contributor

@davidben well, sqlite has 3 independent test suites, I don't think we're overdoing it with just two test servers ;)

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