1. 13 Aug, 2019 19 commits
    • Eric Biggers's avatar
      fscrypt: v2 encryption policy support · 5dae460c
      Eric Biggers authored
      Add a new fscrypt policy version, "v2".  It has the following changes
      from the original policy version, which we call "v1" (*):
      
      - Master keys (the user-provided encryption keys) are only ever used as
        input to HKDF-SHA512.  This is more flexible and less error-prone, and
        it avoids the quirks and limitations of the AES-128-ECB based KDF.
        Three classes of cryptographically isolated subkeys are defined:
      
          - Per-file keys, like used in v1 policies except for the new KDF.
      
          - Per-mode keys.  These implement the semantics of the DIRECT_KEY
            flag, which for v1 policies made the master key be used directly.
            These are also planned to be used for inline encryption when
            support for it is added.
      
          - Key identifiers (see below).
      
      - Each master key is identified by a 16-byte master_key_identifier,
        which is derived from the key itself using HKDF-SHA512.  This prevents
        users from associating the wrong key with an encrypted file or
        directory.  This was easily possible with v1 policies, which
        identified the key by an arbitrary 8-byte master_key_descriptor.
      
      - The key must be provided in the filesystem-level keyring, not in a
        process-subscribed keyring.
      
      The following UAPI additions are made:
      
      - The existing ioctl FS_IOC_SET_ENCRYPTION_POLICY can now be passed a
        fscrypt_policy_v2 to set a v2 encryption policy.  It's disambiguated
        from fscrypt_policy/fscrypt_policy_v1 by the version code prefix.
      
      - A new ioctl FS_IOC_GET_ENCRYPTION_POLICY_EX is added.  It allows
        getting the v1 or v2 encryption policy of an encrypted file or
        directory.  The existing FS_IOC_GET_ENCRYPTION_POLICY ioctl could not
        be used because it did not have a way for userspace to indicate which
        policy structure is expected.  The new ioctl includes a size field, so
        it is extensible to future fscrypt policy versions.
      
      - The ioctls FS_IOC_ADD_ENCRYPTION_KEY, FS_IOC_REMOVE_ENCRYPTION_KEY,
        and FS_IOC_GET_ENCRYPTION_KEY_STATUS now support managing keys for v2
        encryption policies.  Such keys are kept logically separate from keys
        for v1 encryption policies, and are identified by 'identifier' rather
        than by 'descriptor'.  The 'identifier' need not be provided when
        adding a key, since the kernel will calculate it anyway.
      
      This patch temporarily keeps adding/removing v2 policy keys behind the
      same permission check done for adding/removing v1 policy keys:
      capable(CAP_SYS_ADMIN).  However, the next patch will carefully take
      advantage of the cryptographically secure master_key_identifier to allow
      non-root users to add/remove v2 policy keys, thus providing a full
      replacement for v1 policies.
      
      (*) Actually, in the API fscrypt_policy::version is 0 while on-disk
          fscrypt_context::format is 1.  But I believe it makes the most sense
          to advance both to '2' to have them be in sync, and to consider the
          numbering to start at 1 except for the API quirk.
      Reviewed-by: default avatarPaul Crowley <paulcrowley@google.com>
      Reviewed-by: default avatarTheodore Ts'o <tytso@mit.edu>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      5dae460c
    • Eric Biggers's avatar
      fscrypt: add an HKDF-SHA512 implementation · c1144c9b
      Eric Biggers authored
      Add an implementation of HKDF (RFC 5869) to fscrypt, for the purpose of
      deriving additional key material from the fscrypt master keys for v2
      encryption policies.  HKDF is a key derivation function built on top of
      HMAC.  We choose SHA-512 for the underlying unkeyed hash, and use an
      "hmac(sha512)" transform allocated from the crypto API.
      
      We'll be using this to replace the AES-ECB based KDF currently used to
      derive the per-file encryption keys.  While the AES-ECB based KDF is
      believed to meet the original security requirements, it is nonstandard
      and has problems that don't exist in modern KDFs such as HKDF:
      
      1. It's reversible.  Given a derived key and nonce, an attacker can
         easily compute the master key.  This is okay if the master key and
         derived keys are equally hard to compromise, but now we'd like to be
         more robust against threats such as a derived key being compromised
         through a timing attack, or a derived key for an in-use file being
         compromised after the master key has already been removed.
      
      2. It doesn't evenly distribute the entropy from the master key; each 16
         input bytes only affects the corresponding 16 output bytes.
      
      3. It isn't easily extensible to deriving other values or keys, such as
         a public hash for securely identifying the key, or per-mode keys.
         Per-mode keys will be immediately useful for Adiantum encryption, for
         which fscrypt currently uses the master key directly, introducing
         unnecessary usage constraints.  Per-mode keys will also be useful for
         hardware inline encryption, which is currently being worked on.
      
      HKDF solves all the above problems.
      Reviewed-by: default avatarPaul Crowley <paulcrowley@google.com>
      Reviewed-by: default avatarTheodore Ts'o <tytso@mit.edu>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      c1144c9b
    • Eric Biggers's avatar
      fscrypt: add FS_IOC_GET_ENCRYPTION_KEY_STATUS ioctl · 5a7e2992
      Eric Biggers authored
      Add a new fscrypt ioctl, FS_IOC_GET_ENCRYPTION_KEY_STATUS.  Given a key
      specified by 'struct fscrypt_key_specifier' (the same way a key is
      specified for the other fscrypt key management ioctls), it returns
      status information in a 'struct fscrypt_get_key_status_arg'.
      
      The main motivation for this is that applications need to be able to
      check whether an encrypted directory is "unlocked" or not, so that they
      can add the key if it is not, and avoid adding the key (which may
      involve prompting the user for a passphrase) if it already is.
      
      It's possible to use some workarounds such as checking whether opening a
      regular file fails with ENOKEY, or checking whether the filenames "look
      like gibberish" or not.  However, no workaround is usable in all cases.
      
      Like the other key management ioctls, the keyrings syscalls may seem at
      first to be a good fit for this.  Unfortunately, they are not.  Even if
      we exposed the keyring ID of the ->s_master_keys keyring and gave
      everyone Search permission on it (note: currently the keyrings
      permission system would also allow everyone to "invalidate" the keyring
      too), the fscrypt keys have an additional state that doesn't map cleanly
      to the keyrings API: the secret can be removed, but we can be still
      tracking the files that were using the key, and the removal can be
      re-attempted or the secret added again.
      
      After later patches, some applications will also need a way to determine
      whether a key was added by the current user vs. by some other user.
      Reserved fields are included in fscrypt_get_key_status_arg for this and
      other future extensions.
      Reviewed-by: default avatarTheodore Ts'o <tytso@mit.edu>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      5a7e2992
    • Eric Biggers's avatar
      fscrypt: add FS_IOC_REMOVE_ENCRYPTION_KEY ioctl · b1c0ec35
      Eric Biggers authored
      Add a new fscrypt ioctl, FS_IOC_REMOVE_ENCRYPTION_KEY.  This ioctl
      removes an encryption key that was added by FS_IOC_ADD_ENCRYPTION_KEY.
      It wipes the secret key itself, then "locks" the encrypted files and
      directories that had been unlocked using that key -- implemented by
      evicting the relevant dentries and inodes from the VFS caches.
      
      The problem this solves is that many fscrypt users want the ability to
      remove encryption keys, causing the corresponding encrypted directories
      to appear "locked" (presented in ciphertext form) again.  Moreover,
      users want removing an encryption key to *really* remove it, in the
      sense that the removed keys cannot be recovered even if kernel memory is
      compromised, e.g. by the exploit of a kernel security vulnerability or
      by a physical attack.  This is desirable after a user logs out of the
      system, for example.  In many cases users even already assume this to be
      the case and are surprised to hear when it's not.
      
      It is not sufficient to simply unlink the master key from the keyring
      (or to revoke or invalidate it), since the actual encryption transform
      objects are still pinned in memory by their inodes.  Therefore, to
      really remove a key we must also evict the relevant inodes.
      
      Currently one workaround is to run 'sync && echo 2 >
      /proc/sys/vm/drop_caches'.  But, that evicts all unused inodes in the
      system rather than just the inodes associated with the key being
      removed, causing severe performance problems.  Moreover, it requires
      root privileges, so regular users can't "lock" their encrypted files.
      
      Another workaround, used in Chromium OS kernels, is to add a new
      VFS-level ioctl FS_IOC_DROP_CACHE which is a more restricted version of
      drop_caches that operates on a single super_block.  It does:
      
              shrink_dcache_sb(sb);
              invalidate_inodes(sb, false);
      
      But it's still a hack.  Yet, the major users of filesystem encryption
      want this feature badly enough that they are actually using these hacks.
      
      To properly solve the problem, start maintaining a list of the inodes
      which have been "unlocked" using each master key.  Originally this
      wasn't possible because the kernel didn't keep track of in-use master
      keys at all.  But, with the ->s_master_keys keyring it is now possible.
      
      Then, add an ioctl FS_IOC_REMOVE_ENCRYPTION_KEY.  It finds the specified
      master key in ->s_master_keys, then wipes the secret key itself, which
      prevents any additional inodes from being unlocked with the key.  Then,
      it syncs the filesystem and evicts the inodes in the key's list.  The
      normal inode eviction code will free and wipe the per-file keys (in
      ->i_crypt_info).  Note that freeing ->i_crypt_info without evicting the
      inodes was also considered, but would have been racy.
      
      Some inodes may still be in use when a master key is removed, and we
      can't simply revoke random file descriptors, mmap's, etc.  Thus, the
      ioctl simply skips in-use inodes, and returns -EBUSY to indicate that
      some inodes weren't evicted.  The master key *secret* is still removed,
      but the fscrypt_master_key struct remains to keep track of the remaining
      inodes.  Userspace can then retry the ioctl to evict the remaining
      inodes.  Alternatively, if userspace adds the key again, the refreshed
      secret will be associated with the existing list of inodes so they
      remain correctly tracked for future key removals.
      
      The ioctl doesn't wipe pagecache pages.  Thus, we tolerate that after a
      kernel compromise some portions of plaintext file contents may still be
      recoverable from memory.  This can be solved by enabling page poisoning
      system-wide, which security conscious users may choose to do.  But it's
      very difficult to solve otherwise, e.g. note that plaintext file
      contents may have been read in other places than pagecache pages.
      
      Like FS_IOC_ADD_ENCRYPTION_KEY, FS_IOC_REMOVE_ENCRYPTION_KEY is
      initially restricted to privileged users only.  This is sufficient for
      some use cases, but not all.  A later patch will relax this restriction,
      but it will require introducing key hashes, among other changes.
      Reviewed-by: default avatarTheodore Ts'o <tytso@mit.edu>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      b1c0ec35
    • Eric Biggers's avatar
      fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl · 22d94f49
      Eric Biggers authored
      Add a new fscrypt ioctl, FS_IOC_ADD_ENCRYPTION_KEY.  This ioctl adds an
      encryption key to the filesystem's fscrypt keyring ->s_master_keys,
      making any files encrypted with that key appear "unlocked".
      
      Why we need this
      ~~~~~~~~~~~~~~~~
      
      The main problem is that the "locked/unlocked" (ciphertext/plaintext)
      status of encrypted files is global, but the fscrypt keys are not.
      fscrypt only looks for keys in the keyring(s) the process accessing the
      filesystem is subscribed to: the thread keyring, process keyring, and
      session keyring, where the session keyring may contain the user keyring.
      
      Therefore, userspace has to put fscrypt keys in the keyrings for
      individual users or sessions.  But this means that when a process with a
      different keyring tries to access encrypted files, whether they appear
      "unlocked" or not is nondeterministic.  This is because it depends on
      whether the files are currently present in the inode cache.
      
      Fixing this by consistently providing each process its own view of the
      filesystem depending on whether it has the key or not isn't feasible due
      to how the VFS caches work.  Furthermore, while sometimes users expect
      this behavior, it is misguided for two reasons.  First, it would be an
      OS-level access control mechanism largely redundant with existing access
      control mechanisms such as UNIX file permissions, ACLs, LSMs, etc.
      Encryption is actually for protecting the data at rest.
      
      Second, almost all users of fscrypt actually do need the keys to be
      global.  The largest users of fscrypt, Android and Chromium OS, achieve
      this by having PID 1 create a "session keyring" that is inherited by
      every process.  This works, but it isn't scalable because it prevents
      session keyrings from being used for any other purpose.
      
      On general-purpose Linux distros, the 'fscrypt' userspace tool [1] can't
      similarly abuse the session keyring, so to make 'sudo' work on all
      systems it has to link all the user keyrings into root's user keyring
      [2].  This is ugly and raises security concerns.  Moreover it can't make
      the keys available to system services, such as sshd trying to access the
      user's '~/.ssh' directory (see [3], [4]) or NetworkManager trying to
      read certificates from the user's home directory (see [5]); or to Docker
      containers (see [6], [7]).
      
      By having an API to add a key to the *filesystem* we'll be able to fix
      the above bugs, remove userspace workarounds, and clearly express the
      intended semantics: the locked/unlocked status of an encrypted directory
      is global, and encryption is orthogonal to OS-level access control.
      
      Why not use the add_key() syscall
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      
      We use an ioctl for this API rather than the existing add_key() system
      call because the ioctl gives us the flexibility needed to implement
      fscrypt-specific semantics that will be introduced in later patches:
      
      - Supporting key removal with the semantics such that the secret is
        removed immediately and any unused inodes using the key are evicted;
        also, the eviction of any in-use inodes can be retried.
      
      - Calculating a key-dependent cryptographic identifier and returning it
        to userspace.
      
      - Allowing keys to be added and removed by non-root users, but only keys
        for v2 encryption policies; and to prevent denial-of-service attacks,
        users can only remove keys they themselves have added, and a key is
        only really removed after all users who added it have removed it.
      
      Trying to shoehorn these semantics into the keyrings syscalls would be
      very difficult, whereas the ioctls make things much easier.
      
      However, to reuse code the implementation still uses the keyrings
      service internally.  Thus we get lockless RCU-mode key lookups without
      having to re-implement it, and the keys automatically show up in
      /proc/keys for debugging purposes.
      
      References:
      
          [1] https://github.com/google/fscrypt
          [2] https://goo.gl/55cCrI#heading=h.vf09isp98isb
          [3] https://github.com/google/fscrypt/issues/111#issuecomment-444347939
          [4] https://github.com/google/fscrypt/issues/116
          [5] https://bugs.launchpad.net/ubuntu/+source/fscrypt/+bug/1770715
          [6] https://github.com/google/fscrypt/issues/128
          [7] https://askubuntu.com/questions/1130306/cannot-run-docker-on-an-encrypted-filesystemReviewed-by: default avatarTheodore Ts'o <tytso@mit.edu>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      22d94f49
    • Eric Biggers's avatar
      fscrypt: rename keyinfo.c to keysetup.c · feed8258
      Eric Biggers authored
      Rename keyinfo.c to keysetup.c since this better describes what the file
      does (sets up the key), and it matches the new file keysetup_v1.c.
      Reviewed-by: default avatarTheodore Ts'o <tytso@mit.edu>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      feed8258
    • Eric Biggers's avatar
      fscrypt: move v1 policy key setup to keysetup_v1.c · 0109ce76
      Eric Biggers authored
      In preparation for introducing v2 encryption policies which will find
      and derive encryption keys differently from the current v1 encryption
      policies, move the v1 policy-specific key setup code from keyinfo.c into
      keysetup_v1.c.
      Reviewed-by: default avatarTheodore Ts'o <tytso@mit.edu>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      0109ce76
    • Eric Biggers's avatar
      fscrypt: refactor key setup code in preparation for v2 policies · 3ec4f2a6
      Eric Biggers authored
      Do some more refactoring of the key setup code, in preparation for
      introducing a filesystem-level keyring and v2 encryption policies:
      
      - Now that ci_inode exists, don't pass around the inode unnecessarily.
      
      - Define a function setup_file_encryption_key() which handles the crypto
        key setup given an under-construction fscrypt_info.  Don't pass the
        fscrypt_context, since everything is in the fscrypt_info.
        [This will be extended for v2 policies and the fs-level keyring.]
      
      - Define a function fscrypt_set_derived_key() which sets the per-file
        key, without depending on anything specific to v1 policies.
        [This will also be used for v2 policies.]
      
      - Define a function fscrypt_setup_v1_file_key() which takes the raw
        master key, thus separating finding the key from using it.
        [This will also be used if the key is found in the fs-level keyring.]
      Reviewed-by: default avatarTheodore Ts'o <tytso@mit.edu>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      3ec4f2a6
    • Eric Biggers's avatar
      fscrypt: rename fscrypt_master_key to fscrypt_direct_key · a828daab
      Eric Biggers authored
      In preparation for introducing a filesystem-level keyring which will
      contain fscrypt master keys, rename the existing 'struct
      fscrypt_master_key' to 'struct fscrypt_direct_key'.  This is the
      structure in the existing table of master keys that's maintained to
      deduplicate the crypto transforms for v1 DIRECT_KEY policies.
      
      I've chosen to keep this table as-is rather than make it automagically
      add/remove the keys to/from the filesystem-level keyring, since that
      would add a lot of extra complexity to the filesystem-level keyring.
      Reviewed-by: default avatarTheodore Ts'o <tytso@mit.edu>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      a828daab
    • Eric Biggers's avatar
      fscrypt: add ->ci_inode to fscrypt_info · 59dc6a8e
      Eric Biggers authored
      Add an inode back-pointer to 'struct fscrypt_info', such that
      inode->i_crypt_info->ci_inode == inode.
      
      This will be useful for:
      
      1. Evicting the inodes when a fscrypt key is removed, since we'll track
         the inodes using a given key by linking their fscrypt_infos together,
         rather than the inodes directly.  This avoids bloating 'struct inode'
         with a new list_head.
      
      2. Simplifying the per-file key setup, since the inode pointer won't
         have to be passed around everywhere just in case something goes wrong
         and it's needed for fscrypt_warn().
      Reviewed-by: default avatarTheodore Ts'o <tytso@mit.edu>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      59dc6a8e
    • Eric Biggers's avatar
      fscrypt: use FSCRYPT_* definitions, not FS_* · 3b6df59b
      Eric Biggers authored
      Update fs/crypto/ to use the new names for the UAPI constants rather
      than the old names, then make the old definitions conditional on
      !__KERNEL__.
      Reviewed-by: default avatarTheodore Ts'o <tytso@mit.edu>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      3b6df59b
    • Eric Biggers's avatar
      fscrypt: use FSCRYPT_ prefix for uapi constants · 2336d0de
      Eric Biggers authored
      Prefix all filesystem encryption UAPI constants except the ioctl numbers
      with "FSCRYPT_" rather than with "FS_".  This namespaces the constants
      more appropriately and makes it clear that they are related specifically
      to the filesystem encryption feature, and to the 'fscrypt_*' structures.
      With some of the old names like "FS_POLICY_FLAGS_VALID", it was not
      immediately clear that the constant had anything to do with encryption.
      
      This is also useful because we'll be adding more encryption-related
      constants, e.g. for the policy version, and we'd otherwise have to
      choose whether to use unclear names like FS_POLICY_V1 or inconsistent
      names like FS_ENCRYPTION_POLICY_V1.
      
      For source compatibility with existing userspace programs, keep the old
      names defined as aliases to the new names.
      
      Finally, as long as new names are being defined anyway, I skipped
      defining new names for the fscrypt mode numbers that aren't actually
      used: INVALID (0), AES_256_GCM (2), AES_256_CBC (3), SPECK128_256_XTS
      (7), and SPECK128_256_CTS (8).
      Reviewed-by: default avatarTheodore Ts'o <tytso@mit.edu>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      2336d0de
    • Eric Biggers's avatar
      fs, fscrypt: move uapi definitions to new header <linux/fscrypt.h> · 7af0ab0d
      Eric Biggers authored
      More fscrypt definitions are being added, and we shouldn't use a
      disproportionate amount of space in <linux/fs.h> for fscrypt stuff.
      So move the fscrypt definitions to a new header <linux/fscrypt.h>.
      
      For source compatibility with existing userspace programs, <linux/fs.h>
      still includes the new header.
      Reviewed-by: default avatarTheodore Ts'o <tytso@mit.edu>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      7af0ab0d
    • Eric Biggers's avatar
      fscrypt: use ENOPKG when crypto API support missing · 29a98c1c
      Eric Biggers authored
      Return ENOPKG rather than ENOENT when trying to open a file that's
      encrypted using algorithms not available in the kernel's crypto API.
      
      This avoids an ambiguity, since ENOENT is also returned when the file
      doesn't exist.
      
      Note: this is the same approach I'm taking for fs-verity.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      29a98c1c
    • Eric Biggers's avatar
      fscrypt: improve warnings for missing crypto API support · a4d14e91
      Eric Biggers authored
      Users of fscrypt with non-default algorithms will encounter an error
      like the following if they fail to include the needed algorithms into
      the crypto API when configuring the kernel (as per the documentation):
      
          Error allocating 'adiantum(xchacha12,aes)' transform: -2
      
      This requires that the user figure out what the "-2" error means.
      Make it more friendly by printing a warning like the following instead:
      
          Missing crypto API support for Adiantum (API name: "adiantum(xchacha12,aes)")
      
      Also upgrade the log level for *other* errors to KERN_ERR.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      a4d14e91
    • Eric Biggers's avatar
      fscrypt: improve warning messages for unsupported encryption contexts · 63f668f0
      Eric Biggers authored
      When fs/crypto/ encounters an inode with an invalid encryption context,
      currently it prints a warning if the pair of encryption modes are
      unrecognized, but it's silent if there are other problems such as
      unsupported context size, format, or flags.  To help people debug such
      situations, add more warning messages.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      63f668f0
    • Eric Biggers's avatar
      fscrypt: make fscrypt_msg() take inode instead of super_block · 886da8b3
      Eric Biggers authored
      Most of the warning and error messages in fs/crypto/ are for situations
      related to a specific inode, not merely to a super_block.  So to make
      things easier, make fscrypt_msg() take an inode rather than a
      super_block, and make it print the inode number.
      
      Note: This is the same approach I'm taking for fsverity_msg().
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      886da8b3
    • Eric Biggers's avatar
      fscrypt: clean up base64 encoding/decoding · 1c5100a2
      Eric Biggers authored
      Some minor cleanups for the code that base64 encodes and decodes
      encrypted filenames and long name digests:
      
      - Rename "digest_{encode,decode}()" => "base64_{encode,decode}()" since
        they are used for filenames too, not just for long name digests.
      - Replace 'while' loops with more conventional 'for' loops.
      - Use 'u8' for binary data.  Keep 'char' for string data.
      - Fully constify the lookup table (pointer was not const).
      - Improve comment.
      
      No actual change in behavior.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      1c5100a2
    • Eric Biggers's avatar
      fscrypt: remove loadable module related code · 75798f85
      Eric Biggers authored
      Since commit 643fa961 ("fscrypt: remove filesystem specific build
      config option"), fs/crypto/ can no longer be built as a loadable module.
      Thus it no longer needs a module_exit function, nor a MODULE_LICENSE.
      So remove them, and change module_init to late_initcall.
      Reviewed-by: default avatarChandan Rajendra <chandan@linux.ibm.com>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      75798f85
  2. 05 Aug, 2019 1 commit
  3. 04 Aug, 2019 10 commits
  4. 03 Aug, 2019 10 commits