MAISTRA-2124: Ssl: avoid tracking "unknown" ciphers

[oschaaf]

Track ssl.ciphers.xxx counters by cipher name instead of ssl.ciphers.unknown where possible.
Inspired by envoyproxy/envoy#14534

Jira issue: https://issues.redhat.com/browse/MAISTRA-2124

Signed-off-by: Otto van der Schaaf ovanders@redhat.com

[dmitri-d]

This looks good, tyvm @oschaaf. There's a failure in a fuzz test (that I thought I disabled), please ignore it. The other failure is in SslSocketTest.CipherUsageGetsCounted test: C++ exception with description "Failed to initialize cipher suites AEAD-AES128-GCM-SHA256. The following ciphers were rejected when tried individually: AEAD-AES128-GCM-SHA256" thrown in the test body.

[dmitri-d]

@oschaaf: could you also create a ticket (here: https://issues.redhat.com/projects/MAISTRA/issues) and update the commit message to include it?

[oschaaf]

Thanks. I'm stripping out "AEAD-AES128-GCM-SHA256" and am trying to find out if there's others that would cause similar failures, by running that test via docker with the build image. I'll push an amendment once I get that to pass.

[oschaaf]

@dmitri-d

• Merged the latest into this branch, which I hope makes tests pass.
• Filed https://issues.redhat.com/browse/MAISTRA-2124
• Updated the PR description to reference that Jira issue. There are multiple commits, do we squash commits here upon merge?

[jwendell]

@oschaaf Is this an openssl specific thing? Or it applies to upstream as well?

[oschaaf]

@oschaaf Is this an openssl specific thing? Or it applies to upstream as well?

The test is specific to pass for the openssl we use; the change itself is on par with what upstream has today on master.

[jwendell]

Is this critical to have in our fork? Or it;s just safe to wait for the next version, when we will grab the next Envoy version?

[oschaaf]

Is this critical to have in our fork? Or it;s just safe to wait for the next version, when we will grab the next Envoy version?

Well, the change allows one to observe which ciphers effectively get used when looking at the stats. I wouldn't call it critical to operation, but it there are times when that is useful.

[dmitri-d]

@jwendell: I'm ok with getting this merged.