Re: [LTP] [PATCH v3 4/4] Add test for CVE 2023-31248

Martin Doucha
2023-11-21T14:25:20Z

Hi,
I'll send a v4 shortly, just a few comments below.

On 21. 11. 23 14:19, Petr Vorel wrote:
>> + /* Add destination chain */
>> + header.nlmsg_type = (NFNL_SUBSYS_NFTABLES << 8) | NFT_MSG_NEWCHAIN;
>> + header.nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE;
>> + nfpayload.nfgen_family = NFPROTO_IPV4;
>> + nfpayload.res_id = htons(0);
>> + NETLINK_ADD_MESSAGE(ctx, &header, &nfpayload, sizeof(nfpayload));
>> + NETLINK_ADD_ATTR_LIST(ctx, destchain_config);
>> +
>> + /* Delete destination chain */
>> + header.nlmsg_type = (NFNL_SUBSYS_NFTABLES << 8) | NFT_MSG_DELCHAIN;
>> + header.nlmsg_flags = NLM_F_REQUEST;
>> + nfpayload.nfgen_family = NFPROTO_IPV4;
>> + nfpayload.res_id = htons(0);
>> + NETLINK_ADD_MESSAGE(ctx, &header, &nfpayload, sizeof(nfpayload));
>> + NETLINK_ADD_ATTR_LIST(ctx, delchain_config);
>> +
>> + /* Add destination chain */
> nit: this looks to be source chain
> Out of curriosity I'm looking at the reproducer
> (https://bugzilla.suse.com/attachment.cgi?id=868806)
> and it needs just single chain.
> But test needs both for some reason.

Yes, the comment is a copy-paste mistake, I'll fix that in v4. In the
original reproducer, the rule being added to a deleted chain is silently
ignored on fixed kernels and the test does not get any errors that could
be used to determine the presence of vulnerability. I've rewritten the
scenario originally submitted by Souta Kawahara which adds a rule to an
existing chain which references the deleted chain though a GOTO
operation. That'll trigger ENOENT error on fixed kernels.

>> +static struct tst_test test = {
>> + .test_all = run,
>> + .setup = setup,
>> + .cleanup = cleanup,
>> + .taint_check = TST_TAINT_W | TST_TAINT_D,
>> + .needs_kconfigs = (const char *[]) {
>> + "CONFIG_USER_NS=y",
>> + "CONFIG_NF_TABLES",
>> + NULL
>> + },
>> + .save_restore = (const struct tst_path_val[]) {
>> + {"/proc/sys/user/max_user_namespaces", "1024", TST_SR_SKIP},
> Out of curiosity, why this?
>
> CVE mentions "Exploiting it requires CAP_NET_ADMIN in any user or network
> namespace.", but how is it related to changing max_user_namespaces value?
>
> Also, vulnerable kernel reproducers with any max_user_namespaces value, or
> without setting max_user_namespaces at all.

There were some issues with older distros which set max_user_namespaces
to zero by default. It's just standard boilerplate for safely calling
tst_setup_netns().

--
Martin Doucha mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic