Test results from v4

Test results from v4

[Hannes Reinecke]

Hi Chuck,

I've given v4 a spin on my testbed, and ... well, things could've been 
better :-(

Biggest issue is that I've lost connectivity to my testbed 
(openssl-based). It always kicks me back with:

SSL routines:tls_choose_sigalg:no suitable signature algorithm

for whatever reasons.
I try to fiddle with the priority string, but it's _really_ annoying.

And also reference counting is wrong:

unreferenced object 0xffff9989c22a0018 (size 24):
   comm "tlshd", pid 2332, jiffies 4294964098 (age 1513.848s)
   hex dump (first 24 bytes):
     00 00 00 00 00 00 00 00 a8 31 85 c1 89 99 ff ff  .........1......
     00 00 00 00 00 00 00 00                          ........
   backtrace:
     [<ffffffffa00ad3e4>] security_file_alloc+0x24/0x90
     [<ffffffff9ffbb747>] __alloc_file+0x57/0xc0
     [<ffffffff9ffbbbc3>] alloc_empty_file+0x43/0xf0
     [<ffffffff9ffbbcad>] alloc_file+0x2d/0x170
     [<ffffffff9ffbbea7>] alloc_file_pseudo+0xa7/0x100
     [<ffffffffa05158a9>] sock_alloc_file+0x39/0xa0
     [<ffffffffa073594b>] handshake_genl_cmd_accept+0x19b/0x2a0
     [<ffffffffa05d44e7>] genl_family_rcv_msg_doit.isra.19+0xf7/0x120
     [<ffffffffa05d495f>] genl_rcv_msg+0x19f/0x2a0
     [<ffffffffa05d3286>] netlink_rcv_skb+0x56/0x110
     [<ffffffffa05d3cc4>] genl_rcv+0x24/0x40
     [<ffffffffa05d277b>] netlink_unicast+0x1bb/0x290
     [<ffffffffa05d2bc5>] netlink_sendmsg+0x365/0x4d0
     [<ffffffffa0516e9f>] sock_sendmsg+0x5f/0x70
     [<ffffffffa0517211>] ____sys_sendmsg+0x201/0x280
     [<ffffffffa0519ca8>] ___sys_sendmsg+0x88/0xd0

But I guess you've found these already.

Cheers,

Hannes

Re: Test results from v4

[Chuck Lever III]

> On Feb 16, 2023, at 10:58 AM, Hannes Reinecke <hare@suse.de> wrote:
> 
> Hi Chuck,
> 
> I've given v4 a spin on my testbed, and ... well, things could've been better :-(
> 
> Biggest issue is that I've lost connectivity to my testbed (openssl-based). It always kicks me back with:
> 
> SSL routines:tls_choose_sigalg:no suitable signature algorithm
> 
> for whatever reasons.
> I try to fiddle with the priority string, but it's _really_ annoying.

Haven't seen that, could be a PSK-specific thing?


> And also reference counting is wrong:

That's not terribly surprising.


> unreferenced object 0xffff9989c22a0018 (size 24):
>  comm "tlshd", pid 2332, jiffies 4294964098 (age 1513.848s)
>  hex dump (first 24 bytes):
>    00 00 00 00 00 00 00 00 a8 31 85 c1 89 99 ff ff  .........1......
>    00 00 00 00 00 00 00 00                          ........
>  backtrace:
>    [<ffffffffa00ad3e4>] security_file_alloc+0x24/0x90
>    [<ffffffff9ffbb747>] __alloc_file+0x57/0xc0
>    [<ffffffff9ffbbbc3>] alloc_empty_file+0x43/0xf0
>    [<ffffffff9ffbbcad>] alloc_file+0x2d/0x170
>    [<ffffffff9ffbbea7>] alloc_file_pseudo+0xa7/0x100
>    [<ffffffffa05158a9>] sock_alloc_file+0x39/0xa0
>    [<ffffffffa073594b>] handshake_genl_cmd_accept+0x19b/0x2a0
>    [<ffffffffa05d44e7>] genl_family_rcv_msg_doit.isra.19+0xf7/0x120
>    [<ffffffffa05d495f>] genl_rcv_msg+0x19f/0x2a0
>    [<ffffffffa05d3286>] netlink_rcv_skb+0x56/0x110
>    [<ffffffffa05d3cc4>] genl_rcv+0x24/0x40
>    [<ffffffffa05d277b>] netlink_unicast+0x1bb/0x290
>    [<ffffffffa05d2bc5>] netlink_sendmsg+0x365/0x4d0
>    [<ffffffffa0516e9f>] sock_sendmsg+0x5f/0x70
>    [<ffffffffa0517211>] ____sys_sendmsg+0x201/0x280
>    [<ffffffffa0519ca8>] ___sys_sendmsg+0x88/0xd0
> 
> But I guess you've found these already.

Nope, I haven't seen that either, but I'm not looking for
memory leaks yet. My focus right now is to address netdev's
concerns, so I haven't enabled kmemleak (as it slows down
the dev loop). Not crashing is a higher priority than
leaking right at the moment.


--
Chuck Lever

Re: Test results from v4

[Chuck Lever III]

> On Feb 16, 2023, at 11:03 AM, Chuck Lever III <chuck.lever@oracle.com> wrote:
> 
> 
> 
>> On Feb 16, 2023, at 10:58 AM, Hannes Reinecke <hare@suse.de> wrote:
>> 
>> Hi Chuck,
>> 
>> I've given v4 a spin on my testbed, and ... well, things could've been better :-(
>> 
>> Biggest issue is that I've lost connectivity to my testbed (openssl-based). It always kicks me back with:
>> 
>> SSL routines:tls_choose_sigalg:no suitable signature algorithm
>> 
>> for whatever reasons.
>> I try to fiddle with the priority string, but it's _really_ annoying.
> 
> Haven't seen that, could be a PSK-specific thing?
> 
> 
>> And also reference counting is wrong:
> 
> That's not terribly surprising.
> 
> 
>> unreferenced object 0xffff9989c22a0018 (size 24):
>> comm "tlshd", pid 2332, jiffies 4294964098 (age 1513.848s)
>> hex dump (first 24 bytes):
>>   00 00 00 00 00 00 00 00 a8 31 85 c1 89 99 ff ff  .........1......
>>   00 00 00 00 00 00 00 00                          ........
>> backtrace:
>>   [<ffffffffa00ad3e4>] security_file_alloc+0x24/0x90
>>   [<ffffffff9ffbb747>] __alloc_file+0x57/0xc0
>>   [<ffffffff9ffbbbc3>] alloc_empty_file+0x43/0xf0
>>   [<ffffffff9ffbbcad>] alloc_file+0x2d/0x170
>>   [<ffffffff9ffbbea7>] alloc_file_pseudo+0xa7/0x100
>>   [<ffffffffa05158a9>] sock_alloc_file+0x39/0xa0
>>   [<ffffffffa073594b>] handshake_genl_cmd_accept+0x19b/0x2a0
>>   [<ffffffffa05d44e7>] genl_family_rcv_msg_doit.isra.19+0xf7/0x120
>>   [<ffffffffa05d495f>] genl_rcv_msg+0x19f/0x2a0
>>   [<ffffffffa05d3286>] netlink_rcv_skb+0x56/0x110
>>   [<ffffffffa05d3cc4>] genl_rcv+0x24/0x40
>>   [<ffffffffa05d277b>] netlink_unicast+0x1bb/0x290
>>   [<ffffffffa05d2bc5>] netlink_sendmsg+0x365/0x4d0
>>   [<ffffffffa0516e9f>] sock_sendmsg+0x5f/0x70
>>   [<ffffffffa0517211>] ____sys_sendmsg+0x201/0x280
>>   [<ffffffffa0519ca8>] ___sys_sendmsg+0x88/0xd0
>> 
>> But I guess you've found these already.
> 
> Nope, I haven't seen that either, but I'm not looking for
> memory leaks yet. My focus right now is to address netdev's
> concerns, so I haven't enabled kmemleak (as it slows down
> the dev loop). Not crashing is a higher priority than
> leaking right at the moment.

Or, to put this another way: I'm trying to address the
reference counting issues by addressing Paolo's request
for adding a socket destructor callback.


--
Chuck Lever

Re: Test results from v4

[Hannes Reinecke]

On 2/16/23 17:03, Chuck Lever III wrote:
> 
> 
>> On Feb 16, 2023, at 10:58 AM, Hannes Reinecke <hare@suse.de> wrote:
>>
>> Hi Chuck,
>>
[ .. ]
>> unreferenced object 0xffff9989c22a0018 (size 24):
>>   comm "tlshd", pid 2332, jiffies 4294964098 (age 1513.848s)
>>   hex dump (first 24 bytes):
>>     00 00 00 00 00 00 00 00 a8 31 85 c1 89 99 ff ff  .........1......
>>     00 00 00 00 00 00 00 00                          ........
>>   backtrace:
>>     [<ffffffffa00ad3e4>] security_file_alloc+0x24/0x90
>>     [<ffffffff9ffbb747>] __alloc_file+0x57/0xc0
>>     [<ffffffff9ffbbbc3>] alloc_empty_file+0x43/0xf0
>>     [<ffffffff9ffbbcad>] alloc_file+0x2d/0x170
>>     [<ffffffff9ffbbea7>] alloc_file_pseudo+0xa7/0x100
>>     [<ffffffffa05158a9>] sock_alloc_file+0x39/0xa0
>>     [<ffffffffa073594b>] handshake_genl_cmd_accept+0x19b/0x2a0
>>     [<ffffffffa05d44e7>] genl_family_rcv_msg_doit.isra.19+0xf7/0x120
>>     [<ffffffffa05d495f>] genl_rcv_msg+0x19f/0x2a0
>>     [<ffffffffa05d3286>] netlink_rcv_skb+0x56/0x110
>>     [<ffffffffa05d3cc4>] genl_rcv+0x24/0x40
>>     [<ffffffffa05d277b>] netlink_unicast+0x1bb/0x290
>>     [<ffffffffa05d2bc5>] netlink_sendmsg+0x365/0x4d0
>>     [<ffffffffa0516e9f>] sock_sendmsg+0x5f/0x70
>>     [<ffffffffa0517211>] ____sys_sendmsg+0x201/0x280
>>     [<ffffffffa0519ca8>] ___sys_sendmsg+0x88/0xd0
>>
>> But I guess you've found these already.
> 
> Nope, I haven't seen that either, but I'm not looking for
> memory leaks yet. My focus right now is to address netdev's
> concerns, so I haven't enabled kmemleak (as it slows down
> the dev loop). Not crashing is a higher priority than
> leaking right at the moment.
> Sure, perfectly fine.

I've got some patches queued up to enable PSK for v4 (both client and 
server side), how should I post them? Here on the list or on the general 
linux-block / linux-nvme list?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman

Re: Test results from v4

[Chuck Lever III]

> On Feb 16, 2023, at 11:57 AM, Hannes Reinecke <hare@suse.de> wrote:
> 
> 
> I've got some patches queued up to enable PSK for v4 (both client and server side), how should I post them? Here on the list or on the general linux-block / linux-nvme list?

Start here, let's see what you got.


--
Chuck Lever

Re: Test results from v4

[Hannes Reinecke]

On 2/16/23 18:17, Chuck Lever III wrote:
> 
>> On Feb 16, 2023, at 11:57 AM, Hannes Reinecke <hare@suse.de> wrote:
>>
>>
>> I've got some patches queued up to enable PSK for v4 (both client and server side), how should I post them? Here on the list or on the general linux-block / linux-nvme list?
> 
> Start here, let's see what you got.
> 
Done.

Handshake looks good, _except_ that the server side refuses to fetch new 
packets until client side closes the connection due to a timeout:

tlshd[11024]: (11024) gnutls(5): REC[0x209cc40]: Preparing Packet 
Handshake(22) with length: 559 and min pad: 0
tlshd[11024]: (11024) gnutls(9): ENC[0x209cc40]: cipher: NULL, MAC: 
MAC-NULL, Epoch: 0
tlshd[11024]: (11024) gnutls(5): REC[0x209cc40]: Sent Packet[1] 
Handshake(22) in epoch 0 and length: 564
tlshd[11024]: (11024) gnutls: The operation timed out (-319)
tlshd[11024]: (11024) Handshake with c472.arch.suse.de (10.161.60.216) 
failed
tlshd[11023]: (11023) gnutls(5): REC[0x209cbc0]: SSL 3.1 Handshake 
packet received. Epoch 0, length: 559

Any idea what could be causing it?
(And I checked, the ClientHello packet really is on the wire, so it's a 
server-side thingie).
I'm pretty sure the server side doesn't set any callbacks to the socket 
(yet), so I'm a bit at a loss what could be the reason here.

Thanks for any pointers.

Cheers,

Hannes

Re: Test results from v4

[Chuck Lever III]

> On Feb 17, 2023, at 6:36 AM, Hannes Reinecke <hare@suse.de> wrote:
> 
> On 2/16/23 18:17, Chuck Lever III wrote:
>>> On Feb 16, 2023, at 11:57 AM, Hannes Reinecke <hare@suse.de> wrote:
>>> 
>>> 
>>> I've got some patches queued up to enable PSK for v4 (both client and server side), how should I post them? Here on the list or on the general linux-block / linux-nvme list?
>> Start here, let's see what you got.
> Done.
> 
> Handshake looks good, _except_ that the server side refuses to fetch new packets until client side closes the connection due to a timeout:
> 
> tlshd[11024]: (11024) gnutls(5): REC[0x209cc40]: Preparing Packet Handshake(22) with length: 559 and min pad: 0
> tlshd[11024]: (11024) gnutls(9): ENC[0x209cc40]: cipher: NULL, MAC: MAC-NULL, Epoch: 0
> tlshd[11024]: (11024) gnutls(5): REC[0x209cc40]: Sent Packet[1] Handshake(22) in epoch 0 and length: 564
> tlshd[11024]: (11024) gnutls: The operation timed out (-319)
> tlshd[11024]: (11024) Handshake with c472.arch.suse.de (10.161.60.216) failed
> tlshd[11023]: (11023) gnutls(5): REC[0x209cbc0]: SSL 3.1 Handshake packet received. Epoch 0, length: 559
> 
> Any idea what could be causing it?

Since the handshake failed, my first guess is that you haven't
re-enabled your server's data_ready callback when the handshake
fails.

But honestly I haven't tested the case where the handshake
fails but the server wants to continue using the socket.
Obviously that is something we want to work -- the server
itself needs to decide whether to continue using that
connection.


> (And I checked, the ClientHello packet really is on the wire, so it's a server-side thingie).
> I'm pretty sure the server side doesn't set any callbacks to the socket (yet), so I'm a bit at a loss what could be the reason here.

--
Chuck Lever

Re: Test results from v4

[Chuck Lever III]

> On Feb 17, 2023, at 6:56 AM, Chuck Lever III <chuck.lever@oracle.com> wrote:
> 
> 
> 
>> On Feb 17, 2023, at 6:36 AM, Hannes Reinecke <hare@suse.de> wrote:
>> 
>> On 2/16/23 18:17, Chuck Lever III wrote:
>>>> On Feb 16, 2023, at 11:57 AM, Hannes Reinecke <hare@suse.de> wrote:
>>>> 
>>>> 
>>>> I've got some patches queued up to enable PSK for v4 (both client and server side), how should I post them? Here on the list or on the general linux-block / linux-nvme list?
>>> Start here, let's see what you got.
>> Done.
>> 
>> Handshake looks good, _except_ that the server side refuses to fetch new packets until client side closes the connection due to a timeout:
>> 
>> tlshd[11024]: (11024) gnutls(5): REC[0x209cc40]: Preparing Packet Handshake(22) with length: 559 and min pad: 0
>> tlshd[11024]: (11024) gnutls(9): ENC[0x209cc40]: cipher: NULL, MAC: MAC-NULL, Epoch: 0
>> tlshd[11024]: (11024) gnutls(5): REC[0x209cc40]: Sent Packet[1] Handshake(22) in epoch 0 and length: 564
>> tlshd[11024]: (11024) gnutls: The operation timed out (-319)
>> tlshd[11024]: (11024) Handshake with c472.arch.suse.de (10.161.60.216) failed
>> tlshd[11023]: (11023) gnutls(5): REC[0x209cbc0]: SSL 3.1 Handshake packet received. Epoch 0, length: 559
>> 
>> Any idea what could be causing it?
> 
> Since the handshake failed, my first guess is that you haven't
> re-enabled your server's data_ready callback when the handshake
> fails.

I noticed this while working on v5:

+static int tls_handshake_put_accept_resp(struct sk_buff *msg,
+					 struct tls_handshake_req *treq)
+{
+	struct nlattr *entry_attr;
+	int ret;
+
+	ret = -EMSGSIZE;
+	entry_attr = nla_nest_start(msg, HANDSHAKE_GENL_ATTR_ACCEPT);
+	if (!entry_attr)
+		goto out;
+
+	ret = nla_put_u32(msg, HANDSHAKE_GENL_ATTR_TLS_TYPE,
+			  HANDSHAKE_GENL_TLS_TYPE_CLIENTHELLO);
+	if (ret < 0)
+		goto out;

This is unconditionally setting TLS_TYPE to CLIENTHELLO.
Instead, it needs to do this:

+	ret = nla_put_u32(msg, HANDSHAKE_GENL_ATTR_TLS_TYPE,
+			  treq->th_type);

That way the handshake agent sees a SERVERHELLO request.


> But honestly I haven't tested the case where the handshake
> fails but the server wants to continue using the socket.
> Obviously that is something we want to work -- the server
> itself needs to decide whether to continue using that
> connection.
> 
> 
>> (And I checked, the ClientHello packet really is on the wire, so it's a server-side thingie).
>> I'm pretty sure the server side doesn't set any callbacks to the socket (yet), so I'm a bit at a loss what could be the reason here.
> 
> --
> Chuck Lever
> 
> 
> 
> 

--
Chuck Lever