[breaking change] Add the ability to control minimum TLS version in SecurityContext

[brianquinlan]

Change Intent

Add a new property to SecurityContext to control the minimum TLS version like:

abstract interface class SecurityContext {
  ...
  /// The minimum TLS version to use when establishing a secure connection.
  ///
  /// If the value is changed, it will only affect new connections. Existing
  /// connections will continue to use the protocol that was negotiated with the
  /// peer.
  abstract TlsProtocolVersion minimumTlsProtocolVersion;
};

Justification

Allows the developer to refuse TLS connections that aren't sufficiently secure.

See #54901

Impact

All classes that implements SecurityContext (without extends Mock or equivalent noSuchMethod implementation) will need to be updated.

A search on Github finds one such instance outside of the Dart SDK.

Mitigation

Developers implementing SecurityContext must add the minimumTlsProtocolVersion field.

Change Timeline

N/A

Associated CLs

API POC PR: https://dart-review.googlesource.com/c/sdk/+/365664

[mraleph]

Should we consider doing a slightly different breaking change to simplify further changes to this class?

Consider something like:

final class SecurityContextOption<T> {
  final String name;
  const SecurityContextOption._(this.name);

  static const minimumTlsProtocolVersion = SecurityContextOption<TlsProtocolVersion>('TlsProtocolVersion');
}

abstract interface class SecurityContext {
  void setOption<T>(SecurityContextOption<T> option, T value);
  T getOption<T>(SecurityContextOption<T> option);
}

It does look pretty ugly, but then we can add further configuration options without thinking about unfortunate souls who decided to implement SecurityContext for some reason.

I guess another option is to figure out why SecurityContext is marked as interface in the first place. It does not really seem like that is supposed to work to begin with given how closely this class is tied to internals.

[brianquinlan]

I guess another option is to figure out why SecurityContext is marked as interface in the first place. It does not really seem like that is supposed to work to begin with given how closely this class is tied to internals.

I changed it to an interface as part of the class modifier addition process following the general rule that everything that every abstract class that does not define any instance methods is logically an interface.

Changing it to abstract final class SecurityContext seems like a good idea to me. On Github, I only found 2 "implements SecurityContext" and those usages seem trivial.

Any thoughts @lrhn ?

[lrhn]

SGTM, ship it! 😁