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

Improve error messaging for SSL related errors #3568

Conversation

alexander-smolyakov
Copy link
Contributor

@alexander-smolyakov alexander-smolyakov commented Oct 18, 2021

Issue description:
Recently we saw a lot of agent SSL issues not related to the agent. Usually, we see a generic error message like the below:

System.Net.Http.HttpRequestException: The SSL connection could not be established, see inner exception.
---> System.Security.Authentication.AuthenticationException: The remote certificate is invalid according to the validation procedure.

We need to increase verbosity to such errors - to be able to understand the root cause more quickly.

Fix description:
As a fix, we will use ServerCertificateCustomValidationCallback to capture the SSL-related data about web requests. We will set this custom callback for every Vss connection that will be created by the pipeline agent during the runtime.

Changelog:

  • Introduced SslUtil class that contains the implementation of ServerCertificateCustomValidationCallback
  • Updated method CreateConnection of VssUtil class to set ServerCertificateCustomValidationCallback for each Vss connections
  • Other methods were updated to pass TraceWriter in the CreateConnection method to be able to write SSL-related data to the agent's log

Examples of logging:

Examples
[2021-11-16 10:32:39Z INFO AgentServer] Diagnostic data for request:
[SSL Policy Errors]
RemoteCertificateChainErrors: ChainStatus has returned a non empty array
[HttpRequest]
Requested URI: https://v-alsmo.visualstudio.com/_apis/connectionData?connectOptions=0&lastChangeId=109199399&lastChangeId64=109199399
Request method: GET
[HttpRequestHeaders]
X-TFS-Session: 212cdc39-bc97-47ee-be06-87c6f7dddd81
X-VSS-E2EID: 94f6e84e-faf3-4f42-ae8e-cf804e7461c5
User-Agent: VSServices/16.196.31911.0, (NetStandard; Microsoft Windows 10.0.19043), VstsAgentCore-win-x64/2.999.999, (Microsoft Windows 10.0.19043)
[Certificate]
Effective date: 11/10/2021 6:59:48 PM
Expiration date: 11/10/2022 6:59:48 PM
Issuer: CN=Microsoft RSA TLS CA 02, O=Microsoft Corporation, C=US
Subject: CN=visualstudio.com
[2021-11-16 10:31:52Z INFO AgentServer] Diagnostic data for request:
[SSL Policy Errors]
RemoteCertificateNameMismatch: Certificate name mismatch
RemoteCertificateChainErrors: ChainStatus has returned a non empty array
[HttpRequest]
Requested URI: https://v-alsmo.visualstudio.com/_apis/distributedtask/pools/1/sessions/f98284a4-8bd4-43ab-9e37-4c21403c2d7d
Request method: DELETE
[HttpRequestHeaders]
X-TFS-Session: 0252cb29-20b8-4e31-9f5b-7903e236fe76
X-VSS-E2EID: 6f5c22b1-52a0-4f34-878a-ba54688a46ad
User-Agent: VSServices/16.196.31911.0, (NetStandard; Microsoft Windows 10.0.19043), VstsAgentCore-win-x64/2.999.999, (Microsoft Windows 10.0.19043)
[Certificate]
Effective date: 11/10/2021 6:59:48 PM
Expiration date: 11/10/2022 6:59:48 PM
Issuer: CN=Microsoft RSA TLS CA 02, O=Microsoft Corporation, C=US
Subject: CN=visualstudio.com
[2021-11-16 10:30:48Z INFO AgentServer] Diagnostic data for request:
[SSL Policy Errors]
RemoteCertificateNotAvailable: Could not resolve related error message
RemoteCertificateNameMismatch: Certificate name mismatch
RemoteCertificateChainErrors: ChainStatus has returned a non empty array
[HttpRequest]
Requested URI: https://v-alsmo.visualstudio.com/_apis/distributedtask/pools/1/sessions/8788a522-8f1f-4243-bc3a-d993a7f15e48
Request method: DELETE
[HttpRequestHeaders]
X-TFS-Session: 4641a21d-c316-4ddb-a340-8b5322844033
X-VSS-E2EID: 58c19023-50b5-4fe3-9c50-a654e02d1439
User-Agent: VSServices/16.196.31911.0, (NetStandard; Microsoft Windows 10.0.19043), VstsAgentCore-win-x64/2.999.999, (Microsoft Windows 10.0.19043)
[Certificate]
Effective date: 11/10/2021 6:59:48 PM
Expiration date: 11/10/2022 6:59:48 PM
Issuer: CN=Microsoft RSA TLS CA 02, O=Microsoft Corporation, C=US
Subject: CN=visualstudio.com

Documentation changes required: No

Added unit tests: No

Attached related issue:

Checklist:

  • Checked that applied changes work as expected

- Fixed diagnostic message
- Changed location of the `ServerUtil` class methods to align with the project
@alexander-smolyakov alexander-smolyakov changed the title [WIP] Improve error messaging for SSL related errors Improve error messaging for SSL related errors Oct 19, 2021
@alexander-smolyakov alexander-smolyakov marked this pull request as ready for review October 19, 2021 11:14
Copy link
Contributor

@EzzhevNikita EzzhevNikita left a comment

Choose a reason for hiding this comment

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

LGTM! Also tested that this change doesn't break an agent

@alexander-smolyakov alexander-smolyakov requested a review from a team October 22, 2021 12:48
@anatolybolshakov
Copy link
Contributor

A least the following places contain VssConnection creation, but not updated yet:

  • src\Agent.Plugins\Artifact\FileContainerProvider.cs (line 40)
  • src\Agent.Sdk\CommandPlugin.cs (line 153)
  • src\Agent.Sdk\LogPlugin.cs (line 205)

Could you please check? Making trace mandatory for VssUtil.CreateConnection will help to find other places as well I guess.

@anatolybolshakov
Copy link
Contributor

There are 2 places for which trace is still not passed - could you please check?
src\Agent.Sdk\LogPlugin.cs:205
src\Agent.Worker\WorkerUtilties.cs:30

@anatolybolshakov
Copy link
Contributor

The following VssConnections constructor call still not adjusted - so there won't be SSL errors tracing for them:
src\Agent.Plugins\Artifact\FileContainerProvider.cs

@alexander-smolyakov
Copy link
Contributor Author

src\Agent.Sdk\LogPlugin.cs:205
src\Agent.Plugins\Artifact\FileContainerProvider.cs
src\Agent.Worker\Build\FileContainerServer.cs

Per offline discussion, we have decided to postpone enabling SSL error tracing for these places.

Copy link
Contributor

@anatolybolshakov anatolybolshakov left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@alexander-smolyakov alexander-smolyakov merged commit 1adfc03 into master Nov 19, 2021
@alexander-smolyakov alexander-smolyakov deleted the users/alexander-smolyakov/issue1623_improve_error_messaging_for_ssl_errors branch November 19, 2021 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants