TLSv1.3: Upgrade ossltest and TLSProxy#1805
TLSv1.3: Upgrade ossltest and TLSProxy#1805mattcaswell wants to merge 3 commits intoopenssl:masterfrom
Conversation
apps/s_server.c
Outdated
There was a problem hiding this comment.
Reverse order? That's... amusing ;-)
test/testutil.c
Outdated
There was a problem hiding this comment.
Errr... is this some debugging code that slipped through?
There was a problem hiding this comment.
Yes...but you're reviewing the commits from #1804 that I said to ignore for this PR!
There was a problem hiding this comment.
Sorry, went through the "Files changed" tab without thinking...
|
Hmmm, I made that approval when looking over the TLSproxy commit, I thought it might possibly restrict the approval to just the changes in that commit.... Apparently not. But yeah, as far as approvals go, it's for the commits that actually does belong in this PR. |
|
Yes - understood - that's what I would have interpreted it as given I told you to ignore the other commits! I assume you also checked the ossltest commit too. |
|
I did |
ssl/ssl_ciph.c
Outdated
There was a problem hiding this comment.
shouldn't we use the ctype macros here?
There was a problem hiding this comment.
This is a comment on a commit from #1804 - so not relevant to this PR. Depending on where we end up with ciphersuite naming we might not need this change anyway...this was to allow "_" in ciphersuite names.
engines/e_ossltest.c
Outdated
There was a problem hiding this comment.
blank line before/after the define?
engines/e_ossltest.c
Outdated
There was a problem hiding this comment.
moce this init up to the var decl?
engines/e_ossltest.c
Outdated
There was a problem hiding this comment.
remove the ret variable and just do "return EVP..."
ssl/ssl_lib.c
Outdated
There was a problem hiding this comment.
did i already comment on making this a switch statement?
There was a problem hiding this comment.
Yes...fixed in #1804....not a commit for this PR
|
New commit pushed to address feedback comments. |
5197499 to
a15c7b6
Compare
|
I rebased this on top of the latest version of #1804 (that now has 4 commits so please ignore the first 4). That picked up the new ciphersuite name, so there is an amendment in this one to use it. Any more comments on this one? |
This might need more changes once we do a "real" TLS1.3 ciphersuite. But it should do for now.
Now that ossltest knows about a TLS1.3 cipher we can now do TLS1.3 in TLSProxy
Based on feedback received
a15c7b6 to
90dc674
Compare
|
Grrr...I missed a spot in Proxy.pm renaming to the new ciphersuite name. @richsalz please reconfirm? |
|
yes +1 |
|
Merged. Thanks. |
Checklist
Description of change
This builds on #1804. Please ignore the changes from the first 4 commits that are also in that PR.
This upgrades the ossltest engine so that it knows how to deal with a GCM based ciphersuite. It also upgrades TLSProxy so that it will use it where TLSv1.3 has been negotiated, and fixes some tests to work where TLSv1.3 is being used. This will enable us to write tests for the forthcoming TLSv1.3 implementation.