[CORE-8020] tls: Watch for IN_MOVED_TO

[BenPope]

Fixes CORE-8020

A common pattern for updating certificates is to do:

mv new.crt existing.crt

So configure the watch to detect IN_MOVED_TO

[pgellert]

Interesting. The man page made me think that move_to is generated for the containing directory rather than the file and that move_self is more appropriate.

What happens if someone does mv existing.crt existing.crt.bkp as a way of deleting certificates? I'm wondering if that would trigger a delete_self or if we should be watching move_from as well.

https://man7.org/linux/man-pages/man7/inotify.7.html

[oleiman]

fwiw, I don't think mv existing.crt existing.crt.bkp will reliably invalidate the inode corresponding to existing.crt.

[IoannisRP]

When executing

mv existing.crt.bak existing.crt

existing.crt should trigger a delete_self, which is already watched for, and the parent dir should issue a move_to.

I do not understand why adding move_to to the file watcher fixes the issue. What am I missing here?

The add_dir_watch function seems to be there to cater for moving files around, but for some reason it is only applied on rebuild and not on init.

[oleiman]

Testing this naively at the command line, it doesn't seem like this type of mv generates a delete event, or at least not guaranteed right away.

which makes sense I think because inotify monitors the state of the inodes pointed to by the directory entry, rather than the name->inode mapping itself?

[oleiman]

Ah, never mind, this is a file watch. Now I'm confused too!

[IoannisRP]

Apologies, you are right. i should have been more careful in the description.
I was referring to the case where existing.crt already exists and we are trying to overwrite it with a move.

[oleiman]

well, that type of move does not immediately notify delete on the directory watch, but it does notify delete_self on the file watch. Anyway, your original question is apt I think.

[oleiman]

ok, i think i get it. from reading

seastar/src/net/tls-impl.cc

Lines 445 to 464 in 7820b86

	// #756
	// first, add a watch to nearest parent dir we
	// can find. If user deleted folders, we could end
	// up looking at modifications to root.
	// The idea is that should adding a watch to actual file
	// fail (deleted file/folder), we wait for changes to closest
	// parent. When this happens, we will retry all files
	// that have not been successfully replaced (and maybe more),
	// repeating the process. At some point, we hopefully
	// get new, current data.
	add_dir_watch(filename);
	// #756 add watch _first_. File could change while we are
	// reading this.
	try {
	add_watch(filename).get();
	} catch (...) {
	// let's just assume if this happens, it's because the file or folder was deleted.
	// just ignore for now, and hope the dir watch will tell us when it is back...
	return;
	}

so we're sort of extending the parent-dir fallback watcher to hopefully pick this up?

I don't really understand why mv new existing doesn't generate a delete_self event sometimes (ever?). Seems like it should also trigger this watcher though.