StackExchange / StackExchange/StackExchange.Redis

CheckCertificateRevocation is ignored unless SslProtocols is also specified, causing revocations to not be checked

Open
#1,705 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

⚙️ area:connection ⚙️ area:ssl
Dominant language
C#
Stars
6.2k
Forks
1.6k
Avg merge
1d 15h
Merged PRs (30d)
43

Description

As the title states, ConfigurationOptions.CheckCertificateRevocation is ignored unless ConfigurationOptions.SslProtocols is specified, and the resulting behavior is that the revocation check is always skipped. This seems like unexpected behavior, especially given that the default value for ConfigurationOptions.CheckCertificateRevocation is true. At the very least it is inconsistent behavior that can cause surprising bugs to occur when the an application changes to specifying the TLS version.

The fault lies in this block of code:
https://github.com/StackExchange/StackExchange.Redis/blob/40595caf2a08ecf86ea2cfea7ea0d070d07ffb16/src/StackExchange.Redis/ExtensionMethods.cs#L151-L167

In line 166 the overload used does not actually check for certificate revocation. Unfortunately the next best overload does not exist pre 4.7, and the default for SslProtocols changed between 4.6 and 4.7. That makes this not a straightforward change, otherwise this would be a pull request rather than an issue.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/StackExchange.Redis/ExtensionMethods.cs at lines 151-167 and compare the referenced SslStream.AuthenticateAsClient overloads with the supported framework versions. Verify how CheckCertificateRevocation behaves when SslProtocols is omitted or specified, and define completion as revocation checks no longer being silently skipped while preserving the documented protocol defaults across those versions.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
networking, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.