Skip to content

Remove process-wide ServicePointManager.SecurityProtocol override #278

@barnon-apiiro

Description

@barnon-apiiro

Problem

In GitLabHttpFacade.Setup(), the library sets:

ServicePointManager.SecurityProtocol |= SecurityProtocolType.Tls11 | SecurityProtocolType.Tls12;

This has several issues:

1. Mutates process-wide global state

ServicePointManager.SecurityProtocol is a static property that affects all HTTP connections in the host process, not just those made by this library. A NuGet package should not modify global state that impacts other libraries and application code.

2. Prevents TLS 1.3 negotiation

The |= operator adds TLS 1.1 and 1.2 to the default, but never adds TLS 1.3. On .NET Framework where the default is Ssl3 | Tls, the resulting value becomes Ssl3 | Tls | Tls11 | Tls12 — TLS 1.3 is excluded. This is a problem for servers that require TLS 1.3.

On .NET Core/.NET 5+ the default is SystemDefault (value 0), so the |= is effectively a no-op, but it's still bad practice.

3. Adds deprecated TLS 1.1

TLS 1.1 has been deprecated since RFC 8996 (March 2021). Major browsers and services have dropped support for it.

4. Runs on every instantiation

The line executes each time a GitLabClient is constructed, repeatedly mutating global state.

Context

This line was likely added to support older .NET Framework versions (4.5–4.6.2) where SecurityProtocol defaults to Ssl3 | Tls and TLS 1.2 isn't enabled. However:

  • .NET Framework 4.7+ defaults to TLS 1.2 via SystemDefault
  • .NET Framework 4.6.1 (the minimum netstandard2.0-compatible framework) reached end of life in April 2022
  • The library also explicitly targets net48, which doesn't need this

Possible solutions

Option A: Remove the line entirely (recommended)

Remove the ServicePointManager.SecurityProtocol line and the unused System.Net using directive. Per Microsoft's guidance, the application — not individual libraries — should be responsible for configuring SecurityProtocol. Consumers on very old .NET Framework versions would need to configure this themselves, which is already standard practice.

Option B: Use HttpClientHandler.SslProtocols (per-connection)

Replace the global setting with SslProtocols on the HttpClientHandler, which only affects this library's connections. However, this property isn't consistently available across all netstandard2.0 implementations, and would require changes to the constructor logic.

Option C: Conditional compilation

Use #if directives to only apply the setting on .NET Framework targets, and skip it on .NET Core/.NET 5+ where SystemDefault handles it. This reduces the blast radius but still mutates global state on Framework.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions