key_state_gen_auth_control_files has subtle logic mistake

[gcoxmoz]

Describe the bug

openvpn/src/openvpn/ssl_verify.c

Lines 996 to 1013 in 32e6586

	key_state_rm_auth_control_files(ads);
	const char *acf = platform_create_temp_file(opt->tmp_dir, "acf", &gc);
	const char *apf = platform_create_temp_file(opt->tmp_dir, "apf", &gc);
	const char *afr = platform_create_temp_file(opt->tmp_dir, "afr", &gc);
	if (acf && apf)
	{
	ads->auth_control_file = string_alloc(acf, NULL);
	ads->auth_pending_file = string_alloc(apf, NULL);
	ads->auth_failed_reason_file = string_alloc(afr, NULL);
	setenv_str(opt->es, "auth_control_file", ads->auth_control_file);
	setenv_str(opt->es, "auth_pending_file", ads->auth_pending_file);
	setenv_str(opt->es, "auth_failed_reason_file", ads->auth_failed_reason_file);
	}
	gc_free(&gc);
	return (acf && apf);

It feels like this function has grown from 2 to 3 files over time, so minimally it feels like you want to make the if and return lines be if (acf && apf && afr) to cover afr's creation. That is, you could end up in the true areas if afr failed but the other two succeeded.
OR, if this is intentional, it looks like a miss and might warrant a comment.

To Reproduce
Unknown, found by code reading. It's highly unlikely this edge case would fail you.

Expected behavior

Version information (please complete the following information):

Additional context

[flichtenheld]

This code was added in commit 8893fe4 before 2.6.0.
It definitely looks like an oversight at first glance. On the other hand key_state_check_auth_failed_message_file has a check if (ads->auth_failed_reason_file) so it is handled and might be intentional. Since this wasn't brought up in the code review only @schwabe might know.

[cron2]

This is safe (string_alloc() is well-defined for NULL pointers) but indeed looks a bit like an oversight. If creating any of these fails, "something is wrong with the system" (out of memory, disk full, ...) so I wouldn't consider "succeeding authentication" a good idea in that case - no matter which file failed. Anyway, assigning to @schwabe :-)