Fix host name verification for PerfdataWriterConnection#10799
Conversation
This was omitted by accident from the original PR, despite being done in the original perfdata writer connection code. Without setting this parameter, host name verification will be disabled, which poses a security risk.
8691b83 to
f4127d2
Compare
c06009c to
6172edb
Compare
6172edb to
aa3a0c5
Compare
yhabteab
left a comment
There was a problem hiding this comment.
I don't know what's going on with LibreSSL either, so no idea what else to do.
aa3a0c5 to
f5be692
Compare
c5c1123 to
aab0b64
Compare
julianbrost
left a comment
There was a problem hiding this comment.
The implementation change itself is fine.
Regarding the tests, correct me if I'm wrong: there was a new test case previously added in this PR which triggered some race condition (in other test cases) with high probability the ARM-emulated Docker builds (to an extent that retry didn't help). Thus, include the fix in the 2.16.0 (so that we don't have a security regression), but delay the addition of the test case itself past that release.
|
I don't really know what's going on with this PR today but what makes you think the tests run fine now? The docker arm build still hangs. |
|
I've cherry-picked a commit from #10780 because I noticed that it's only the Edit: Ignore the back and forth on the |
8a1fcc0 to
88dc49d
Compare
88dc49d to
7ee3386
Compare
Fixes the host name verification for
PerfdataWriterConnectionby correctly appending the optionalhostparameter when constructing theAsioTlsStream. This was omitted from the original PR #10668 by accident, despite the writers correctly passing the parameter before.Testing
Given that the writers worked with this check in place before #10668, and
PerfdataWriterConnectionotherwise was verified in that PR, I'll not retest all TLS-capable backends this time.Also, since the
PerfdataWriterConnectionunit-tests failed after this change and the fixtures used needed two small adjustments to work again, that verifies both that the check does fail if the host names don't match up and succeeds when they are made to match up again.I've also added an additional test-case that ensures the verification fails when the host name doesn't match.The test case will be moved to a separate PR linking to this one at a later point.Fixes #10797.