Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VFS server hardening for the use as resource multiplexer #1751

Closed
nfeske opened this issue Oct 29, 2015 · 15 comments
Closed

VFS server hardening for the use as resource multiplexer #1751

nfeske opened this issue Oct 29, 2015 · 15 comments
Labels

Comments

@nfeske
Copy link
Member

nfeske commented Oct 29, 2015

The VFS server as added in issue #1648 allows us to export an arbitrary virtual file systems as a file-system service. In the future, it will eventually replace most of the other file-system services because their functionality can be easily emulated by the combination of the VFS with various VFS plugins.

However, this aspired future raises security concerns expressed in the discussion of #1648.

  • The node cache as present in the original VFS implementation may be misused as denial-of-service vector or as a covert channel.
  • The RAM consumption of the VFS must be subjected to a policy that ensures the robustness of the VFS in the presence of misbehaving clients.
  • The use of env()->heap() for allocating VFS handles does not follow the heap partitioning scheme as advised in our book http://genode.org/documentation/genode-foundations-15-05.pdf in Section 3.3.3. This was acceptable as long as the VFS was solely used as a library but it is an unfortunate design choice in a resource multiplexer.

More elaborate descriptions of those problems are given in the comments of #1648.

@nfeske nfeske added the cleanup label Oct 29, 2015
@ehmry
Copy link
Contributor

ehmry commented Nov 3, 2015

One of the reasons for the node cache was to propagate notification signals between sessions, something that should happen at the VFS anyway for the sake of consistency between the VFS and the File_system interfaces.

@ehmry
Copy link
Contributor

ehmry commented Apr 1, 2016

I have rewritten most of the VFS server with the new Out_of_metadata exception in mind.

Each session uses its own ram session and heap to allocate handles, the node cache is gone. I also changed the behaviour for clients that do not match policy from explicit denial to read-only access at the root. This is because I think its better to deny clients before the request reaches the server, and an empty <policy/> node looks rather vague if you don't know why it is there.

The VFS server now uses more memory overall, but only needs a small initial donation from the parent. I fixed the dangling handle pointer issue in the ram file system and put in allocation guards to throw NO_SPACE, so memory exhaustion from the ram plugin shouldn't effect the reliability of the server.

I made some other trailing changes to the VFS, these I made while debugging the server.

A few of these commits overlap other issues, so I say cherry pick from my issue1751 branch, and let that close them.

ehmry added a commit to ehmry/genode that referenced this issue Apr 1, 2016
Opening a VFS handle previously involved allocating from the global heap
at each VFS file system. By amending open with an allocator argument,
dynamic allocation can be partitioned.

A new close method is used to deallocate open handles.

Issue genodelabs#1751
Issue genodelabs#1891
ehmry added a commit to ehmry/genode that referenced this issue Apr 1, 2016
Reference count files to prevent dangling handles.
Catch out-of-memory conditions and throw NO_SPACE.

Issue genodelabs#1751
ehmry added a commit to ehmry/genode that referenced this issue Apr 1, 2016
Replace the Out_of_node_handles exception with Out_of_metadata.
Clients need to know when the server is out of internal resources,
but not why.

Cleanup and sort the errors at file_system_session.h.
Remove 'Size_limit_reached exception' from File_system, which was
internal to ram_fs.

Issue genodelabs#1751
Fixes genodelabs#1909
ehmry added a commit to ehmry/genode that referenced this issue Apr 1, 2016
Upgrade the File_system session RAM quota when an Out_of_metadata
exception is caught.

Issue genodelabs#1751
Issue genodelabs#1909
ehmry added a commit to ehmry/genode that referenced this issue Apr 1, 2016
ehmry added a commit to ehmry/genode that referenced this issue Apr 1, 2016
VFS handles are allocated from per-session heaps.

Fixes genodelabs#1751
ehmry added a commit to ehmry/genode that referenced this issue Apr 1, 2016
ehmry added a commit to ehmry/genode that referenced this issue Apr 1, 2016
New errors STAT_ERR_NO_PERM, DIRENT_ERR_NO_PERM, and READLINK_NO_PERM to
distinguish lookup errors from permissions or other errors.

Issue genodelabs#1751
ehmry added a commit to ehmry/genode that referenced this issue Apr 1, 2016
@ehmry
Copy link
Contributor

ehmry commented Apr 3, 2016

"lib/vfs: consistent device and inode enumeration" d974fb3 moved to #1929.

@nfeske
Copy link
Member Author

nfeske commented Apr 4, 2016

That sounds fantastic!

@ehmry
Copy link
Contributor

ehmry commented Apr 6, 2016

The next improvement on the VFS should be the fair processing of packets, right now a client can hog the server thread by submitting a constant stream of packets.

chelmuth pushed a commit that referenced this issue Apr 11, 2016
Opening a VFS handle previously involved allocating from the global heap
at each VFS file system. By amending open with an allocator argument,
dynamic allocation can be partitioned.

A new close method is used to deallocate open handles.

Issue #1751
Issue #1891
chelmuth pushed a commit that referenced this issue Apr 11, 2016
Reference count files to prevent dangling handles.
Catch out-of-memory conditions and throw NO_SPACE.

Issue #1751
chelmuth pushed a commit that referenced this issue Apr 11, 2016
Replace the Out_of_node_handles exception with Out_of_metadata.
Clients need to know when the server is out of internal resources,
but not why.

Cleanup and sort the errors at file_system_session.h.
Remove 'Size_limit_reached exception' from File_system, which was
internal to ram_fs.

Issue #1751
Fixes #1909
chelmuth pushed a commit that referenced this issue Apr 11, 2016
Upgrade the File_system session RAM quota when an Out_of_metadata
exception is caught.

Issue #1751
Issue #1909
chelmuth pushed a commit that referenced this issue Apr 11, 2016
VFS handles are allocated from per-session heaps.

Fixes #1751
chelmuth pushed a commit that referenced this issue Apr 11, 2016
chelmuth pushed a commit that referenced this issue Apr 11, 2016
New errors STAT_ERR_NO_PERM, DIRENT_ERR_NO_PERM, and READLINK_NO_PERM to
distinguish lookup errors from permissions or other errors.

Issue #1751
@chelmuth
Copy link
Member

@ehmry I merged this great line of work, but have one remaining question you may help me to answer. In 784c7eb, you changed inode from unsigned long to unsigned in Vfs::Directory_service::Stat and I'm wondering what is your intention behind this change. Also in some places, the VFS code stores (addr_t)this in inode and device members, which is incompatible with unsigned integers on 64-bit.

@ehmry
Copy link
Contributor

ehmry commented Apr 11, 2016

I bumped it down because ino_t is a uint32 in the libc, and I intended the addr_t inodes to get narrowed, I hoped as long as the low bits come through they should be unique.

@chelmuth
Copy link
Member

I'd prefer to stay with unsigned long for both members. (At least my 64-bit) Ubuntu uses

sizeof(unsigned) = 4
sizeof(unsigned long) = 8
sizeof(ino_t) = 8
sizeof(dev_t) = 8

ehmry added a commit to ehmry/genode that referenced this issue Apr 11, 2016
ehmry added a commit to ehmry/genode that referenced this issue Apr 11, 2016
@cnuke
Copy link
Member

cnuke commented Apr 11, 2016

Only for historical reasons FreeBSD uses (or maybe used by now) 32bit inode numbers. I would also vote for using unsigned long in the VFS.

@ehmry
Copy link
Contributor

ehmry commented Apr 11, 2016

Ok, I can make a fixup commit now.

@chelmuth
Copy link
Member

Great.

ehmry added a commit to ehmry/genode that referenced this issue Apr 11, 2016
Increase width of inodes from 32 to 64 bits.

Issue genodelabs#1751
@ehmry
Copy link
Contributor

ehmry commented Apr 11, 2016

I left device as it is, 32bits.

ehmry added a commit to ehmry/genode that referenced this issue Apr 11, 2016
Increase width of inodes and devices from 32 to 64 bits.

Issue genodelabs#1751
@ehmry
Copy link
Contributor

ehmry commented Apr 11, 2016

Just ignore 32e0a62 and use e8bdf61.

ehmry added a commit to ehmry/genode that referenced this issue Apr 11, 2016
ehmry added a commit to ehmry/genode that referenced this issue Apr 11, 2016
chelmuth pushed a commit that referenced this issue Apr 11, 2016
Increase width of inodes and devices from 32 to 64 bits.

Issue #1751
@chelmuth
Copy link
Member

Merged it just now. Thanks for addressing device too - I'll sleep better at night with the patch ;-)

ehmry added a commit to ehmry/genode that referenced this issue Apr 13, 2016
Check for zero-length Name arguments.
Prevent the root session argument from modifiying a trailing root
directory element.
Hide a debugging printf.

Issue genodelabs#1751
chelmuth pushed a commit that referenced this issue Apr 13, 2016
Check for zero-length Name arguments.
Prevent the root session argument from modifiying a trailing root
directory element.
Hide a debugging printf.

Issue #1751
ehmry added a commit to ehmry/genode that referenced this issue Apr 13, 2016
Check for zero-length Name arguments.
Prevent the root session argument from modifiying a trailing root
directory element.
Hide a debugging printf.

Issue genodelabs#1751
@ehmry
Copy link
Contributor

ehmry commented Apr 16, 2016

I have another small fixup, 31925b7. This one causes the server to exit if there is no <vfs/> configured.

ehmry added a commit to ehmry/genode that referenced this issue Apr 16, 2016
Exit if <vfs/> is not found.
Add timer parent services to vfs_stress_fs.run, vfs_stress_ram.run.

Issue genodelabs#1751
ehmry added a commit to ehmry/genode that referenced this issue Apr 16, 2016
Exit if <vfs/> is not found.
Add timer parent services to vfs_stress_fs.run, vfs_stress_ram.run.

Issue genodelabs#1751
nfeske pushed a commit that referenced this issue Apr 16, 2016
Exit if <vfs/> is not found.
Add timer parent services to vfs_stress_fs.run, vfs_stress_ram.run.

Issue #1751
chelmuth pushed a commit that referenced this issue Apr 25, 2016
Replace the Out_of_node_handles exception with Out_of_metadata.
Clients need to know when the server is out of internal resources,
but not why.

Cleanup and sort the errors at file_system_session.h.
Remove 'Size_limit_reached exception' from File_system, which was
internal to ram_fs.

Issue #1751
Fixes #1909
chelmuth pushed a commit that referenced this issue Apr 25, 2016
Upgrade the File_system session RAM quota when an Out_of_metadata
exception is caught.

Issue #1751
Issue #1909
chelmuth pushed a commit that referenced this issue Apr 25, 2016
chelmuth pushed a commit that referenced this issue Apr 25, 2016
New errors STAT_ERR_NO_PERM, DIRENT_ERR_NO_PERM, and READLINK_NO_PERM to
distinguish lookup errors from permissions or other errors.

Issue #1751
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants