Skip to content

Add documentation for missing ssl bits#7136

Merged
aik-jahoda merged 5 commits into
dotnet:net6-rc1from
aik-jahoda:jajahoda/ssl
Sep 13, 2021
Merged

Add documentation for missing ssl bits#7136
aik-jahoda merged 5 commits into
dotnet:net6-rc1from
aik-jahoda:jajahoda/ssl

Conversation

@aik-jahoda

Copy link
Copy Markdown
Contributor

Summary

M:System.Net.Security.SslCertificateTrust.CreateForX509Collection(System.Security.Cryptography.X509Certificates.X509Certificate2Collection,System.Boolean)
M:System.Net.Security.SslCertificateTrust.CreateForX509Store(System.Security.Cryptography.X509Certificates.X509Store,System.Boolean)
M:System.Net.Security.SslStream.NegotiateClientCertificateAsync(System.Threading.CancellationToken)
M:System.Net.Security.SslStreamCertificateContext.Create(System.Security.Cryptography.X509Certificates.X509Certificate2,System.Security.Cryptography.X509Certificates.X509Certificate2Collection,System.Boolean,System.Net.Security.SslCertificateTrust)

part of dotnet/runtime#54857

@opbld32

opbld32 commented Sep 10, 2021

Copy link
Copy Markdown

Docs Build status updates of commit 4378c4a:

❌ Validation status: errors

Please follow instructions here which may help to resolve issue.

File Status Preview URL Details
xml/System.Net.Security/SslStreamCertificateContext.xml ❌Error Details
❌Error Details

xml/System.Net.Security/SslStreamCertificateContext.xml

  • Line 0, Column 0: [Error-ECMA2Yaml_InternalError]
Intenal Several Error: System.Xml.XmlException: The 'exception' start tag on line 122 position 10 does not match the end tag of 'Docs'. Line 123, position 9.
   at System.Xml.XmlTextReaderImpl.Throw(Exception e)
   at System.Xml.XmlTextReaderImpl.Throw(String res, String[] args)
   at System.Xml.XmlTextReaderImpl.ThrowTagMismatch(NodeData startTag)
   at System.Xml.XmlTextReaderImpl.ParseEndElement()
   at System.Xml.XmlTextReaderImpl.ParseElementContent()
   at System.Xml.Linq.XContainer.ReadContentFrom(XmlReader r)
   at System.Xml.Linq.XDocument.Load(XmlReader reader, LoadOptions options)
   at System.Xml.Linq.XDocument.Parse(String text, LoadOptions options)
   at ECMA2Yaml.ECMALoader.LoadType(FileItem typeFile)
   at ECMA2Yaml.ECMALoader.LoadTypes(String basePath, Namespace ns)

  • Line 0, Column 0: [Error-ECMA2Yaml_File_LoadFailed] Failed to load 1 files, aborting...

For more details, please refer to the build report.

If you see build warnings/errors with permission issues, it might be due to single sign-on (SSO) enabled on Microsoft's GitHub organizations. Please follow instructions here to re-authorize your GitHub account to Docs Build.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

Note: Your PR may contain errors or warnings unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.

For any questions, please:

<Docs>
<param name="trustList">To be added.</param>
<param name="trustList">The store containig the trusted certificates.</param>
<param name="sendTrustInHandshake">To be added.</param>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@wfurt can you please provide description for this param?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If the is set to true server will send list of trusted CAs during TLS Handshake. This is used to guide client to select appropriate client certificate.

Now, this has many caveats and we should put some big warnings about it: 1. In 6,0 it works only on Windows - we cut out macOS and Linux. 2) on Windows it depends on registry setting.

Since this increases size of the handshake message and can be viewed as info leak about system configuration, this should be off unless there is specific reason. e.g. it would be nice to have some warning like "don't use it unless you know what you are doing".

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@wfurt would you mind providing these comments as suggestions? Both for the empty <param> and for the <remarks> section below with all those caveats you mentioned.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I already did that in my review.

Comment thread xml/System.Net.Security/SslCertificateTrust.xml Outdated
<summary>To be added.</summary>
<returns>To be added.</returns>
<summary>Creates new SslCertificateTrust.</summary>
<returns>A collection of certificates replacing the default certificate trust.</returns>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It doesn't return a collection of certificates, it returns the trust policy thing.

@carlossanlop carlossanlop Sep 10, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yep, it returns an object of this file's Type. So in that case, the <returns> value should start similarl to the suggestion you provided here, right?:
https://github.com/dotnet/dotnet-api-docs/pull/7136/files#r706271785

Comment thread xml/System.Net.Security/SslCertificateTrust.xml Outdated
Comment thread xml/System.Net.Security/SslStreamCertificateContext.xml Outdated
Comment thread xml/System.Net.Security/SslCertificateTrust.xml Outdated
@opbld30

opbld30 commented Sep 10, 2021

Copy link
Copy Markdown

Docs Build status updates of commit 0a968a0:

✅ Validation status: passed

File Status Preview URL Details
xml/System.Net.Security/SslCertificateTrust.xml ✅Succeeded View
xml/System.Net.Security/SslStream.xml ✅Succeeded View
xml/System.Net.Security/SslStreamCertificateContext.xml ✅Succeeded View

For more details, please refer to the build report.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

For any questions, please:

Comment thread xml/System.Net.Security/SslCertificateTrust.xml Outdated
Comment thread xml/System.Net.Security/SslCertificateTrust.xml Outdated
Comment thread xml/System.Net.Security/SslCertificateTrust.xml Outdated
Comment thread xml/System.Net.Security/SslStream.xml Outdated
Comment thread xml/System.Net.Security/SslStream.xml Outdated
aik-jahoda and others added 2 commits September 13, 2021 06:06
Co-authored-by: Jeremy Barton <jbarton@microsoft.com>
Co-authored-by: Genevieve Warren <24882762+gewarren@users.noreply.github.com>
Co-authored-by: Genevieve Warren <24882762+gewarren@users.noreply.github.com>
@opbld31

opbld31 commented Sep 13, 2021

Copy link
Copy Markdown

Docs Build status updates of commit c097ec7:

✅ Validation status: passed

File Status Preview URL Details
xml/System.Net.Security/SslCertificateTrust.xml ✅Succeeded View
xml/System.Net.Security/SslStream.xml ✅Succeeded View
xml/System.Net.Security/SslStreamCertificateContext.xml ✅Succeeded View

For more details, please refer to the build report.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

For any questions, please:

@opbld31

opbld31 commented Sep 13, 2021

Copy link
Copy Markdown

Docs Build status updates of commit 0c636e3:

✅ Validation status: passed

File Status Preview URL Details
xml/System.Net.Security/SslCertificateTrust.xml ✅Succeeded View
xml/System.Net.Security/SslStream.xml ✅Succeeded View
xml/System.Net.Security/SslStreamCertificateContext.xml ✅Succeeded View

For more details, please refer to the build report.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

For any questions, please:

@opbld30

opbld30 commented Sep 13, 2021

Copy link
Copy Markdown

Docs Build status updates of commit 0c636e3:

✅ Validation status: passed

File Status Preview URL Details
xml/System.Net.Security/SslCertificateTrust.xml ✅Succeeded View
xml/System.Net.Security/SslStream.xml ✅Succeeded View
xml/System.Net.Security/SslStreamCertificateContext.xml ✅Succeeded View

For more details, please refer to the build report.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

For any questions, please:

@opbld31

opbld31 commented Sep 13, 2021

Copy link
Copy Markdown

Docs Build status updates of commit b805c07:

✅ Validation status: passed

File Status Preview URL Details
xml/System.Net.Security/SslCertificateTrust.xml ✅Succeeded View
xml/System.Net.Security/SslStream.xml ✅Succeeded View
xml/System.Net.Security/SslStreamCertificateContext.xml ✅Succeeded View

For more details, please refer to the build report.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

For any questions, please:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-Meta Concerns something that extends across runtime area boundaries, for example, IDisposable.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants