Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto/tls: same x509 certificate in different tls connections costs too much memory #46035

Closed
nejisama opened this issue May 7, 2021 · 18 comments · May be fixed by #46036
Closed

crypto/tls: same x509 certificate in different tls connections costs too much memory #46035

nejisama opened this issue May 7, 2021 · 18 comments · May be fixed by #46036
Assignees
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@nejisama
Copy link

nejisama commented May 7, 2021

What version of Go are you using (go version)?

$ go version
go version go1.16.3 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

What did you do?

In our production environment, we use tls to communicate between applications. A client application will access dozens to thousands of server applications, and server applications will have dozens to thousands of machines.
Each machine will maintain one or two long TCP connections for communication. A client may have tens of thousands of tls connections, and each connection will save a copy of the server's x509 certificate information, resulting in a large amount of memory usage.
For the same server application, the certificates held are the same. The actual number of different certificates is much lower than the number of connections, but the same size of memory is still allocated for each connection in the memory.
For the mtls(Mutual TLS) scenario, the server's tls connection may also save the client's certificate, which will cause the same memory consumption, and the number of client certificates is also much lower than the number of client connections.

What did you expect to see?

The same x509 certificate is reused through caching, and memory is no longer calculated and allocated separately for each connection, which reduces the memory usage.

What did you see instead?

Each connection will parse and save the x509 certificate information, which takes up a lot of memory.

@gopherbot
Copy link

gopherbot commented May 7, 2021

Change https://golang.org/cl/317931 mentions this issue: crypto/tls: use a cache to save x509 certificate in the tls connection

@slrz
Copy link

slrz commented May 7, 2021

Aren't those parsed certificates also passed to user-provided code? Like with tls.Config's VerifyPeerCertificate, for example? What happens when that code mutates the cached certificates?

@andrius4669
Copy link
Contributor

andrius4669 commented May 7, 2021

Not to mention that they're never deleted...

@nejisama
Copy link
Author

nejisama commented May 8, 2021

Aren't those parsed certificates also passed to user-provided code? Like with tls.Config's VerifyPeerCertificate, for example? What happens when that code mutates the cached certificates?

The x509.Certificate is stateless,I think if the cached certificate is also used elsewhere, it will have no effect.

@nejisama
Copy link
Author

nejisama commented May 8, 2021

Aren't those parsed certificates also passed to user-provided code? Like with tls.Config's VerifyPeerCertificate, for example? What happens when that code mutates the cached certificates?

If user-provided code also wanna parse certificate with cache, needs add an export function, but i do not sure if it is necessary

@nejisama
Copy link
Author

nejisama commented May 8, 2021

Not to mention that they're never deleted...

Yes, the current cache does not provide a delete/expiration plan. Maybe set a maximum cache size is a simple way to control the cache memory use, but it is difficult to determine what this value is

@nejisama
Copy link
Author

nejisama commented May 10, 2021

Not to mention that they're never deleted...

How about add an expire time to the cache. The cache keeps the time that cache was cleaned, Whenever the cache needs to be updated, it is judged whether the expiration time is reached. If it is, the previous cache is cleared.

just like:

type Cache struct {
  lastCleaned time.Time 
}

const expire time.Duration = xxxxx // set a expire 

func (c *Cache) Update() {
  now := time.Now()
  if now.Sub(expire) >= c.lastCleaned {
     // clean the cache
  }
}

@nejisama
Copy link
Author

nejisama commented May 10, 2021

Aren't those parsed certificates also passed to user-provided code? Like with tls.Config's VerifyPeerCertificate, for example? What happens when that code mutates the cached certificates?

A way to not increase the export function is to directly put the cache in the x509.ParseCertificate?

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 10, 2021
@cagedmantis cagedmantis added this to the Backlog milestone May 10, 2021
@cagedmantis
Copy link
Contributor

cagedmantis commented May 10, 2021

/cc @FiloSottile

@taoyuanyuan
Copy link
Contributor

taoyuanyuan commented May 11, 2021

type ipv6ZoneCache struct {

ipv6ZoneCache has an expire time to the cache too.

@taoyuanyuan
Copy link
Contributor

taoyuanyuan commented May 21, 2021

ping @rolandshoemaker @FiloSottile

@nejisama
Copy link
Author

nejisama commented May 31, 2021

ping @FiloSottile

@nejisama
Copy link
Author

nejisama commented Jul 5, 2021

hello, any suggestions for this issue/ pr ? @FiloSottile @cagedmantis

@xdg
Copy link

xdg commented Dec 10, 2021

I'm also interested in this, as we've also seen it in production.

I'd prefer an approach that adds an x509 cache field to the tls.Config struct (defaulting to nil) along with an associated interface. Then users can opt into a cache, taking responsibility for the integrity of the data within and for designing their own eviction strategies.

@nejisama
Copy link
Author

nejisama commented Aug 25, 2022

we found boring ssl have certificate cache too, see https://boringssl.googlesource.com/boringssl/+/HEAD/PORTING.md#crypto_buffer

With the standard OpenSSL APIs, when making many TLS connections, the certificate data for each connection is retained in memory in an expensive X509 structure. Additionally, common certificates often appear in the chains for multiple connections and are needlessly duplicated in memory.

@gopherbot
Copy link

gopherbot commented Aug 29, 2022

Change https://go.dev/cl/426455 mentions this issue: crypto/tls: use certificate cache in client

@gopherbot
Copy link

gopherbot commented Aug 29, 2022

Change https://go.dev/cl/426454 mentions this issue: crypto/tls: add a certificate cache implementation

@rolandshoemaker rolandshoemaker self-assigned this Oct 3, 2022
gopherbot pushed a commit that referenced this issue Nov 7, 2022
Adds a BoringSSL CRYPTO_BUFFER_POOL style reference counted intern
table for x509.Certificates. This can be used to significantly reduce
the amount of memory used by TLS clients when certificates are reused
across connections.

Updates #46035

Change-Id: I8d7af3bc659a93c5d524990d14e5254212ae70f4
Reviewed-on: https://go-review.googlesource.com/c/go/+/426454
Run-TryBot: Roland Shoemaker <roland@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
@taoyuanyuan

This comment was marked as disruptive content.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants