feat: add support for wildcard on SNIs for SSL#12668
Conversation
dosubot
Bot
added
size:L
| if s ~= "*" then | ||
| j = j + 1 | ||
| sni[j] = s:reverse() | ||
| end |
There was a problem hiding this comment.
It is reasonable not to perform additional processing, as it does not cause any issues.
This is not necessary, as ("*"):reverse() functions correctly. Even if this extra check were added, it would only apply in extremely rare cases and would require explanation for maintainability. I do not believe it differs from normal sni case.
| if ssl.value.sni ~= "*" then | ||
| sni = ssl.value.sni:reverse() | ||
| end |
| handler = function (api_ctx) | ||
| if not api_ctx then | ||
| return | ||
| if sni and (type(sni) == "table" and #sni > 0 or type(sni) == "string") then |
There was a problem hiding this comment.
This seems to cover all existing scenarios. Is this necessary?
| local sni_rev = sni:reverse() | ||
| local ok = radixtree_router:dispatch(sni_rev, nil, api_ctx) | ||
|
|
||
| -- if no SSL matched, try to find a wildcard SSL |
There was a problem hiding this comment.
Can't radix trees perform wildcard matching directly? Why do wildcard matches work fine in our HTTP routes (/test/*) and SSL SNI (*.example.com)?
In my view, there's no fundamental difference between *.example.com and *. If the former works correctly after reverse sort, the latter should also function out of the box. We shouldn't need to add any special logic for this.
There was a problem hiding this comment.
I saw your reply at #12668 (comment), but it doesn't resolve this concern. We should investigate the root cause.
| for _, s in ipairs(ssl.value.snis) do | ||
| j = j + 1 | ||
| sni[j] = s:reverse() | ||
| if s ~= "*" then |
There was a problem hiding this comment.
to me, I think we can keep old code, it seems work fine too? all right?
There was a problem hiding this comment.
The main point of contention lies in #12668 (comment), which determines whether we must handle the * case separately.
There was a problem hiding this comment.
Okay i was wrong. Due to an initial mistake in my code I got that failure and falsely assumed it was because radixtree couldn't handle it. I have modified the PR. Apologies for mistake
4 participants
The current APISIX SSL SNI only allows exact domain name matching "abc.test.com" and partial wildcard ".test.com". It does not support the complete wildcard value of "" for SNI, which does not meet the core consistency test requirements of HTTPSListener in the Gateway API. This situation needs to be supported.
Checklist