Skip to content

Commit

Permalink
pkg/cri/(server|sbserver): criService.getTLSConfig() add TODO to veri…
Browse files Browse the repository at this point in the history
…fy nolint

This `//nolint`  was added in containerd/cri@f5c7ac9
to suppress warnings about the `NameToCertificate` function being deprecated:

    // Deprecated: NameToCertificate only allows associating a single certificate
    // with a given name. Leave that field nil to let the library select the first
    // compatible chain from Certificates.

Looking at that, it was deprecated in Go 1.14 through
golang/go@eb93c68
(https://go-review.googlesource.com/c/go/+/205059), which describes:

    crypto/tls: select only compatible chains from Certificates

    Now that we have a full implementation of the logic to check certificate
    compatibility, we can let applications just list multiple chains in
    Certificates (for example, an RSA and an ECDSA one) and choose the most
    appropriate automatically.

    NameToCertificate only maps each name to one chain, so simply deprecate
    it, and while at it simplify its implementation by not stripping
    trailing dots from the SNI (which is specified not to have any, see RFC
    6066, Section 3) and by not supporting multi-level wildcards, which are
    not a thing in the WebPKI (and in crypto/x509).

We should at least have a comment describing why we are ignoring this, but preferably
review whether we should still use it.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit d215725)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
  • Loading branch information
thaJeztah committed Mar 7, 2023
1 parent e66397d commit 9f0617e
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/cri/server/image_pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ func (c *criService) getTLSConfig(registryTLSConfig criconfig.TLSConfig) (*tls.C
if len(cert.Certificate) != 0 {
tlsConfig.Certificates = []tls.Certificate{cert}
}
tlsConfig.BuildNameToCertificate() //nolint:staticcheck
tlsConfig.BuildNameToCertificate() //nolint:staticcheck // TODO(thaJeztah): verify if we should ignore the deprecation; see https://github.com/containerd/containerd/pull/7349/files#r990644833
}

if registryTLSConfig.CAFile != "" {
Expand Down

0 comments on commit 9f0617e

Please sign in to comment.