deps: set CARES_RANDOM_FILE for c-ares

[richardlau]

Upstream c-ares renamed RANDOM_FILE to CARES_RANDOM_FILE some time ago in c-ares 1.17.2.

Refs: c-ares/c-ares#397

[nodejs-github-bot]

Review requested:

• @nodejs/net

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/51948/

[mhdawson]

LGTM

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/51951/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/51954/

[bnoordhuis]

LGTM but the c-ares logic from before yesterday's upgrade looks broken to me:

node/deps/cares/src/lib/ares_init.c

Lines 2189 to 2202 in 0e79635

	# ifdef CARES_RANDOM_FILE
	FILE *f = fopen(CARES_RANDOM_FILE, "rb");
	if(f) {
	setvbuf(f, NULL, _IONBF, 0);
	counter = aresx_uztosi(fread(key, 1, key_data_len, f));
	fclose(f);
	}
	# endif
	#endif /* WIN32 */
	if (!randomized) {
	for (;counter<key_data_len;counter++)
	key[counter]=(unsigned char)(rand() % 256); /* LCOV_EXCL_LINE */
	}

It reads from /dev/urandom but then overwrites the result with rand()...

Should probably be filed as a security bug against the current release lines because it results in somewhat predictable DNS sequence ids.

The latest c-ares still falls back to that code path when /dev/urandom isn't accessible.

[richardlau]

LGTM but the c-ares logic from before yesterday's upgrade looks broken to me:

node/deps/cares/src/lib/ares_init.c

Lines 2189 to 2202 in 0e79635

	# ifdef CARES_RANDOM_FILE
	FILE *f = fopen(CARES_RANDOM_FILE, "rb");
	if(f) {
	setvbuf(f, NULL, _IONBF, 0);
	counter = aresx_uztosi(fread(key, 1, key_data_len, f));
	fclose(f);
	}
	# endif
	#endif /* WIN32 */
	if (!randomized) {
	for (;counter<key_data_len;counter++)
	key[counter]=(unsigned char)(rand() % 256); /* LCOV_EXCL_LINE */
	}

It reads from /dev/urandom but then overwrites the result with rand()...

It doesn't reset counter so should not be overwriting the bytes read from /dev/urandom.

[bnoordhuis]

Right, but it assumes the read always succeeds with the requested number of bytes. At least musl libc sometimes return short or zero reads (e.g. when interrupted by a signal) and other libcs probably do too.

#define HAVE_ARC4RANDOM_BUF 1 on Linux could help because it's provided by glibc and handles edge cases like ☝️ but it's missing from musl...

[bnoordhuis]

#define HAVE_ARC4RANDOM_BUF 1 on Linux could help because it's provided by glibc and handles edge cases like ☝️ but it's missing from musl...

Next best thing: c-ares/c-ares#526

[nodejs-github-bot]

Landed in 70da075