Store and verify parent dir dev/ino and pass O_PATH fds

In order to be robust against symlink attacks (i.e. make a document
for a path, then replace it with a symlink somewhere else and have the
portal read that instead) we store the parent dev/ino when we create
the document id and always verify that (atomically with the *at
syscalls) on each use.

Also, we pass O_PATH fds when creating documents, as it allows us
to be a bit safer. For instance we can verify that the fd is a O_PATH
fd before doing any ops on it, and it makes it possible to avoid other
symlink trickery.

Also, we drop the double add methods, and just use the O_PATH version.

4 files changed, 272 insertions, 156 deletions

diff --git a/document-portal/xdp-fuse.c b/document-portal/xdp-fuse.c
index cbaaa6e..22e70ae 100644
--- a/document-portal/xdp-fuse.c
+++ b/document-portal/xdp-fuse.c
@@ -108,7 +108,7 @@ get_entry_cache_time (int st_mode)
  * the list (add/remove) or name of a tmpfile.
  *
  * Each instance has a mutex that locks access to the backing path,
- * as it can be removed at runtime. Use get/steal_backing_path() to
+ * as it can be removed at runtime. Use get/steal_backing_basename() to
  * safely access it.
  *
  ******************************* XdpTmp *******************************/
@@ -122,6 +122,7 @@ typedef struct
   /* These are immutable, no lock needed */
   guint64 parent_inode;
   guint32 tmp_id;
+  XdgAppDbEntry *entry;
 
   /* Changes always done under tmp_files lock */
   char *name;
@@ -129,7 +130,7 @@ typedef struct
   GMutex mutex;
 
   /* protected by mutex */
-  char *backing_path;
+  char *backing_basename;
 } XdpTmp;
 
 /* Owns a ref to the files */
@@ -148,30 +149,31 @@ xdp_tmp_unref (XdpTmp *tmp)
 {
   if (g_atomic_int_dec_and_test (&tmp->ref_count))
     {
+      xdg_app_db_entry_unref (tmp->entry);
       g_free (tmp->name);
-      g_free (tmp->backing_path);
+      g_free (tmp->backing_basename);
       g_free (tmp);
     }
 }
 
 char *
-xdp_tmp_get_backing_path (XdpTmp *tmp)
+xdp_tmp_get_backing_basename (XdpTmp *tmp)
 {
   char *res;
   g_mutex_lock (&tmp->mutex);
-  res = g_strdup (tmp->backing_path);
+  res = g_strdup (tmp->backing_basename);
   g_mutex_unlock (&tmp->mutex);
 
   return res;
 }
 
 char *
-xdp_tmp_steal_backing_path (XdpTmp *tmp)
+xdp_tmp_steal_backing_basename (XdpTmp *tmp)
 {
   char *res;
   g_mutex_lock (&tmp->mutex);
-  res = tmp->backing_path;
-  tmp->backing_path = NULL;
+  res = tmp->backing_basename;
+  tmp->backing_basename = NULL;
   g_mutex_unlock (&tmp->mutex);
 
   return res;
@@ -227,17 +229,25 @@ find_tmp_by_id (guint32 tmp_id)
 /* Caller must hold tmp_files lock */
 static XdpTmp *
 xdp_tmp_new_nolock (fuse_ino_t parent,
+                    XdgAppDbEntry *entry,
                     const char *name,
-                    const char *path)
+                    const char *tmp_basename)
 {
   XdpTmp *tmp;
+  g_autofree char *tmp_dirname = NULL;
+
+  /* We store the pathname instead of dir_fd + basename, because
+     its very easy to get a lot of tempfiles leaking and that would
+     mean quite a lot of open fds */
+  tmp_dirname = xdp_entry_dup_dirname (entry);
 
   tmp = g_new0 (XdpTmp, 1);
+  tmp->ref_count = 2; /* One owned by tmp_files */
+  tmp->tmp_id = g_atomic_int_add (&next_tmp_id, 1);
   tmp->parent_inode = parent;
   tmp->name = g_strdup (name);
-  tmp->backing_path = g_strdup (path);
-  tmp->tmp_id = g_atomic_int_add (&next_tmp_id, 1);
-  tmp->ref_count = 1; /* Owned by tmp_files */
+  tmp->entry = xdg_app_db_entry_ref (entry);
+  tmp->backing_basename = g_strdup (tmp_basename);
 
   tmp_files = g_list_prepend (tmp_files, tmp);
 
@@ -249,11 +259,15 @@ static void
 xdp_tmp_unlink_nolock (XdpTmp *tmp)
 {
 
-  g_autofree char *backing_path = NULL;
+  g_autofree char *backing_basename = NULL;
 
-  backing_path = xdp_tmp_steal_backing_path (tmp);
-  if (backing_path)
-    unlink (backing_path);
+  backing_basename = xdp_tmp_steal_backing_basename (tmp);
+  if (backing_basename)
+    {
+      glnx_fd_close int dir_fd = xdp_entry_open_dir (tmp->entry);
+      if (dir_fd)
+        unlinkat (dir_fd, backing_basename, 0);
+    }
 
   tmp_files = g_list_remove (tmp_files, tmp);
   xdp_tmp_unref (tmp);
@@ -291,8 +305,9 @@ typedef struct
   /* These are immutable, no lock needed */
   guint32 tmp_id;
   fuse_ino_t inode;
-  char *trunc_path;
-  char *real_path;
+  int dir_fd;
+  char *trunc_basename;
+  char *real_basename;
 
   /* These need a lock whenever they are used */
   int fd;
@@ -319,24 +334,24 @@ xdp_fh_finalize (XdpFh *fh)
   if (fh->truncated)
     {
       fsync (fh->trunc_fd);
-      if (rename (fh->trunc_path, fh->real_path) != 0)
+      if (renameat (fh->dir_fd, fh->trunc_basename,
+                    fh->dir_fd, fh->real_basename) != 0)
         g_warning ("Unable to replace truncated document");
     }
-  else if (fh->trunc_path)
-    unlink (fh->trunc_path);
+  else if (fh->trunc_basename)
+    unlinkat (fh->dir_fd, fh->trunc_basename, 0);
 
   if (fh->fd >= 0)
     close (fh->fd);
-  if (fh->trunc_fd >= 0)
-    close (fh->trunc_fd);
 
-  if (fh->fd >= 0)
-    close (fh->fd);
   if (fh->trunc_fd >= 0)
     close (fh->trunc_fd);
 
-  g_clear_pointer (&fh->trunc_path, g_free);
-  g_clear_pointer (&fh->real_path, g_free);
+  if (fh->dir_fd >= 0)
+    close (fh->dir_fd);
+
+  g_clear_pointer (&fh->trunc_basename, g_free);
+  g_clear_pointer (&fh->real_basename, g_free);
 
   g_free (fh);
 }
@@ -398,6 +413,7 @@ xdp_fh_new (fuse_ino_t inode,
   fh->fd = fd;
   if (tmp)
     fh->tmp_id = tmp->tmp_id;
+  fh->dir_fd = -1;
   fh->trunc_fd = -1;
   fh->ref_count = 1; /* Owned by fuse_file_info fi */
 
@@ -656,7 +672,7 @@ app_can_see_doc (XdgAppDbEntry *entry, guint32 app_id)
   const char *app_name = get_app_name_from_id (app_id);
 
   if (app_name != NULL &&
-      xdp_has_permissions (entry, app_name, XDP_PERMISSION_FLAGS_READ))
+      xdp_entry_has_permissions (entry, app_name, XDP_PERMISSION_FLAGS_READ))
     return TRUE;
 
   return FALSE;
@@ -670,10 +686,9 @@ xdp_stat (fuse_ino_t ino,
   XdpInodeClass class = get_class (ino);
   guint64 class_ino = get_class_ino (ino);
   g_autoptr (XdgAppDbEntry) entry = NULL;
-  const char *path = NULL;
   struct stat tmp_stbuf;
   g_autoptr(XdpTmp) tmp = NULL;
-  g_autofree char *backing_path = NULL;
+  g_autofree char *backing_basename = NULL;
 
   stbuf->st_ino = ino;
 
@@ -736,8 +751,7 @@ xdp_stat (fuse_ino_t ino,
 
       stbuf->st_nlink = DOC_FILE_NLINK;
 
-      path = xdp_get_path (entry);
-      if (stat (path, &tmp_stbuf) != 0)
+      if (xdp_entry_stat (entry, &tmp_stbuf, AT_SYMLINK_NOFOLLOW) != 0)
         return ENOENT;
 
       stbuf->st_mode = S_IFREG | get_user_perms (&tmp_stbuf);
@@ -759,10 +773,16 @@ xdp_stat (fuse_ino_t ino,
       stbuf->st_mode = S_IFREG;
       stbuf->st_nlink = DOC_FILE_NLINK;
 
-      backing_path = xdp_tmp_get_backing_path (tmp);
+      backing_basename = xdp_tmp_get_backing_basename (tmp);
 
-      if (backing_path == NULL || stat (backing_path, &tmp_stbuf) != 0)
-        return ENOENT;
+      {
+        glnx_fd_close int dir_fd = xdp_entry_open_dir (tmp->entry);
+
+        if (backing_basename == NULL ||
+            dir_fd == -1 ||
+            fstatat (dir_fd, backing_basename, &tmp_stbuf, 0) != 0)
+          return ENOENT;
+      }
 
       stbuf->st_mode = S_IFREG | get_user_perms (&tmp_stbuf);
       stbuf->st_size = tmp_stbuf.st_size;
@@ -905,7 +925,7 @@ xdp_lookup (fuse_ino_t parent,
         entry = xdp_lookup_doc (parent_class_ino);
       if (entry != NULL)
         {
-          g_autofree char *basename = xdp_dup_basename (entry);
+          g_autofree char *basename = xdp_entry_dup_basename (entry);
           if (strcmp (name, basename) == 0)
             {
               *inode = make_inode (DOC_FILE_INO_CLASS, parent_class_ino);
@@ -1036,9 +1056,8 @@ dirbuf_add_doc_file (fuse_req_t req,
                      guint32 doc_id)
 {
   struct stat tmp_stbuf;
-  const char *path = xdp_get_path (entry);
-  g_autofree char *basename = xdp_dup_basename (entry);
-  if (stat (path, &tmp_stbuf) == 0)
+  g_autofree char *basename = xdp_entry_dup_basename (entry);
+  if (xdp_entry_stat (entry, &tmp_stbuf, AT_SYMLINK_NOFOLLOW) == 0)
     dirbuf_add (req, b, basename,
                 make_inode (DOC_FILE_INO_CLASS, doc_id));
 }
@@ -1222,14 +1241,13 @@ get_open_flags (struct fuse_file_info *fi)
 }
 
 static char *
-create_tmp_for_doc (XdgAppDbEntry *entry, int flags, int *fd_out)
+create_tmp_for_doc (XdgAppDbEntry *entry, int dir_fd, int flags, int *fd_out)
 {
-  g_autofree char *dirname = xdp_dup_dirname (entry);
-  g_autofree char *basename = xdp_dup_basename (entry);
-  g_autofree char *template = g_strconcat (dirname, "/.", basename, ".XXXXXX", NULL);
+  g_autofree char *basename = xdp_entry_dup_basename (entry);
+  g_autofree char *template = g_strconcat (".xdp_", basename, ".XXXXXX", NULL);
   int fd;
 
-  fd = g_mkstemp_full (template, flags, 0600);
+  fd = xdg_app_mkstempat (dir_fd, template, flags, 0600);
   if (fd == -1)
     return NULL;
 
@@ -1246,7 +1264,6 @@ xdp_fuse_open (fuse_req_t req,
   guint64 class_ino = get_class_ino (ino);
   struct stat stbuf = {0};
   g_autoptr (XdgAppDbEntry) entry = NULL;
-  const char *path = NULL;
   g_autoptr(XdpTmp) tmp = NULL;
   glnx_fd_close int fd = -1;
   int res;
@@ -1268,50 +1285,59 @@ xdp_fuse_open (fuse_req_t req,
 
   if (entry && class == DOC_FILE_INO_CLASS)
     {
-      g_autofree char *write_path = NULL;
+      g_autofree char *tmp_basename = NULL;
       glnx_fd_close int write_fd = -1;
+      glnx_fd_close int dir_fd = -1;
+      g_autofree char *basename = xdp_entry_dup_basename (entry);
 
-      path = xdp_get_path (entry);
+      dir_fd = xdp_entry_open_dir (entry);
+      if (dir_fd == -1)
+        {
+          fuse_reply_err (req, errno);
+          return;
+        }
 
       if ((fi->flags & 3) != O_RDONLY)
         {
-          if (access (path, W_OK) != 0)
+          if (faccessat (dir_fd, basename, W_OK, 0) != 0)
             {
               fuse_reply_err (req, errno);
               return;
             }
-          write_path = create_tmp_for_doc (entry, O_RDWR, &write_fd);
-          if (write_path == NULL)
+          tmp_basename = create_tmp_for_doc (entry, dir_fd, O_RDWR, &write_fd);
+          if (tmp_basename == NULL)
             {
               fuse_reply_err (req, errno);
               return;
             }
         }
 
-      fd = open (path, O_RDONLY);
+      fd = openat (dir_fd, basename, O_RDONLY|O_NOFOLLOW|O_CLOEXEC);
       if (fd < 0)
         {
           fuse_reply_err (req, errno);
           return;
         }
       fh = xdp_fh_new (ino, fi, steal_fd(&fd), NULL);
+      fh->dir_fd = steal_fd (&dir_fd);
       fh->trunc_fd = steal_fd (&write_fd);
-      fh->trunc_path = g_steal_pointer (&write_path);
-      fh->real_path = g_strdup (path);
+      fh->trunc_basename = g_steal_pointer (&tmp_basename);
+      fh->real_basename = g_strdup (basename);
       if (fuse_reply_open (req, fi))
         xdp_fh_unref (fh);
     }
   else if (class == TMPFILE_INO_CLASS &&
            (tmp = find_tmp_by_id (class_ino)))
     {
-      g_autofree char *backing_path = xdp_tmp_get_backing_path (tmp);
-      if (backing_path == NULL)
+      glnx_fd_close int dir_fd = xdp_entry_open_dir (tmp->entry);
+      g_autofree char *backing_basename = xdp_tmp_get_backing_basename (tmp);
+      if (dir_fd == -1 || backing_basename == NULL)
         {
           fuse_reply_err (req, ENOENT);
           return;
         }
 
-      fd = open (backing_path, get_open_flags (fi));
+      fd = openat (dir_fd, backing_basename, get_open_flags (fi));
       if (fd < 0)
         {
           fuse_reply_err (req, errno);
@@ -1338,7 +1364,6 @@ xdp_fuse_create (fuse_req_t req,
   XdpFh *fh;
   g_autoptr(XdgAppDbEntry) entry = NULL;
   g_autofree char *basename = NULL;
-  const char *path = NULL;
   glnx_fd_close int fd = -1;
   int res;
 
@@ -1363,23 +1388,29 @@ xdp_fuse_create (fuse_req_t req,
       return;
     }
 
-  basename = xdp_dup_basename (entry);
+  basename = xdp_entry_dup_basename (entry);
   if (strcmp (name, basename) == 0)
     {
-      g_autofree char *write_path = NULL;
+      g_autofree char *tmp_basename = NULL;
       glnx_fd_close int write_fd = -1;
       guint32 doc_id = xdp_id_from_name (name);
+      glnx_fd_close int dir_fd = -1;
 
-      write_path = create_tmp_for_doc (entry, O_RDWR, &write_fd);
-      if (write_path == NULL)
+      dir_fd = xdp_entry_open_dir (entry);
+      if (dir_fd == -1)
         {
           fuse_reply_err (req, errno);
           return;
         }
 
-      path = xdp_get_path (entry);
+      tmp_basename = create_tmp_for_doc (entry, dir_fd, O_RDWR, &write_fd);
+      if (tmp_basename == NULL)
+        {
+          fuse_reply_err (req, errno);
+          return;
+        }
 
-      fd = open (path, O_CREAT|O_EXCL|O_RDONLY);
+      fd = openat (dir_fd, basename, O_CREAT|O_EXCL|O_RDONLY|O_NOFOLLOW|O_CLOEXEC);
       if (fd < 0)
         {
           fuse_reply_err (req, errno);
@@ -1389,10 +1420,11 @@ xdp_fuse_create (fuse_req_t req,
       e.ino = make_inode (DOC_FILE_INO_CLASS, doc_id);
 
       fh = xdp_fh_new (e.ino, fi, steal_fd (&fd), NULL);
+      fh->dir_fd = steal_fd (&dir_fd);
       fh->truncated = TRUE;
       fh->trunc_fd = steal_fd (&write_fd);
-      fh->trunc_path = g_steal_pointer (&write_path);
-      fh->real_path = g_strdup (path);
+      fh->trunc_basename = g_steal_pointer (&tmp_basename);
+      fh->real_basename = g_strdup (basename);
 
       if (xdp_fh_fstat_locked (fh, &e.attr) != 0)
         {
@@ -1422,18 +1454,19 @@ xdp_fuse_create (fuse_req_t req,
 
       if (tmpfile)
         {
-          g_autofree char *backing_path = NULL;
+          glnx_fd_close int dir_fd = xdp_entry_open_dir (tmpfile->entry);
+          g_autofree char *backing_basename = NULL;
 
           G_UNLOCK(tmp_files);
 
-          backing_path = xdp_tmp_get_backing_path (tmpfile);
-          if (backing_path == NULL)
+          backing_basename = xdp_tmp_get_backing_basename (tmpfile);
+          if (dir_fd == -1 || backing_basename == NULL)
             {
               fuse_reply_err (req, EINVAL);
               return;
             }
 
-          fd = open (backing_path, get_open_flags (fi));
+          fd = openat (dir_fd, backing_basename, get_open_flags (fi));
           if (fd == -1)
             {
               fuse_reply_err (req, errno);
@@ -1443,13 +1476,21 @@ xdp_fuse_create (fuse_req_t req,
       else
         {
           int errsv;
-          g_autofree char *tmp_path = NULL;
+          g_autofree char *tmp_basename = NULL;
+          glnx_fd_close int dir_fd = -1;
+
+          dir_fd = xdp_entry_open_dir (entry);
+          if (dir_fd == -1)
+            {
+              fuse_reply_err (req, errno);
+              return;
+            }
 
-          tmp_path = create_tmp_for_doc (entry, get_open_flags (fi), &fd);
-          if (tmp_path == NULL)
-            return NULL;
+          tmp_basename = create_tmp_for_doc (entry, dir_fd, get_open_flags (fi), &fd);
+          if (tmp_basename == NULL)
+            return;
 
-          tmpfile = xdp_tmp_new_nolock (parent, name, tmp_path);
+          tmpfile = xdp_tmp_new_nolock (parent, entry, name, tmp_basename);
           errsv = errno;
           G_UNLOCK(tmp_files);
 
@@ -1626,17 +1667,24 @@ xdp_fuse_rename (fuse_req_t req,
       return;
     }
 
-  basename = xdp_dup_basename (entry);
+  basename = xdp_entry_dup_basename (entry);
 
   if (strcmp (newname, basename) == 0)
     {
-      const char *real_path = xdp_get_path (entry);
-      g_autofree char *backing_path = NULL;
+      glnx_fd_close int dir_fd = -1;
+      g_autofree char *backing_basename = NULL;
       /* Rename tmpfile to regular file */
 
+      dir_fd = xdp_entry_open_dir (entry);
+      if (dir_fd == -1)
+        {
+          fuse_reply_err (req, errno);
+          return;
+        }
+
       /* Steal backing path so we don't delete it when unlinking tmp */
-      backing_path = xdp_tmp_steal_backing_path (tmp);
-      if (backing_path == NULL)
+      backing_basename = xdp_tmp_steal_backing_basename (tmp);
+      if (backing_basename == NULL)
         {
           fuse_reply_err (req, EINVAL);
           return;
@@ -1645,7 +1693,8 @@ xdp_fuse_rename (fuse_req_t req,
       /* Stop writes to all outstanding fds to the temp file */
       mark_open_tmp_file_readonly (tmp->tmp_id);
 
-      if (rename (backing_path, real_path) != 0)
+      if (renameat (dir_fd, backing_basename,
+                    dir_fd, basename) != 0)
         {
           fuse_reply_err (req, errno);
           return;
@@ -1797,7 +1846,7 @@ xdp_fuse_fsyncdir (fuse_req_t req,
       entry = xdp_lookup_doc (doc_id);
       if (entry != NULL)
         {
-          g_autofree char *dirname = xdp_dup_dirname (entry);
+          g_autofree char *dirname = xdp_entry_dup_dirname (entry);
           int fd = open (dirname, O_DIRECTORY|O_RDONLY);
           if (fd >= 0)
             {
@@ -1868,12 +1917,19 @@ xdp_fuse_unlink (fuse_req_t req,
       return;
     }
 
-  basename = xdp_dup_basename (entry);
+  basename = xdp_entry_dup_basename (entry);
   if (strcmp (name, basename) == 0)
     {
-      const char *real_path = xdp_get_path (entry);
+      glnx_fd_close int dir_fd = -1;
+
+      dir_fd = xdp_entry_open_dir (entry);
+      if (dir_fd == -1)
+        {
+          fuse_reply_err (req, errno);
+          return;
+        }
 
-      if (unlink (real_path) != 0)
+      if (unlinkat (dir_fd, basename, 0) != 0)
         {
           fuse_reply_err (req, errno);
           return;
diff --git a/document-portal/xdp-main.c b/document-portal/xdp-main.c
index f6f9d7e..cace699 100644
--- a/document-portal/xdp-main.c
+++ b/document-portal/xdp-main.c
@@ -134,7 +134,8 @@ portal_grant_permissions (GDBusMethodInvocation *invocation,
   perms = xdp_parse_permissions (permissions);
 
   /* Must have grant-permissions and all the newly granted permissions */
-  if (!xdp_has_permissions (entry, app_id, XDP_PERMISSION_FLAGS_GRANT_PERMISSIONS | perms))
+  if (!xdp_entry_has_permissions (entry, app_id,
+                                  XDP_PERMISSION_FLAGS_GRANT_PERMISSIONS | perms))
     {
       g_dbus_method_invocation_return_error (invocation, XDG_APP_ERROR, XDG_APP_ERROR_NOT_ALLOWED,
                                              "Not enough permissions");
@@ -142,7 +143,7 @@ portal_grant_permissions (GDBusMethodInvocation *invocation,
     }
 
   do_set_permissions (entry, id, target_app_id,
-                      perms | xdp_get_permissions (entry, target_app_id));
+                      perms | xdp_entry_get_permissions (entry, target_app_id));
 
   g_dbus_method_invocation_return_value (invocation, g_variant_new ("()"));
 }
@@ -178,7 +179,8 @@ portal_revoke_permissions (GDBusMethodInvocation *invocation,
   perms = xdp_parse_permissions (permissions);
 
   /* Must have grant-permissions, or be itself */
-  if (!xdp_has_permissions (entry, app_id, XDP_PERMISSION_FLAGS_GRANT_PERMISSIONS) ||
+  if (!xdp_entry_has_permissions (entry, app_id,
+                                  XDP_PERMISSION_FLAGS_GRANT_PERMISSIONS) ||
       strcmp (app_id, target_app_id) == 0)
     {
       g_dbus_method_invocation_return_error (invocation, XDG_APP_ERROR, XDG_APP_ERROR_NOT_ALLOWED,
@@ -187,7 +189,7 @@ portal_revoke_permissions (GDBusMethodInvocation *invocation,
     }
 
   do_set_permissions (entry, id, target_app_id,
-                      ~perms & xdp_get_permissions (entry, target_app_id));
+                      ~perms & xdp_entry_get_permissions (entry, target_app_id));
 
   g_dbus_method_invocation_return_value (invocation, g_variant_new ("()"));
 }
@@ -210,7 +212,7 @@ portal_delete (GDBusMethodInvocation *invocation,
       return;
     }
 
-  if (!xdp_has_permissions (entry, app_id, XDP_PERMISSION_FLAGS_DELETE))
+  if (!xdp_entry_has_permissions (entry, app_id, XDP_PERMISSION_FLAGS_DELETE))
     {
       g_dbus_method_invocation_return_error (invocation, XDG_APP_ERROR, XDG_APP_ERROR_NOT_ALLOWED,
                                              "Not enough permissions");
@@ -226,13 +228,19 @@ portal_delete (GDBusMethodInvocation *invocation,
 }
 
 char *
-do_create_doc (const char *path)
+do_create_doc (struct stat *parent_st_buf, const char *path)
 {
-  g_autoptr(GVariant) data = g_variant_ref_sink (g_variant_new_bytestring (path));
+  g_autoptr(GVariant) data = NULL;
   g_autoptr (XdgAppDbEntry) entry = NULL;
   g_auto(GStrv) ids = NULL;
   char *id = NULL;
 
+  data =
+    g_variant_ref_sink (g_variant_new ("(^aytt)",
+                                       path,
+                                       (guint64)parent_st_buf->st_dev,
+                                       (guint64)parent_st_buf->st_ino));
+
   ids = xdg_app_db_list_ids_by_value (db, data);
 
   if (ids[0] != NULL)
@@ -268,47 +276,18 @@ portal_add (GDBusMethodInvocation *invocation,
             GVariant *parameters,
             const char *app_id)
 {
-  const char *path;
-  g_autofree char *id = NULL;
-
-  if (app_id[0] != '\0')
-    {
-      /* don't allow this from within the sandbox */
-      g_dbus_method_invocation_return_error (invocation,
-                                             XDG_APP_ERROR, XDG_APP_ERROR_NOT_ALLOWED,
-                                             "Not allowed inside sandbox");
-      return;
-    }
-
-  g_variant_get (parameters, "(^ay)", &path);
-  if (!g_path_is_absolute (path))
-    {
-      g_dbus_method_invocation_return_error (invocation,
-                                             XDG_APP_ERROR, XDG_APP_ERROR_INVALID_ARGUMENT,
-                                             "Document paths must be absolute");
-      return;
-    }
-
-  id = do_create_doc (path);
-
-  g_dbus_method_invocation_return_value (invocation,
-                                         g_variant_new ("(s)", id));
-}
-
-static void
-portal_add_local (GDBusMethodInvocation *invocation,
-                  GVariant *parameters,
-                  const char *app_id)
-{
   GDBusMessage *message;
   GUnixFDList *fd_list;
   g_autofree char *id = NULL;
   g_autofree char *proc_path = NULL;
   int fd_id, fd, fds_len, fd_flags;
+  glnx_fd_close int dir_fd = -1;
   const int *fds;
   char path_buffer[PATH_MAX+1];
   ssize_t symlink_size;
-  struct stat st_buf, real_st_buf;
+  struct stat st_buf, real_st_buf, real_parent_st_buf;
+  g_autofree char *dirname = NULL;
+  g_autofree char *name = NULL;
 
   g_variant_get (parameters, "(h)", &fd_id);
 
@@ -326,15 +305,18 @@ portal_add_local (GDBusMethodInvocation *invocation,
   proc_path = g_strdup_printf ("/proc/self/fd/%d", fd);
 
   if (fd == -1 ||
+      /* Must be able to get fd flags */
+      (fd_flags = fcntl (fd, F_GETFL)) == -1 ||
+      /* Must be O_PATH */
+      ((fd_flags & O_PATH) != O_PATH) ||
+      /* Must not be O_NOFOLLOW (because we want the target file) */
+      ((fd_flags & O_NOFOLLOW) == O_PATH) ||
       /* Must be able to fstat */
       fstat (fd, &st_buf) < 0 ||
       /* Must be a regular file */
       (st_buf.st_mode & S_IFMT) != S_IFREG ||
-      /* Must be able to get fd flags */
-      (fd_flags = fcntl (fd, F_GETFL)) == -1 ||
-      /* Must be able to read */
-      (fd_flags & O_ACCMODE) == O_WRONLY ||
       /* Must be able to read path from /proc/self/fd */
+      /* This is an absolute and (at least at open time) symlink-expanded path */
       (symlink_size = readlink (proc_path, path_buffer, sizeof (path_buffer) - 1)) < 0)
     {
       g_dbus_method_invocation_return_error (invocation,
@@ -345,7 +327,15 @@ portal_add_local (GDBusMethodInvocation *invocation,
 
   path_buffer[symlink_size] = 0;
 
-  if (lstat (path_buffer, &real_st_buf) < 0 ||
+  /* We open the parent directory and do the stat in that, so that we have
+     trustworthy parent dev/ino for later verification. Otherwise the caller
+     could later replace a parent with a symlink and make us read some other file */
+  dirname = g_path_get_dirname (path_buffer);
+  name = g_path_get_basename (path_buffer);
+  dir_fd = open (dirname, O_CLOEXEC|O_PATH);
+
+  if (fstat (dir_fd, &real_parent_st_buf) < 0 ||
+      fstatat (dir_fd, name, &real_st_buf, AT_SYMLINK_NOFOLLOW) < 0 ||
       st_buf.st_dev != real_st_buf.st_dev ||
       st_buf.st_ino != real_st_buf.st_ino)
     {
@@ -356,19 +346,17 @@ portal_add_local (GDBusMethodInvocation *invocation,
       return;
     }
 
-  id = do_create_doc (path_buffer);
+  id = do_create_doc (&real_parent_st_buf, path_buffer);
 
   if (app_id[0] != '\0')
     {
-      /* also grant app-id perms based on file_mode */
-      guint32 perms = XDP_PERMISSION_FLAGS_GRANT_PERMISSIONS | XDP_PERMISSION_FLAGS_READ;
       g_autoptr(XdgAppDbEntry) entry = NULL;
       entry = xdg_app_db_lookup (db, id);
 
-      if ((fd_flags & O_ACCMODE) == O_RDWR)
-        perms |= XDP_PERMISSION_FLAGS_WRITE;
-
-      do_set_permissions (entry, id, app_id, perms);
+      do_set_permissions (entry, id, app_id,
+                          XDP_PERMISSION_FLAGS_GRANT_PERMISSIONS |
+                          XDP_PERMISSION_FLAGS_READ |
+                          XDP_PERMISSION_FLAGS_WRITE);
     }
 
   g_dbus_method_invocation_return_value (invocation,
@@ -428,7 +416,6 @@ on_bus_acquired (GDBusConnection *connection,
 
   g_signal_connect_swapped (helper, "handle-get-mount-point", G_CALLBACK (handle_get_mount_point), NULL);
   g_signal_connect_swapped (helper, "handle-add", G_CALLBACK (handle_method), portal_add);
-  g_signal_connect_swapped (helper, "handle-add-local", G_CALLBACK (handle_method), portal_add_local);
   g_signal_connect_swapped (helper, "handle-grant-permissions", G_CALLBACK (handle_method), portal_grant_permissions);
   g_signal_connect_swapped (helper, "handle-revoke-permissions", G_CALLBACK (handle_method), portal_revoke_permissions);
   g_signal_connect_swapped (helper, "handle-delete", G_CALLBACK (handle_method), portal_delete);
diff --git a/document-portal/xdp-util.c b/document-portal/xdp-util.c
index d838d75..d0c6d59 100644
--- a/document-portal/xdp-util.c
+++ b/document-portal/xdp-util.c
@@ -50,7 +50,7 @@ xdp_parse_permissions (const char **permissions)
 }
 
 XdpPermissionFlags
-xdp_get_permissions (XdgAppDbEntry *entry,
+xdp_entry_get_permissions (XdgAppDbEntry *entry,
                      const char *app_id)
 {
   g_autofree const char **permissions = NULL;
@@ -63,13 +63,13 @@ xdp_get_permissions (XdgAppDbEntry *entry,
 }
 
 gboolean
-xdp_has_permissions (XdgAppDbEntry *entry,
+xdp_entry_has_permissions (XdgAppDbEntry *entry,
                      const char *app_id,
                      XdpPermissionFlags perms)
 {
   XdpPermissionFlags current_perms;
 
-  current_perms = xdp_get_permissions (entry, app_id);
+  current_perms = xdp_entry_get_permissions (entry, app_id);
 
   return (current_perms & perms) == perms;
 }
@@ -87,28 +87,92 @@ xdp_name_from_id (guint32 doc_id)
 }
 
 const char *
-xdp_get_path (XdgAppDbEntry *entry)
+xdp_entry_get_path (XdgAppDbEntry *entry)
 {
   g_autoptr(GVariant) v = xdg_app_db_entry_get_data (entry);
-  return g_variant_get_bytestring (v);
+  g_autoptr(GVariant) c = g_variant_get_child_value (v, 0);
+  return g_variant_get_bytestring (c);
 }
 
 char *
-xdp_dup_basename (XdgAppDbEntry *entry)
+xdp_entry_dup_basename (XdgAppDbEntry *entry)
 {
-  const char *path = xdp_get_path (entry);
+  const char *path = xdp_entry_get_path (entry);
 
   return g_path_get_basename (path);
 }
 
 char *
-xdp_dup_dirname (XdgAppDbEntry *entry)
+xdp_entry_dup_dirname (XdgAppDbEntry *entry)
 {
-  const char *path = xdp_get_path (entry);
+  const char *path = xdp_entry_get_path (entry);
 
   return g_path_get_dirname (path);
 }
 
+guint64
+xdp_entry_get_device (XdgAppDbEntry *entry)
+{
+  g_autoptr(GVariant) v = xdg_app_db_entry_get_data (entry);
+  g_autoptr(GVariant) c = g_variant_get_child_value (v, 1);
+  return g_variant_get_uint64 (c);
+}
+
+guint64
+xdp_entry_get_inode (XdgAppDbEntry *entry)
+{
+  g_autoptr(GVariant) v = xdg_app_db_entry_get_data (entry);
+  g_autoptr(GVariant) c = g_variant_get_child_value (v, 2);
+  return g_variant_get_uint64 (c);
+}
+
+int
+xdp_entry_open_dir (XdgAppDbEntry *entry)
+{
+  g_autofree char *dirname = xdp_entry_dup_dirname (entry);
+  struct stat st_buf;
+  int fd;
+
+  fd = open (dirname, O_CLOEXEC | O_PATH | O_DIRECTORY);
+  if (fd == -1)
+    return -1;
+
+  if (fstat (fd, &st_buf) < 0)
+    {
+      close (fd);
+      errno = ENOENT;
+      return -1;
+    }
+
+  if (st_buf.st_ino != xdp_entry_get_inode (entry) ||
+      st_buf.st_dev != xdp_entry_get_device (entry))
+    {
+      close (fd);
+      errno = ENOENT;
+      return -1;
+    }
+
+  return fd;
+}
+
+int
+xdp_entry_stat (XdgAppDbEntry *entry,
+                struct stat *buf,
+                int flags)
+{
+  glnx_fd_close int fd = -1;
+  g_autofree char *basename = xdp_entry_dup_basename (entry);
+
+  fd = xdp_entry_open_dir (entry);
+  if (fd < 0)
+    return -1;
+
+  if (fstatat (fd, basename, buf, flags) != 0)
+    return -1;
+
+  return 0;
+}
+
 static GHashTable *app_ids;
 
 typedef struct {
diff --git a/document-portal/xdp-util.h b/document-portal/xdp-util.h
index 3fa8de1..cb67817 100644
--- a/document-portal/xdp-util.h
+++ b/document-portal/xdp-util.h
@@ -9,16 +9,25 @@ G_BEGIN_DECLS
 
 const char **      xdg_unparse_permissions (XdpPermissionFlags   permissions);
 XdpPermissionFlags xdp_parse_permissions   (const char         **permissions);
-XdpPermissionFlags xdp_get_permissions     (XdgAppDbEntry       *entry,
-                                            const char          *app_id);
-gboolean           xdp_has_permissions     (XdgAppDbEntry       *entry,
-                                            const char          *app_id,
-                                            XdpPermissionFlags   perms);
-const char *       xdp_get_path            (XdgAppDbEntry       *entry);
-char *             xdp_dup_basename        (XdgAppDbEntry       *entry);
-char *             xdp_dup_dirname         (XdgAppDbEntry       *entry);
-guint32            xdp_id_from_name        (const char          *name);
-char *             xdp_name_from_id        (guint32              doc_id);
+
+XdpPermissionFlags xdp_entry_get_permissions (XdgAppDbEntry      *entry,
+                                              const char         *app_id);
+gboolean           xdp_entry_has_permissions (XdgAppDbEntry      *entry,
+                                              const char         *app_id,
+                                              XdpPermissionFlags  perms);
+const char *       xdp_entry_get_path        (XdgAppDbEntry      *entry);
+char *             xdp_entry_dup_basename    (XdgAppDbEntry      *entry);
+char *             xdp_entry_dup_dirname     (XdgAppDbEntry      *entry);
+guint64            xdp_entry_get_device      (XdgAppDbEntry      *entry);
+guint64            xdp_entry_get_inode       (XdgAppDbEntry      *entry);
+int                xdp_entry_open_dir        (XdgAppDbEntry      *entry);
+int                xdp_entry_stat            (XdgAppDbEntry      *entry,
+                                              struct stat        *buf,
+                                              int                 flags);
+
+guint32 xdp_id_from_name (const char *name);
+char *  xdp_name_from_id (guint32     doc_id);
+
 
 void  xdp_invocation_lookup_app_id        (GDBusMethodInvocation  *invocation,
                                            GCancellable           *cancellable,