Skip to content

Commit

Permalink
cockpit: support setting owner/group in fsreplace1
Browse files Browse the repository at this point in the history
Cockpit-files wants to support uploading or creating a file owned by the
current directory which might be different from the logged in user.

For example as superuser uploading a database into `/var/lib/postgresql`
which would be owned by `postgres` and the database file should receive
the same permissions.
  • Loading branch information
jelly committed Nov 1, 2024
1 parent e57b414 commit 489d3f5
Show file tree
Hide file tree
Showing 8 changed files with 124 additions and 5 deletions.
8 changes: 8 additions & 0 deletions doc/protocol.md
Original file line number Diff line number Diff line change
Expand Up @@ -1103,6 +1103,14 @@ The following options can be specified in the "open" control message:
you don't set this field, the actual tag will not be checked. To
express that you expect the file to not exist, use "-" as the tag.

* "uid": The expected uid of the file. If set, fsreplace1 chowns the
file to the given uid, and optionally gid if set. This requires
superuser privileges to be available

* "gid": The expected gid of the file. If set, fsreplace chowns the
file to the given gid if the uid is also set. This requires
superuser privileges to be available

You should write the new content to the channel as one or more
messages. To indicate the end of the content, send a "done" message.

Expand Down
2 changes: 1 addition & 1 deletion pkg/lib/cockpit.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ declare module 'cockpit' {

interface FileHandle<T> {
read(): Promise<T>;
replace(new_content: T | null, expected_tag?: FileTag): Promise<FileTag>;
replace(new_content: T | null, expected_tag?: FileTag, owner?: string, group_name?: string): Promise<FileTag>;
watch(callback: FileWatchCallback<T>, options?: { read?: boolean }): FileWatchHandle;
modify(callback: (data: T | null) => T | null, initial_content?: string, initial_tag?: FileTag): Promise<[T, FileTag]>;
close(): void;
Expand Down
12 changes: 11 additions & 1 deletion pkg/lib/cockpit.js
Original file line number Diff line number Diff line change
Expand Up @@ -2245,7 +2245,8 @@ function factory() {

let replace_channel = null;

function replace(new_content, expected_tag) {
function replace(new_content, expected_tag, owner, group_name) {
console.log("replace", owner, group_name);
const dfd = cockpit.defer();

let file_content;
Expand All @@ -2265,6 +2266,15 @@ function factory() {
path,
tag: expected_tag
};

if (owner) {
opts.owner = owner;
}
if (group_name) {
opts.group_name = group_name;
}
console.log("fsreplace opts", opts);

replace_channel = cockpit.channel(opts);

replace_channel.addEventListener("close", function (event, message) {
Expand Down
2 changes: 2 additions & 0 deletions pkg/lib/cockpit/_internal/channel.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { ensure_transport, transport_globals } from './transport';

export function Channel(options) {
const self = this;
console.log("channel options", options);

/* We can trigger events */
event_mixin(self, { });
Expand Down Expand Up @@ -106,6 +107,7 @@ export function Channel(options) {
command[i] = options[i];
command.command = "open";
command.channel = id;
console.log("open", command);

if (!command.host) {
if (transport_globals.default_host)
Expand Down
12 changes: 12 additions & 0 deletions pkg/playground/test.html
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,18 @@
<p>cockpit.user() information</p>
<div id="user-info" />
</div>
<br/>
<div id="fsreplace1-div">
<h2>fsreplace1 test</h2>
<p>new filename</p>
<input id="fsreplace1-filename" />
<p>text content</p>
<input id="fsreplace1-content" />
<p>owner mode</p>
<input id="fsreplace1-owner" placeholder="For example, admin" />
<input id="fsreplace1-group" placeholder="For example, users" />
<button id="fsreplace1-create" class="pf-v5-c-button pf-m-secondary">Create file</button>
</div>
</section>
</main>
</div>
Expand Down
14 changes: 14 additions & 0 deletions pkg/playground/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,20 @@ document.addEventListener("DOMContentLoaded", () => {
document.getElementById("user-info").textContent = JSON.stringify(info);
});

const fsreplace_btn = document.getElementById("fsreplace1-create");
fsreplace_btn.addEventListener("click", e => {
fsreplace_btn.disabled = true;
const filename = document.getElementById("fsreplace1-filename").value;
console.log("Filename", filename);
const content = document.getElementById("fsreplace1-content").value;
const owner = document.getElementById("fsreplace1-owner").value;
const group = document.getElementById("fsreplace1-group").value;
cockpit.file(filename, { superuser: "try" }).replace(content, undefined, owner, group).then(() => {
fsreplace_btn.disabled = false;
})
.catch(exc => console.log(exc));
});

cockpit.addEventListener("visibilitychange", show_hidden);
show_hidden();
});
34 changes: 31 additions & 3 deletions src/cockpit/channels/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,31 @@ def delete(self, path: str, tag: 'str | None') -> str:
os.unlink(path)
return '-'

async def set_contents(self, path: str, tag: 'str | None', data: 'bytes | None', size: 'int | None') -> str:
def get_owner_id(self, owner: 'str') -> int:
try:
return pwd.getpwnam(owner).pw_uid
except KeyError:
raise ChannelError('internal-error', message=f'uid not found for {owner}') from None

def get_group_id(self, group: 'str') -> int:
try:
return grp.getgrnam(group).gr_gid
except KeyError:
raise ChannelError('internal-error', message=f'gid not found for {group}') from None

async def set_contents(self, path: str, tag: 'str | None', data: 'bytes | None', size: 'int | None',
owner: 'str | None', group: 'str | None') -> str:
dirname, basename = os.path.split(path)
tmpname: str | None
fd, tmpname = tempfile.mkstemp(dir=dirname, prefix=f'.{basename}-')

def chown_if_required(fd: 'int'):
if owner is not None and group is not None:
os.fchown(fd, self.get_owner_id(owner), self.get_group_id(group))
elif owner is not None:
uid = self.get_owner_id(owner)
os.fchown(fd, uid, uid)

try:
if size is not None:
logger.debug('fallocate(%s.tmp, %d)', path, size)
Expand All @@ -195,12 +216,14 @@ async def set_contents(self, path: str, tag: 'str | None', data: 'bytes | None',
# no preconditions about what currently exists or not
# calculate the file mode from the umask
os.fchmod(fd, 0o666 & ~my_umask())
chown_if_required(fd)
os.rename(tmpname, path)
tmpname = None

elif tag == '-':
# the file must not exist. file mode from umask.
os.fchmod(fd, 0o666 & ~my_umask())
chown_if_required(fd)
os.link(tmpname, path) # will fail if file exists

else:
Expand All @@ -225,22 +248,27 @@ async def run(self, options: JsonObject) -> JsonObject:
path = get_str(options, 'path')
size = get_int(options, 'size', None)
tag = get_str(options, 'tag', None)
owner = get_str(options, 'owner', None)
group = get_str(options, 'group_name', None)

if owner is None and group is not None:
raise ChannelError('protocol-error', message='cannot provide a group without an owner')

try:
# In the `size` case, .set_contents() sends the ready only after
# it knows that the allocate was successful. In the case without
# `size`, we need to send the ready() up front in order to
# receive the first frame and decide if we're creating or deleting.
if size is not None:
tag = await self.set_contents(path, tag, b'', size)
tag = await self.set_contents(path, tag, b'', size, owner, group)
else:
self.ready()
data = await self.read()
# if we get EOF right away, that's a request to delete
if data is None:
tag = self.delete(path, tag)
else:
tag = await self.set_contents(path, tag, data, None)
tag = await self.set_contents(path, tag, data, None, owner, group)

self.done()
return {'tag': tag}
Expand Down
45 changes: 45 additions & 0 deletions test/verify/check-pages
Original file line number Diff line number Diff line change
Expand Up @@ -997,6 +997,51 @@ OnCalendar=daily
self.assertEqual(str(user_info["id"]), m.execute("id -u admin").strip())
self.assertEqual(str(user_info["gid"]), m.execute("id -g admin").strip())

def testFsreplaceOwnership(self) -> None:
b = self.browser

self.login_and_go("/playground/test")

def stat(fmt: str, path: str) -> str:
return self.machine.execute(['stat', f'--format={fmt}', path]).strip()

def assert_stat(fmt: str, path: str, expected: str) -> None:
self.assertEqual(stat(fmt, path), expected)

def assert_owner(path: str, owner: str) -> None:
assert_stat('%U:%G', path, owner)

def assert_content(path: str, expected: str) -> None:
self.assertEqual(self.machine.execute(f"cat {path}").strip(), expected)

def set_file_bits(filename: str, content: str, owner: str, group: str = "") -> None:
b.set_input_text("#fsreplace1-filename", filename)
b.set_input_text("#fsreplace1-content", content)
b.set_input_text("#fsreplace1-owner", owner)
b.set_input_text("#fsreplace1-group", group)

b.click("#fsreplace1-create")
b.wait_visible("#fsreplace1-create:not(:disabled)")

filename = "/home/admin/admin.txt"
content = "adminfile"
set_file_bits(filename, content, "admin", "users")
assert_owner(filename, "admin:users")
assert_content(filename, content)

filename = "/home/admin/root.txt"
content = "rootfile"
set_file_bits(filename, content, "root", "root")
assert_owner(filename, "root:root")
assert_content(filename, content)

# only passing an owner becomes admin:admin
filename = "/home/admin/admin-special.txt"
content = "admin-special"
set_file_bits(filename, content, "admin")
assert_owner(filename, "admin:admin")
assert_content(filename, content)


if __name__ == '__main__':
testlib.test_main()

0 comments on commit 489d3f5

Please sign in to comment.