Skip to content

Add supoort for Tls 1.3 in Web cmdlets#13409

Merged
rjmholt merged 5 commits into
PowerShell:masterfrom
iSazonov:add-tls13
Sep 2, 2020
Merged

Add supoort for Tls 1.3 in Web cmdlets#13409
rjmholt merged 5 commits into
PowerShell:masterfrom
iSazonov:add-tls13

Conversation

@iSazonov
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov commented Aug 12, 2020 •
edited
Loading

PR Summary

Fix #13398

Add new element in WebSslProtocol enum.

PR Context

PR Checklist

@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Aug 12, 2020
@ghost ghost assigned rjmholt Aug 12, 2020
@iSazonov iSazonov mentioned this pull request Aug 12, 2020
Closed
17 tasks
@SteveL-MSFT
Copy link
Copy Markdown
Member

SteveL-MSFT commented Aug 12, 2020

Would be great if we can add a test.

@iSazonov
Copy link
Copy Markdown
Collaborator Author

iSazonov commented Aug 13, 2020

I added tests.

@iSazonov iSazonov requested a review from anmenaga as a code owner August 13, 2020 05:11
@iSazonov
Copy link
Copy Markdown
Collaborator Author

iSazonov commented Aug 13, 2020

After investigations of new test failures I found that .Net delegates the TLS support to underlying OS. The TLS 1.3 support on all OS-s is limited.
I enable TLS 1.3 on Windows 10 1903 and get:

Invoke-WebRequest -SslProtocol tls13 https://tls13.akamai.io/

Invoke-WebRequest: Authentication failed because the remote party sent a TLS alert: 'HandshakeFailure'.

MacOS haven't the support at all. Linux - depends on used OpenSSL lib.

@SteveL-MSFT We need a conclusion:

  • remove all new tests and our test WebListener updates
  • put new tests in pending
  • ???

TravisEz13
TravisEz13 reviewed Aug 14, 2020
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 Outdated
Copy link
Copy Markdown
Member

@TravisEz13 TravisEz13 Aug 14, 2020

Choose a reason for hiding this comment

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

Can you add a comment that these are failing test cases

TravisEz13
TravisEz13 reviewed Aug 14, 2020
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 Outdated
Copy link
Copy Markdown
Member

@TravisEz13 TravisEz13 Aug 14, 2020

Choose a reason for hiding this comment

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

similar comment here

@TravisEz13
Copy link
Copy Markdown
Member

TravisEz13 commented Aug 14, 2020

@SteveL-MSFT We need a conclusion:

  • remove all new tests and our test WebListener updates
  • put new tests in pending
  • ???

Put the tests at pending and file an issue. Note the issue in a comment in the test code.

Best if you pend the test cases like this:

PowerShell/test/powershell/Modules/PSDesiredStateConfiguration/PSDesiredStateConfiguration.Tests.ps1

Line 129 in f438220

Set-ItResult -Pending -Because "https://github.com/PowerShell/PSDesiredStateConfiguration/issues/26"

@iSazonov iSazonov mentioned this pull request Aug 14, 2020
Closed
@iSazonov
Copy link
Copy Markdown
Collaborator Author

iSazonov commented Aug 14, 2020

@TravisEz13 Tracking issue is created and the comment is added to the tests. (I can not use Set-ItResult without refactoring the tests.)

@iSazonov
Copy link
Copy Markdown
Collaborator Author

iSazonov commented Aug 21, 2020

Rebased to pass CIs.

@ghost ghost added the Review - Needed The PR is being reviewed label Aug 28, 2020
@ghost
Copy link
Copy Markdown

ghost commented Aug 28, 2020

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

@rjmholt
Copy link
Copy Markdown
Collaborator

rjmholt commented Aug 28, 2020

@iSazonov looks like this may need another rebase. Once CI passes, I'll merge

@ghost ghost removed the Review - Needed The PR is being reviewed label Aug 28, 2020
@rjmholt rjmholt added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Aug 28, 2020
@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Aug 28, 2020
@rjmholt rjmholt added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Sep 1, 2020
@rjmholt
Copy link
Copy Markdown
Collaborator

rjmholt commented Sep 1, 2020

CI still failing

@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Sep 2, 2020
@rjmholt rjmholt merged commit 81d5a86 into PowerShell:master Sep 2, 2020
@iSazonov iSazonov deleted the add-tls13 branch September 3, 2020 03:08
@iSazonov iSazonov added this to the 7.1.0-preview.7 milestone Sep 3, 2020
@ghost
Copy link
Copy Markdown

ghost commented Sep 8, 2020

🎉v7.1.0-preview.7 has been released which incorporates this pull request.:tada:

Handy links:

@ghost ghost mentioned this pull request Sep 8, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TravisEz13 TravisEz13 TravisEz13 approved these changes

@SteveL-MSFT SteveL-MSFT SteveL-MSFT approved these changes

@daxian-dbw daxian-dbw Awaiting requested review from daxian-dbw

@adityapatwardhan adityapatwardhan Awaiting requested review from adityapatwardhan

@markekraus markekraus Awaiting requested review from markekraus

@anmenaga anmenaga Awaiting requested review from anmenaga

+2 more reviewers

@rjmholt rjmholt rjmholt approved these changes

@vexx32 vexx32 vexx32 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@rjmholt rjmholt

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Milestone

7.1.0-preview.7

Development

Successfully merging this pull request may close these issues.

Support for TLS 1.3

5 participants

@iSazonov @SteveL-MSFT @TravisEz13 @rjmholt @vexx32