Only exposing necessary information

[tumbl3w33d]

I'd argue it would make sense to, by default, not expose the running kani version (currently sent as response header). I don't see how this helps anyone (but developers or someone actively debugging) and in that case it should be something you deliberately activate. You might just as well execute kanidmd version in the running container. If you want to monitor what version was running at a certain point in time, I think this should be part of your (internal) metrics endpoint.

Actually it's the same question about the swagger endpoint being active by default, however, that seems less critical. Curious to hear your opinions about it.

Basic principle is that you only expose as much information as necessary to not make it easier for attackers (especially automated information gathering).

[yaleman]

The response header's currently used as a check to ensure the client isn't doing weird things against the wrong version, and yes both that and the swagger docs could be used to infer the version...

... but so could hashing the WASM returned for the UI and a variety of other things? 🤔

[tumbl3w33d]

I see. So if I go full paranoid and drop the header in my proxy, do I risk malfunction of the client?

[yaleman]

It will just whinge about the version mismatch, that's all -

kanidm/libs/client/src/lib.rs

Lines 592 to 620 in 12900ee

	/// Check that we're getting the right version back from the server.
	async fn expect_version(&self, response: &reqwest::Response) {
	let mut guard = self.check_version.lock().await;
	if !*guard {
	return;
	}
	let ver = response
	.headers()
	.get(KVERSION)
	.and_then(|hv| hv.to_str().ok())
	.unwrap_or("");
	let matching = ver == EXPECT_VERSION;
	if !matching {
	warn!(server_version = ?ver, client_version = ?EXPECT_VERSION, "Mismatched client and server version - features may not work, or other unforeseen errors may occur.")
	}
	#[cfg(any(test, debug_assertions))]
	if !matching {
	error!("You're in debug/dev mode, so we're going to quit here.");
	std::process::exit(1);
	}
	// Check is done once, mark as no longer needing to occur
	*guard = false;
	}

[Firstyear]

Basic principle is that you only expose as much information as necessary to not make it easier for attackers (especially automated information gathering).

I'm going to challenge this notion. Similar to Kerckhoffs principle, knowledge of a system should not weaken the system.

Knowing the version of Kanidm does not improve the situation for an attacker. Inversely, even if they didn't know the version, there are still ways to fingerprint the version based on feature availability. And finally, you don't need to know the version to attempt exploits (if they existed) against the server.

As a result, the version does not change anything for an attacker IMO.

[tumbl3w33d]

I agree, but it's what you keep hearing from security people and written in penetration test reports here. I think it's on the level of moving a default port of an app to dodge some scripts and basically it's fine for me to follow such advice when it doesn't cost anything. But now I know you connect the client's compatibility check to it.

[Firstyear]

Then you hire better security people. :)

And yes, we use it to check for compatibility with the client, so we can warn if there will be issues, so removing it would break that and potentially lead to other issues.