1. 26 Jul, 2021 6 commits
    • Eric Biggers's avatar
      fscrypt: align Base64 encoding with RFC 4648 base64url · ba47b515
      Eric Biggers authored
      fscrypt uses a Base64 encoding to encode no-key filenames (the filenames
      that are presented to userspace when a directory is listed without its
      encryption key).  There are many variants of Base64, but the most common
      ones are specified by RFC 4648.  fscrypt can't use the regular RFC 4648
      "base64" variant because "base64" uses the '/' character, which isn't
      allowed in filenames.  However, RFC 4648 also specifies a "base64url"
      variant for use in URLs and filenames.  "base64url" is less common than
      "base64", but it's still implemented in many programming libraries.
      
      Unfortunately, what fscrypt actually uses is a custom Base64 variant
      that differs from "base64url" in several ways:
      
      - The binary data is divided into 6-bit chunks differently.
      
      - Values 62 and 63 are encoded with '+' and ',' instead of '-' and '_'.
      
      - '='-padding isn't used.  This isn't a problem per se, as the padding
        isn't technically necessary, and RFC 4648 doesn't strictly require it.
        But it needs to be properly documented.
      
      There have been two attempts to copy the fscrypt Base64 code into lib/
      (https://lkml.kernel.org/r/20200821182813.52570-6-jlayton@kernel.org and
      https://lkml.kernel.org/r/20210716110428.9727-5-hare@suse.de), and both
      have been caught up by the fscrypt Base64 variant being nonstandard and
      not properly documented.  Also, the planned use of the fscrypt Base64
      code in the CephFS storage back-end will prevent it from being changed
      later (whereas currently it can still be changed), so we need to choose
      an encoding that we're happy with before it's too late.
      
      Therefore, switch the fscrypt Base64 variant to base64url, in order to
      align more closely with RFC 4648 and other implementations and uses of
      Base64.  However, I opted not to implement '='-padding, as '='-padding
      adds complexity, is unnecessary, and isn't required by the RFC.
      
      Link: https://lore.kernel.org/r/20210718000125.59701-1-ebiggers@kernel.orgReviewed-by: default avatarHannes Reinecke <hare@suse.de>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      ba47b515
    • Eric Biggers's avatar
      fscrypt: remove mention of symlink st_size quirk from documentation · e538b098
      Eric Biggers authored
      Now that the correct st_size is reported for encrypted symlinks on all
      filesystems, update the documentation accordingly.
      
      Link: https://lore.kernel.org/r/20210702065350.209646-6-ebiggers@kernel.orgSigned-off-by: default avatarEric Biggers <ebiggers@google.com>
      e538b098
    • Eric Biggers's avatar
      ubifs: report correct st_size for encrypted symlinks · 064c7349
      Eric Biggers authored
      The stat() family of syscalls report the wrong size for encrypted
      symlinks, which has caused breakage in several userspace programs.
      
      Fix this by calling fscrypt_symlink_getattr() after ubifs_getattr() for
      encrypted symlinks.  This function computes the correct size by reading
      and decrypting the symlink target (if it's not already cached).
      
      For more details, see the commit which added fscrypt_symlink_getattr().
      
      Fixes: ca7f85be ("ubifs: Add support for encrypted symlinks")
      Cc: stable@vger.kernel.org
      Link: https://lore.kernel.org/r/20210702065350.209646-5-ebiggers@kernel.orgSigned-off-by: default avatarEric Biggers <ebiggers@google.com>
      064c7349
    • Eric Biggers's avatar
      f2fs: report correct st_size for encrypted symlinks · 461b43a8
      Eric Biggers authored
      The stat() family of syscalls report the wrong size for encrypted
      symlinks, which has caused breakage in several userspace programs.
      
      Fix this by calling fscrypt_symlink_getattr() after f2fs_getattr() for
      encrypted symlinks.  This function computes the correct size by reading
      and decrypting the symlink target (if it's not already cached).
      
      For more details, see the commit which added fscrypt_symlink_getattr().
      
      Fixes: cbaf042a ("f2fs crypto: add symlink encryption")
      Cc: stable@vger.kernel.org
      Link: https://lore.kernel.org/r/20210702065350.209646-4-ebiggers@kernel.orgSigned-off-by: default avatarEric Biggers <ebiggers@google.com>
      461b43a8
    • Eric Biggers's avatar
      ext4: report correct st_size for encrypted symlinks · 8c4bca10
      Eric Biggers authored
      The stat() family of syscalls report the wrong size for encrypted
      symlinks, which has caused breakage in several userspace programs.
      
      Fix this by calling fscrypt_symlink_getattr() after ext4_getattr() for
      encrypted symlinks.  This function computes the correct size by reading
      and decrypting the symlink target (if it's not already cached).
      
      For more details, see the commit which added fscrypt_symlink_getattr().
      
      Fixes: f348c252 ("ext4 crypto: add symlink encryption")
      Cc: stable@vger.kernel.org
      Link: https://lore.kernel.org/r/20210702065350.209646-3-ebiggers@kernel.orgSigned-off-by: default avatarEric Biggers <ebiggers@google.com>
      8c4bca10
    • Eric Biggers's avatar
      fscrypt: add fscrypt_symlink_getattr() for computing st_size · d1876056
      Eric Biggers authored
      Add a helper function fscrypt_symlink_getattr() which will be called
      from the various filesystems' ->getattr() methods to read and decrypt
      the target of encrypted symlinks in order to report the correct st_size.
      
      Detailed explanation:
      
      As required by POSIX and as documented in various man pages, st_size for
      a symlink is supposed to be the length of the symlink target.
      Unfortunately, st_size has always been wrong for encrypted symlinks
      because st_size is populated from i_size from disk, which intentionally
      contains the length of the encrypted symlink target.  That's slightly
      greater than the length of the decrypted symlink target (which is the
      symlink target that userspace usually sees), and usually won't match the
      length of the no-key encoded symlink target either.
      
      This hadn't been fixed yet because reporting the correct st_size would
      require reading the symlink target from disk and decrypting or encoding
      it, which historically has been considered too heavyweight to do in
      ->getattr().  Also historically, the wrong st_size had only broken a
      test (LTP lstat03) and there were no known complaints from real users.
      (This is probably because the st_size of symlinks isn't used too often,
      and when it is, typically it's for a hint for what buffer size to pass
      to readlink() -- which a slightly-too-large size still works for.)
      
      However, a couple things have changed now.  First, there have recently
      been complaints about the current behavior from real users:
      
      - Breakage in rpmbuild:
        https://github.com/rpm-software-management/rpm/issues/1682
        https://github.com/google/fscrypt/issues/305
      
      - Breakage in toybox cpio:
        https://www.mail-archive.com/toybox@lists.landley.net/msg07193.html
      
      - Breakage in libgit2: https://issuetracker.google.com/issues/189629152
        (on Android public issue tracker, requires login)
      
      Second, we now cache decrypted symlink targets in ->i_link.  Therefore,
      taking the performance hit of reading and decrypting the symlink target
      in ->getattr() wouldn't be as big a deal as it used to be, since usually
      it will just save having to do the same thing later.
      
      Also note that eCryptfs ended up having to read and decrypt symlink
      targets in ->getattr() as well, to fix this same issue; see
      commit 3a60a168 ("eCryptfs: Decrypt symlink target for stat size").
      
      So, let's just bite the bullet, and read and decrypt the symlink target
      in ->getattr() in order to report the correct st_size.  Add a function
      fscrypt_symlink_getattr() which the filesystems will call to do this.
      
      (Alternatively, we could store the decrypted size of symlinks on-disk.
      But there isn't a great place to do so, and encryption is meant to hide
      the original size to some extent; that property would be lost.)
      
      Cc: stable@vger.kernel.org
      Link: https://lore.kernel.org/r/20210702065350.209646-2-ebiggers@kernel.orgSigned-off-by: default avatarEric Biggers <ebiggers@google.com>
      d1876056
  2. 25 Jul, 2021 9 commits
  3. 24 Jul, 2021 25 commits