feat: add ocsp stapling support to mqtt ssl listener (5.0) #10128
Conversation
Needed for OCSP / CRL tests because of a bug that makes emqtt hang forever on TLS handshake errors.
Co-authored-by: ieQu1 <99872536+ieQu1@users.noreply.github.com>
Co-authored-by: Zaiming (Stone) Shi <zmstone@gmail.com>
For some yet unknown reason the old test version using `open_port` does not work in OTP 25, but works fine in OTP 24. There are no messages at all received from The openssl client port program in OTP 25.
Pull Request Test Coverage Report for Build 4419358589
💛 - Coveralls |
Co-authored-by: William Yang <mscame@gmail.com>
|
would be good if we could have PR template? |
Ok, I'll get it and put it in the description. I has been some time since the templates don't show up automatically anymore. 🤔 Maybe we could reconfigure them to be automatic again? |
Sometimes this test might retry more times, so we check the prefix of the trace only.
I couldn't find a way to let GitHub support templates for different target branches.
|
Why do we need different templates for different target branches? What is the use case? Right now we have
We are not going to keep v4 and v5 in sync, may be we can just have one standard template here? |
yeah, I agree to merge them into one and manually strip the parts that are irrelevant. |
Co-authored-by: Zaiming (Stone) Shi <zmstone@gmail.com>
| [ | ||
| {"enable_ocsp_stapling", | ||
| sc( | ||
| boolean(), |
There was a problem hiding this comment.
there had been some discussion about abusing enable flag in many places.
is it really necessary for ocsp ?
e.g. if the parent field is set to null (undefined from HOCON map), doesn't that serve as disable ?
There was a problem hiding this comment.
This struct is always present because of the fields with default values, right?
We could interpret the absence of responder_url as a sign that it's disabled, but in my opinion being explicit with an enable flag is more clear than it being implicitly disabled by the absence of a certain field. 🤔
If we want to avoid using explicit configs, then we can use the absence of that field to disable it, yes.
https://emqx.atlassian.net/browse/EMQX-8816
Spin off of #10067
Since there were some fundamental changes requested to the CRL part of that PR, I've split the OCSP stapling bits for separate review to uncouple the merge.
PR Checklist
Please convert it to a draft if any of the following conditions are not met. Reviewers may skip over until all the items are checked:
changes/{ce,ee}/(feat|perf|fix)-<PR-id>.en.mdand.zh.mdfiles