tlshd: skip reverse DNS resolution for PSK handshakes

[Dwyane-Yan]

ISSUE

tlshd_genl_valid_handler() resolves the peer address into a hostname
via getnameinfo(NI_NAMEREQD) on every accepted handshake request,
unless the kernel supplied a peer name (which only sunrpc does today).

That hostname is consumed exclusively by X.509 hostname verification
(SNI + certificate SAN/CN check). PSK handshakes, e.g. NVMe-TLS, never
read it (yet they pay for the lookup on every queue).

The cost is not theoretical. Inside a network namespace resolv.conf
commonly points at a resolver that cannot be reached from that netns,
so the UDP query disappears and getnameinfo() blocks for the full
glibc retry budget (default timeout:5 x attempts:2 ≈ 10s).

Address families without a local NSS shortcut always take this path --
notably v4-mapped AF_INET6 addresses: the myhostname module answers
reverse lookups for the plain-v4/v6 default gateway but not for the
mapped form, so the query falls through to DNS.

Benifits

Each NVMe/TLS queue setup stalls ~10s before the TLS handshake even
starts. The stall races the kernel's own 10s TLS handshake timeout
(nvme_tcp tls_handshake_timeout), producing spurious connect
failures:

nvme nvme0: queue 0: TLS handshake failed, error -110   (kernel wins the race)
nvme nvme0: queue 0: TLS handshake complete, error -13  (gnutls wins: EACCES)

Measured on v4-mapped NVMe/TCP (2 netns + veth, no reachable resolver):
three-way handshake completes, then ClientHello appears on the wire
10.04s later (exactly timeout:5 x attempts:2). With the patch, the
same setup hands over in milliseconds.

Reported-by: Geliang Tang geliang@kernel.org
Tested-by: Geliang Tang geliang@kernel.org
Signed-off-by: Gang Yan yangang@kylinos.cn

[chucklever]

Thanks for the patch. The premise holds: nothing on the PSK path
reads parms->peername. Four things before I can take it.

1. tlshd_tls13_client_anon_handshake() strlen()s
   parms->peername at client.c:125 with no NULL check, and
   this change stops supplying one for UNAUTH. No in-tree caller
   trips it today, since SUNRPC is the only user of
   tls_client_hello_anon() and it always passes a servername.
   But th_peername is optional at net/handshake/tlshd.c:221,
   so tlshd cannot rely on it. Test auth_mode != HANDSHAKE_AUTH_PSK instead, or guard client.c:125.

2. Predates your patch, same defect, so please fix it in the
   same series: the NL_STOP at netlink.c:657 does not abort
   the request. libnl turns it into err = 0, and
   tlshd_genl_get_handshake_parms() returns success with a
   NULL peername. client.c:446 crashes on it when the
   reverse lookup returns NXDOMAIN.

3. log.c:81 reads peername as well, so PSK handshakes stop
   naming their peer in syslog. Fall back to peeraddr. That
   consumer also contradicts "consumed exclusively by X.509
   hostname verification" in the commit message.

4. The commit message reads as an optimization. The failure in
   your PR description is the reason to take the patch. Put the
   unreachable resolver, the timeout:5 x attempts:2 stall,
   the race with nvme_tcp's tls_handshake_timeout, and the two
   error lines into the commit log. Keep the v4-mapped
   AF_INET6 detail.

Leaving the lookup in place for SERVERHELLO is right. server.c
never reads peername, but log.c does.

[Dwyane-Yan]

Hi, @chucklever

Thanks for your review.

I have fixed the points 1,3,4 you mentioned in the first commit.

Predates your patch, same defect, so please fix it in the
same series: the NL_STOP at netlink.c:657 does not abort
the request. libnl turns it into err = 0, and
tlshd_genl_get_handshake_parms() returns success with a
NULL peername. client.c:446 crashes on it when the
reverse lookup returns NXDOMAIN.

But for the Second point, sorry to admit that I am not yet familiar with the tlshd internals. I just added a NULL check to avoid the crash and would like to confirm whether it meets your expectation.

Or do you mean that tlshd should abort the request right at the NL_STOP? I did look into aborting from the netlink callback itself, but as far as I know, a new field should be added to 'struct tlshd_handshake_parms' for recording the 'errno'. That seemed like a bigger change, and I was not sure it was what you wanted, so I went with the NULL check first.

Thanks
Gang

[chucklever]

Predates your patch, same defect, so please fix it in the
same series: the NL_STOP at netlink.c:657 does not abort
the request. libnl turns it into err = 0, and
tlshd_genl_get_handshake_parms() returns success with a
NULL peername. client.c:446 crashes on it when the
reverse lookup returns NXDOMAIN.

But for the Second point, sorry to admit that I am not yet familiar with the tlshd internals. I just added a NULL check to avoid the crash and would like to confirm whether it meets your expectation.

Or do you mean that tlshd should abort the request right at the NL_STOP?

Yes, that is what I meant. However it's already done! I spent a few minutes to fix it up yesterday afternoon after writing the review comment above.

Rebase #159 on current main and drop commit 2. Commit 1 should then apply as a one-line netlink.c change plus the log.c fallback.

[Dwyane-Yan]

Thanks for your work. Please review the latest commit.

Cherrs,
Gang