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.

Sign up for GitHub

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

Jump to bottom

List of SSL cipher stat names is incomplete #14524

Closed
akonradi opened this issue Dec 28, 2020 · 1 comment · Fixed by #14534
Closed

List of SSL cipher stat names is incomplete #14524

akonradi opened this issue Dec 28, 2020 · 1 comment · Fixed by #14534
Assignees
@jmarantz
Labels
area/stats area/tls help wanted Needs help!

Comments

@akonradi
Copy link
Contributor

The list of cipher names and curves supported by BoringSSL is larger than the set that Envoy exports stats for ("cluster.[cluster_name].ssl.ciphers.[cipher]" and ("cluster.[cluster_name].ssl.curves.[cipher]"). The list in Envoy is currently hard-coded and separate from the list of actual supported stats.

/cc @jmarantz @PiotrSikora
@akonradi akonradi added the triage Issue requires triage label Dec 28, 2020
@jmarantz jmarantz self-assigned this Dec 28, 2020
@jmarantz
Copy link
Contributor

It's easy enough to use the configured ciphers; should've done that in the first place. Moreover we should probably log (rate-limited) when we see one we didn't expect, otherwise you can't learn from production what it was.

@mattklein123 mattklein123 added area/stats area/tls help wanted Needs help! and removed triage Issue requires triage labels Dec 28, 2020
@jmarantz jmarantz mentioned this issue Dec 29, 2020
Merged
jmarantz added a commit that referenced this issue Jan 6, 2021
@jmarantz
ssl: remember stat names for configured ciphers. (#14534)
568e6d6 
Commit Message: Stats are only kept for a set of known SSL ciphers, to bound memory use. That set was previously determined by running unit tests and capturing which ciphers were referenced. This PR changes it to use the configured ciphers.
Additional Description:
Risk Level: medium
Testing: //test/...
Docs Changes: n/a
Release Notes: n/a
Fixes: #14524

Signed-off-by: Joshua Marantz <jmarantz@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jmarantz jmarantz

Labels
area/stats area/tls help wanted Needs help!
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ssl: remember stat names for configured ciphers. jmarantz/envoy
3 participants
@jmarantz @akonradi @mattklein123