[RFC,00/12] Multi-Key Total Memory Encryption API (MKTME)

Message

Alison Schofield Sept. 7, 2018, 10:23 p.m. UTC

Seeking comments on the APIs supporting MKTME on future Intel platforms.

MKTME (Multi-Key Total Memory Encryption) is a technology supporting
memory encryption on upcoming Intel platforms. Whereas TME allows
encryption of the entire system memory using a single key, MKTME
allows mulitple encryption domains, each having their own key. While 
the main use case for the feature is virtual machine isolation, the
API needs the flexibility to work for a wide range of use cases.

This RFC presents the 2 API additions that enable userspace to:
 1) Create Encryption Keys: Kernel Key Service type "mktme"
 2) Use the Encryption Keys: system call encrypt_mprotect()

In order to share between: the Kernel Key Service, the new system call,
and the existing mm code, helper functions were created in arch/x86/mktme

This patchset is built upon Kirill Shutemov's patchset for the core MKTME
support. You can find that here:
git://git.kernel.org/pub/scm/linux/kernel/git/kas/linux.git mktme/wip


Alison Schofield (12):
  docs/x86: Document the Multi-Key Total Memory Encryption API
  mm: Generalize the mprotect implementation to support extensions
  syscall/x86: Wire up a new system call for memory encryption keys
  x86/mm: Add helper functions to manage memory encryption keys
  x86/mm: Add a helper function to set keyid bits in encrypted VMA's
  mm: Add the encrypt_mprotect() system call
  x86/mm: Add helper functions to track encrypted VMA's
  mm: Track VMA's in use for each memory encryption keyid
  mm: Restrict memory encryption to anonymous VMA's
  x86/pconfig: Program memory encryption keys on a system-wide basis
  keys/mktme: Add a new key service type for memory encryption keys
  keys/mktme: Do not revoke in use memory encryption keys

 Documentation/x86/mktme-keys.txt       | 153 ++++++++++++++++
 arch/x86/Kconfig                       |   1 +
 arch/x86/entry/syscalls/syscall_32.tbl |   1 +
 arch/x86/entry/syscalls/syscall_64.tbl |   1 +
 arch/x86/include/asm/intel_pconfig.h   |  42 ++++-
 arch/x86/include/asm/mktme.h           |  21 +++
 arch/x86/mm/mktme.c                    | 141 ++++++++++++++
 fs/exec.c                              |   4 +-
 include/keys/mktme-type.h              |  28 +++
 include/linux/key.h                    |   2 +
 include/linux/mm.h                     |   9 +-
 include/linux/syscalls.h               |   2 +
 include/uapi/asm-generic/unistd.h      |   4 +-
 kernel/fork.c                          |   2 +
 kernel/sys_ni.c                        |   2 +
 mm/mmap.c                              |  12 ++
 mm/mprotect.c                          |  93 +++++++++-
 mm/nommu.c                             |   4 +
 security/keys/Kconfig                  |  11 ++
 security/keys/Makefile                 |   1 +
 security/keys/internal.h               |   6 +
 security/keys/keyctl.c                 |   7 +
 security/keys/mktme_keys.c             | 325 +++++++++++++++++++++++++++++++++
 23 files changed, 855 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/x86/mktme-keys.txt
 create mode 100644 include/keys/mktme-type.h
 create mode 100644 security/keys/mktme_keys.c

Comments

Randy Dunlap Sept. 8, 2018, 6:44 p.m. UTC | #1

On 09/07/2018 03:34 PM, Alison Schofield wrote:
> Document the API's used for MKTME on Intel platforms.
> MKTME: Multi-KEY Total Memory Encryption
> 
> Signed-off-by: Alison Schofield <[e-mail]>

Hi,
A few comments below...

> ---
>  Documentation/x86/mktme-keys.txt | 153 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 153 insertions(+)
>  create mode 100644 Documentation/x86/mktme-keys.txt
> 
> diff --git a/Documentation/x86/mktme-keys.txt b/Documentation/x86/mktme-keys.txt
> new file mode 100644
> index 000000000000..2dea7acd2a17
> --- /dev/null
> +++ b/Documentation/x86/mktme-keys.txt
> @@ -0,0 +1,153 @@
> +MKTME (Multi-Key Total Memory Encryption) is a technology that allows
> +memory encryption on Intel platforms. Whereas TME (Total Memory Encryption)
> +allows encryption of the entire system memory using a single key, MKTME
> +allows multiple encryption domains, each having their own key. The main use
> +case for the feature is virtual machine isolation. The API's introduced here
> +are intended to offer flexibility to work in a wide range of uses.
> +
> +The externally available Intel Architecture Spec:
> +https://software.intel.com/sites/default/files/managed/a5/16/Multi-Key-Total-Memory-Encryption-Spec.pdf
> +
> +============================  API Overview  ============================
> +
> +There are 2 MKTME specific API's that enable userspace to create and use
> +the memory encryption keys:
> +
> +1) Kernel Key Service: MKTME Type
> +
> +   MKTME is a new key type added to the existing Kernel Key Services
> +   to support the memory encryption keys. The MKTME service manages
> +   the addition and removal of MKTME keys. It maps userspace keys
> +   to hardware keyids and programs the hardware with user requested
> +   encryption parameters.
> +
> +   o An understanding of the Kernel Key Service is required in order
> +     to use the MKTME key type as it is a subset of that service.

reference available?

> +
> +   o MKTME keys are a limited resource. There is a single pool of
> +     MKTME keys for a system and that pool can be from 3 to 63 keys.
> +     With that in mind, userspace may take advantage of the kernel
> +     key services sharing and permissions model for userspace keys.
> +     One key can be shared as long as each user has the permission
> +     of "KEY_NEED_VIEW" to use it.
> +
> +   o MKTME key type uses capabilities to restrict the allocation
> +     of keys. It only requires CAP_SYS_RESOURCE, but will accept
> +     the broader capability of CAP_SYS_ADMIN.  See capabilities(7).
> +
> +   o The MKTME key service blocks kernel key service commands that
> +     could lead to reprogramming of in use keys, or loss of keys from
> +     the pool. This means MKTME does not allow a key to be invalidated,
> +     unlinked, or timed out. These operations are blocked by MKTME as
> +     it creates all keys with the internal flag KEY_FLAG_KEEP.
> +
> +   o MKTME does not support the keyctl option of UPDATE. Userspace
> +     may change the programming of a key by revoking it and adding
> +     a new key with the updated encryption options (or vice-versa).
> +
> +2) System Call: encrypt_mprotect()
> +
> +   MKTME encryption is requested by calling encrypt_mprotect(). The
> +   caller passes the serial number to a previously allocated and
> +   programmed encryption key. That handle was created with the MKTME
> +   Key Service.
> +
> +   o The caller must have KEY_NEED_VIEW permission on the key

                                                             key.

> +
> +   o The range of memory that is to be protected must be mapped as
> +     ANONYMOUS. If it is not, the entire encrypt_mprotect() request
> +     fails with EINVAL.
> +
> +   o As an extension to the existing mprotect() system call,
> +     encrypt_mprotect() supports the legacy mprotect behavior plus
> +     the enabling of memory encryption. That means that in addition
> +     to encrypting the memory, the protection flags will be updated
> +     as requested in the call.
> +
> +   o Additional mprotect() calls to memory already protected with
> +     MKTME will not alter the MKTME status.
> +
> +======================  Usage: MKTME Key Service  ======================
> +
> +MKTME is enabled on supported Intel platforms by selecting
> +CONFIG_X86_INTEL_MKTME which selects CONFIG_MKTME_KEYS.
> +
> +Allocating MKTME Keys via command line or system call:
> +    keyctl add mktme name "[options]" ring
> +
> +    key_serial_t add_key(const char *type, const char *description,
> +                         const void *payload, size_t plen,
> +                         key_serial_t keyring);
> +
> +Revoking MKTME Keys via command line or system call::
> +   keyctl revoke <key>
> +
> +   long keyctl(KEYCTL_REVOKE, key_serial_t key);
> +
> +Options Field Definition:
> +    userkey=      ASCII HEX value encryption key. Defaults to a CPU
> +		  generated key if a userkey is not defined here.
> +
> +    algorithm=    Encryption algorithm name as a string.
> +		  Valid algorithm: "aes-xts-128"
> +
> +    tweak=        ASCII HEX value tweak key. Tweak key will be added to the
> +                  userkey...  (need to be clear here that this is being sent
> +                  to the hardware - kernel not messing w it)

                                                          with it)

> +
> +    entropy=      ascii hex value entropy.

                     ASCII

> +                  This entropy will be used to generated the CPU key and

                                               to generate the

> +		  the tweak key when CPU generated key is requested.
> +
> +Algorithm Dependencies:
> +    AES-XTS 128 is the only supported algorithm.
> +    There are only 2 ways that AES-XTS 128 may be used:
> +
> +    1) User specified encryption key
> +	- The user specified encryption key must be exactly
> +	  16 ASCII Hex bytes (128 bits).
> +	- A tweak key must be specified and it must be exactly
> +	  16 ASCII Hex bytes (128 bits).
> +	- No entropy field is accepted.
> +
> +    2) CPU generated encryption key
> +	- When no user specified encryption key is provided, the
> +	  default encryption key will be CPU generated.
> +	- User must specify 16 ASCII Hex bytes of entropy. This 

above line ends with a space.  Please drop it.

> +	  entropy will be used by the CPU to generate both the
> +	  encryption key and the tweak key.
> +	- No entropy field is accepted.
> +
> +======================  Usage: encrypt_mprotect()  ======================
> +
> +System Call encrypt_mprotect()::
> +
> +    This system call is an extension of the existing mprotect() system
> +    call. It requires the same parameters as legary mprotect() plus

                                                legacy

> +    one additional parameter, the keyid. Userspace must provide the
> +    key serial number assigned through the kernel key service.
> +
> +    int encrypt_mprotect(void *addr, size_t len, int prot, int keyid);
> +
> +======================  Usage: Sample Roundtrip  ======================
> +
> +Sample usage of MKTME Key Service API with encrypt_mprotect() API:
> +
> +  Add a key:
> +        key = add_key(mktme, name, options, strlen(option), keyring);

                                               strlen(options),

> +
> +  Map memory:
> +        ptr = mmap(NULL, size, prot, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
> +
> +  Protect memory:
> +        ret = syscall(sys_encrypt_mprotect, ptr, size, prot, keyid);
> +
> +  Use protected memory:
> +        ................
> +
> +  Free memory:
> +        ret = munmap(ptr, size);
> +
> +  Revoke key:
> +        ret = keyctl(KEYCTL_REVOKE, key);
> +
>

Huang, Kai Sept. 10, 2018, 1:10 a.m. UTC | #2

> -----Original Message-----
> From: [e-mail] [mailto:keyrings-
> [e-mail]] On Behalf Of Alison Schofield
> Sent: Saturday, September 8, 2018 10:23 AM
> To: [e-mail]; [e-mail]
> Cc: Huang, Kai <[e-mail]>; Nakajima, Jun
> <[e-mail]>; Shutemov, Kirill <[e-mail]>;
> Hansen, Dave <[e-mail]>; Sakkinen, Jarkko
> <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]
> Subject: [RFC 00/12] Multi-Key Total Memory Encryption API (MKTME)
> 
> Seeking comments on the APIs supporting MKTME on future Intel platforms.
> 
> MKTME (Multi-Key Total Memory Encryption) is a technology supporting
> memory encryption on upcoming Intel platforms. Whereas TME allows
> encryption of the entire system memory using a single key, MKTME allows
> mulitple encryption domains, each having their own key. While the main use
> case for the feature is virtual machine isolation, the API needs the flexibility to
> work for a wide range of use cases.
> 
> This RFC presents the 2 API additions that enable userspace to:
>  1) Create Encryption Keys: Kernel Key Service type "mktme"
>  2) Use the Encryption Keys: system call encrypt_mprotect()
> 
> In order to share between: the Kernel Key Service, the new system call, and the
> existing mm code, helper functions were created in arch/x86/mktme

IMHO, we can separate this series into 2 parts, as you did above, and send out them separately. The reason is, in general I think adding new MKTME type to key retention services is not that related to memory management code, namely the encrypt_mprotect() API part.

So if we split the two parts and send them out separately, the first part can be reviewed by keyring and security guys, without involving mm guys, and the encrypt_mprotect() part can be more reviewed more by mm guys. 

And since encrypt_mprotect() is a new syscall, you may need to add more lists for the review, ie, linux-api, and maybe linux-kernel as well.

Thanks,
-Kai

> 
> This patchset is built upon Kirill Shutemov's patchset for the core MKTME
> support. You can find that here:
> git://git.kernel.org/pub/scm/linux/kernel/git/kas/linux.git mktme/wip
> 
> 
> Alison Schofield (12):
>   docs/x86: Document the Multi-Key Total Memory Encryption API
>   mm: Generalize the mprotect implementation to support extensions
>   syscall/x86: Wire up a new system call for memory encryption keys
>   x86/mm: Add helper functions to manage memory encryption keys
>   x86/mm: Add a helper function to set keyid bits in encrypted VMA's
>   mm: Add the encrypt_mprotect() system call
>   x86/mm: Add helper functions to track encrypted VMA's
>   mm: Track VMA's in use for each memory encryption keyid
>   mm: Restrict memory encryption to anonymous VMA's
>   x86/pconfig: Program memory encryption keys on a system-wide basis
>   keys/mktme: Add a new key service type for memory encryption keys
>   keys/mktme: Do not revoke in use memory encryption keys
> 
>  Documentation/x86/mktme-keys.txt       | 153 ++++++++++++++++
>  arch/x86/Kconfig                       |   1 +
>  arch/x86/entry/syscalls/syscall_32.tbl |   1 +
>  arch/x86/entry/syscalls/syscall_64.tbl |   1 +
>  arch/x86/include/asm/intel_pconfig.h   |  42 ++++-
>  arch/x86/include/asm/mktme.h           |  21 +++
>  arch/x86/mm/mktme.c                    | 141 ++++++++++++++
>  fs/exec.c                              |   4 +-
>  include/keys/mktme-type.h              |  28 +++
>  include/linux/key.h                    |   2 +
>  include/linux/mm.h                     |   9 +-
>  include/linux/syscalls.h               |   2 +
>  include/uapi/asm-generic/unistd.h      |   4 +-
>  kernel/fork.c                          |   2 +
>  kernel/sys_ni.c                        |   2 +
>  mm/mmap.c                              |  12 ++
>  mm/mprotect.c                          |  93 +++++++++-
>  mm/nommu.c                             |   4 +
>  security/keys/Kconfig                  |  11 ++
>  security/keys/Makefile                 |   1 +
>  security/keys/internal.h               |   6 +
>  security/keys/keyctl.c                 |   7 +
>  security/keys/mktme_keys.c             | 325
> +++++++++++++++++++++++++++++++++
>  23 files changed, 855 insertions(+), 17 deletions(-)  create mode 100644
> Documentation/x86/mktme-keys.txt  create mode 100644 include/keys/mktme-
> type.h  create mode 100644 security/keys/mktme_keys.c
> 
> --
> 2.14.1

Huang, Kai Sept. 10, 2018, 1:28 a.m. UTC | #3

> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]] On
> Behalf Of Alison Schofield
> Sent: Saturday, September 8, 2018 10:34 AM
> To: [e-mail]; [e-mail]
> Cc: Huang, Kai <[e-mail]>; Nakajima, Jun
> <[e-mail]>; Shutemov, Kirill <[e-mail]>;
> Hansen, Dave <[e-mail]>; Sakkinen, Jarkko
> <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]
> Subject: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory
> Encryption API
> 
> Document the API's used for MKTME on Intel platforms.
> MKTME: Multi-KEY Total Memory Encryption
> 
> Signed-off-by: Alison Schofield <[e-mail]>
> ---
>  Documentation/x86/mktme-keys.txt | 153
> +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 153 insertions(+)
>  create mode 100644 Documentation/x86/mktme-keys.txt
> 
> diff --git a/Documentation/x86/mktme-keys.txt b/Documentation/x86/mktme-
> keys.txt
> new file mode 100644
> index 000000000000..2dea7acd2a17
> --- /dev/null
> +++ b/Documentation/x86/mktme-keys.txt
> @@ -0,0 +1,153 @@
> +MKTME (Multi-Key Total Memory Encryption) is a technology that allows
> +memory encryption on Intel platforms. Whereas TME (Total Memory
> +Encryption) allows encryption of the entire system memory using a
> +single key, MKTME allows multiple encryption domains, each having their
> +own key. The main use case for the feature is virtual machine
> +isolation. The API's introduced here are intended to offer flexibility to work in a
> wide range of uses.
> +
> +The externally available Intel Architecture Spec:
> +https://software.intel.com/sites/default/files/managed/a5/16/Multi-Key-
> +Total-Memory-Encryption-Spec.pdf
> +
> +============================  API Overview
> +============================
> +
> +There are 2 MKTME specific API's that enable userspace to create and
> +use the memory encryption keys:
> +
> +1) Kernel Key Service: MKTME Type
> +
> +   MKTME is a new key type added to the existing Kernel Key Services
> +   to support the memory encryption keys. The MKTME service manages
> +   the addition and removal of MKTME keys. It maps userspace keys
> +   to hardware keyids and programs the hardware with user requested
> +   encryption parameters.
> +
> +   o An understanding of the Kernel Key Service is required in order
> +     to use the MKTME key type as it is a subset of that service.
> +
> +   o MKTME keys are a limited resource. There is a single pool of
> +     MKTME keys for a system and that pool can be from 3 to 63 keys.

Why 3 to 63 keys? Architecturally we are able to support up to 15-bit keyID, although in the first generation server we only support 6-bit keyID, which is 63 key/keyIDs (excluding keyID 0, which is TME's keyID).

> +     With that in mind, userspace may take advantage of the kernel
> +     key services sharing and permissions model for userspace keys.
> +     One key can be shared as long as each user has the permission
> +     of "KEY_NEED_VIEW" to use it.
> +
> +   o MKTME key type uses capabilities to restrict the allocation
> +     of keys. It only requires CAP_SYS_RESOURCE, but will accept
> +     the broader capability of CAP_SYS_ADMIN.  See capabilities(7).
> +
> +   o The MKTME key service blocks kernel key service commands that
> +     could lead to reprogramming of in use keys, or loss of keys from
> +     the pool. This means MKTME does not allow a key to be invalidated,
> +     unlinked, or timed out. These operations are blocked by MKTME as
> +     it creates all keys with the internal flag KEY_FLAG_KEEP.
> +
> +   o MKTME does not support the keyctl option of UPDATE. Userspace
> +     may change the programming of a key by revoking it and adding
> +     a new key with the updated encryption options (or vice-versa).
> +
> +2) System Call: encrypt_mprotect()
> +
> +   MKTME encryption is requested by calling encrypt_mprotect(). The
> +   caller passes the serial number to a previously allocated and
> +   programmed encryption key. That handle was created with the MKTME
> +   Key Service.
> +
> +   o The caller must have KEY_NEED_VIEW permission on the key
> +
> +   o The range of memory that is to be protected must be mapped as
> +     ANONYMOUS. If it is not, the entire encrypt_mprotect() request
> +     fails with EINVAL.
> +
> +   o As an extension to the existing mprotect() system call,
> +     encrypt_mprotect() supports the legacy mprotect behavior plus
> +     the enabling of memory encryption. That means that in addition
> +     to encrypting the memory, the protection flags will be updated
> +     as requested in the call.
> +
> +   o Additional mprotect() calls to memory already protected with
> +     MKTME will not alter the MKTME status.

I think it's better to separate encrypt_mprotect() into another doc so both parts can be reviewed easier.

> +
> +======================  Usage: MKTME Key Service
> +======================
> +
> +MKTME is enabled on supported Intel platforms by selecting
> +CONFIG_X86_INTEL_MKTME which selects CONFIG_MKTME_KEYS.
> +
> +Allocating MKTME Keys via command line or system call:
> +    keyctl add mktme name "[options]" ring
> +
> +    key_serial_t add_key(const char *type, const char *description,
> +                         const void *payload, size_t plen,
> +                         key_serial_t keyring);
> +
> +Revoking MKTME Keys via command line or system call::
> +   keyctl revoke <key>
> +
> +   long keyctl(KEYCTL_REVOKE, key_serial_t key);
> +
> +Options Field Definition:
> +    userkey=      ASCII HEX value encryption key. Defaults to a CPU
> +		  generated key if a userkey is not defined here.
> +
> +    algorithm=    Encryption algorithm name as a string.
> +		  Valid algorithm: "aes-xts-128"
> +
> +    tweak=        ASCII HEX value tweak key. Tweak key will be added to the
> +                  userkey...  (need to be clear here that this is being sent
> +                  to the hardware - kernel not messing w it)
> +
> +    entropy=      ascii hex value entropy.
> +                  This entropy will be used to generated the CPU key and
> +		  the tweak key when CPU generated key is requested.
> +
> +Algorithm Dependencies:
> +    AES-XTS 128 is the only supported algorithm.
> +    There are only 2 ways that AES-XTS 128 may be used:
> +
> +    1) User specified encryption key
> +	- The user specified encryption key must be exactly
> +	  16 ASCII Hex bytes (128 bits).
> +	- A tweak key must be specified and it must be exactly
> +	  16 ASCII Hex bytes (128 bits).
> +	- No entropy field is accepted.
> +
> +    2) CPU generated encryption key
> +	- When no user specified encryption key is provided, the
> +	  default encryption key will be CPU generated.
> +	- User must specify 16 ASCII Hex bytes of entropy. This
> +	  entropy will be used by the CPU to generate both the
> +	  encryption key and the tweak key.
> +	- No entropy field is accepted.

This is not true. The spec says in CPU generated random mode, both 'key' and 'tweak' part are used to generate the final key and tweak respectively.

Actually, simple 'XOR' is used to generate the final key:

case KEYID_SET_KEY_RANDOM:
	......
	(* Mix user supplied entropy to the data key and tweak key *)
	TMP_RND_DATA_KEY = TMP_RND_KEY XOR
		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_1.BYTES[15:0];
	TMP_RND_TWEAK_KEY = TMP_RND_TWEAK_KEY XOR
		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_2.BYTES[15:0];

So I think we can either just remove 'entropy' parameter, since we can use both 'userkey' and 'tweak' even for random key mode.

In fact, which might be better IMHO, we can simply disallow or ignore 'userkey' and 'tweak' part for random key mode, since if we allow user to specify both entropies, and if user passes value with all 1, we are effectively making the key and tweak to be all 1, which is not random anymore.

Instead, kernel can generate random for both entropies, or we can simply uses 0, ignoring user input.

Thanks,
-Kai
> +
> +======================  Usage: encrypt_mprotect()
> +======================
> +
> +System Call encrypt_mprotect()::
> +
> +    This system call is an extension of the existing mprotect() system
> +    call. It requires the same parameters as legary mprotect() plus
> +    one additional parameter, the keyid. Userspace must provide the
> +    key serial number assigned through the kernel key service.
> +
> +    int encrypt_mprotect(void *addr, size_t len, int prot, int keyid);
> +
> +======================  Usage: Sample Roundtrip
> ======================
> +
> +Sample usage of MKTME Key Service API with encrypt_mprotect() API:
> +
> +  Add a key:
> +        key = add_key(mktme, name, options, strlen(option), keyring);
> +
> +  Map memory:
> +        ptr = mmap(NULL, size, prot, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
> +
> +  Protect memory:
> +        ret = syscall(sys_encrypt_mprotect, ptr, size, prot, keyid);
> +
> +  Use protected memory:
> +        ................
> +
> +  Free memory:
> +        ret = munmap(ptr, size);
> +
> +  Revoke key:
> +        ret = keyctl(KEYCTL_REVOKE, key);
> +
> --
> 2.14.1

Jarkko Sakkinen Sept. 10, 2018, 5:29 p.m. UTC | #4

On Fri, 2018-09-07 at 15:23 -0700, Alison Schofield wrote:
> Seeking comments on the APIs supporting MKTME on future Intel platforms.
> 
> MKTME (Multi-Key Total Memory Encryption) is a technology supporting
> memory encryption on upcoming Intel platforms. Whereas TME allows
> encryption of the entire system memory using a single key, MKTME
> allows mulitple encryption domains, each having their own key. While 
> the main use case for the feature is virtual machine isolation, the
> API needs the flexibility to work for a wide range of use cases.

Is it a common knowledge what TME is? I kind of dropped out of the wagon
in the 2nd sentence of this paragraph.

/Jarkko

Jarkko Sakkinen Sept. 10, 2018, 5:32 p.m. UTC | #5

On Fri, 2018-09-07 at 15:34 -0700, Alison Schofield wrote:
> Document the API's used for MKTME on Intel platforms.
> MKTME: Multi-KEY Total Memory Encryption
> 
> Signed-off-by: Alison Schofield <[e-mail]>
> ---
>  Documentation/x86/mktme-keys.txt | 153
> +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 153 insertions(+)
>  create mode 100644 Documentation/x86/mktme-keys.txt
> 
> diff --git a/Documentation/x86/mktme-keys.txt b/Documentation/x86/mktme-
> keys.txt
> new file mode 100644
> index 000000000000..2dea7acd2a17
> --- /dev/null
> +++ b/Documentation/x86/mktme-keys.txt
> @@ -0,0 +1,153 @@
> +MKTME (Multi-Key Total Memory Encryption) is a technology that allows
> +memory encryption on Intel platforms. Whereas TME (Total Memory Encryption)
> +allows encryption of the entire system memory using a single key, MKTME
> +allows multiple encryption domains, each having their own key. The main use
> +case for the feature is virtual machine isolation. The API's introduced here
> +are intended to offer flexibility to work in a wide range of uses.
> +
> +The externally available Intel Architecture Spec:
> +https://software.intel.com/sites/default/files/managed/a5/16/Multi-Key-Total-
> Memory-Encryption-Spec.pdf
> +
> +============================  API Overview  ============================
> +
> +There are 2 MKTME specific API's that enable userspace to create and use
> +the memory encryption keys:

This is like saying that they are different APIs to do semantically the
same exact thing. Is that so?

/Jarkko

Jarkko Sakkinen Sept. 10, 2018, 6:02 p.m. UTC | #6

On Fri, 2018-09-07 at 15:36 -0700, Alison Schofield wrote:
> Implement memory encryption with a new system call that is an
> extension of the legacy mprotect() system call.
> 
> In encrypt_mprotect the caller must pass a handle to a previously
> allocated and programmed encryption key. Validate the key and store
> the keyid bits in the vm_page_prot for each VMA in the protection
> range.
> 
> Signed-off-by: Alison Schofield <[e-mail]>
> ---
>  fs/exec.c           |  4 ++--
>  include/linux/key.h |  2 ++
>  include/linux/mm.h  |  3 ++-
>  mm/mprotect.c       | 67 ++++++++++++++++++++++++++++++++++++++++++++++++--
> ---
>  4 files changed, 67 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/exec.c b/fs/exec.c
> index a1a246062561..b681a413db9c 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -754,8 +754,8 @@ int setup_arg_pages(struct linux_binprm *bprm,
>  	vm_flags |= mm->def_flags;
>  	vm_flags |= VM_STACK_INCOMPLETE_SETUP;
>  
> -	ret = mprotect_fixup(vma, &prev, vma->vm_start, vma->vm_end,
> -			vm_flags);
> +	ret = mprotect_fixup(vma, &prev, vma->vm_start, vma->vm_end,
> vm_flags,
> +			     -1);

Why you pass a magic number here when you went the trouble having
a named constant?

>  	if (ret)
>  		goto out_unlock;
>  	BUG_ON(prev != vma);
> diff --git a/include/linux/key.h b/include/linux/key.h
> index e58ee10f6e58..fb8a7d5f6149 100644
> --- a/include/linux/key.h
> +++ b/include/linux/key.h
> @@ -346,6 +346,8 @@ static inline key_serial_t key_serial(const struct key
> *key)
>  
>  extern void key_set_timeout(struct key *, unsigned);
>  
> +extern key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
> +				 key_perm_t perm);
>  /*
>   * The permissions required on a key that we're looking up.
>   */
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index ac85c0805761..0f9422c7841e 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1579,7 +1579,8 @@ extern unsigned long change_protection(struct
> vm_area_struct *vma, unsigned long
>  			      int dirty_accountable, int prot_numa);
>  extern int mprotect_fixup(struct vm_area_struct *vma,
>  			  struct vm_area_struct **pprev, unsigned long start,
> -			  unsigned long end, unsigned long newflags);
> +			  unsigned long end, unsigned long newflags,
> +			  int newkeyid);
>  
>  /*
>   * doesn't attempt to fault and will return short.
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 56e64ef7931e..6c2e1106525c 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -28,14 +28,17 @@
>  #include <linux/ksm.h>
>  #include <linux/uaccess.h>
>  #include <linux/mm_inline.h>
> +#include <linux/key.h>
>  #include <asm/pgtable.h>
>  #include <asm/cacheflush.h>
>  #include <asm/mmu_context.h>
>  #include <asm/tlbflush.h>
> +#include <asm/mktme.h>
>  
>  #include "internal.h"
>  
>  #define NO_PKEY  -1
> +#define NO_KEYID -1

Should have only single named constant IMHO. This ambiguity
is worse than some reasonable constant name for both cases.
Maybe NO_KEYID would be adequate?

>  
>  static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
>  		unsigned long addr, unsigned long end, pgprot_t newprot,
> @@ -310,7 +313,8 @@ unsigned long change_protection(struct vm_area_struct
> *vma, unsigned long start,
>  
>  int
>  mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
> -	unsigned long start, unsigned long end, unsigned long newflags)
> +	       unsigned long start, unsigned long end, unsigned long
> newflags,
> +	       int newkeyid)
>  {
>  	struct mm_struct *mm = vma->vm_mm;
>  	unsigned long oldflags = vma->vm_flags;
> @@ -320,10 +324,24 @@ mprotect_fixup(struct vm_area_struct *vma, struct
> vm_area_struct **pprev,
>  	int error;
>  	int dirty_accountable = 0;
>  
> +	/*
> +	 * Flags match and Keyids match or we have NO_KEYID.
> +	 * This _fixup is usually called from do_mprotect_ext() except
> +	 * for one special case: caller fs/exec.c/setup_arg_pages()
> +	 * In that case, newkeyid is passed as -1 (NO_KEYID).
> +	 */
> +	if (newflags == oldflags &&
> +	    (newkeyid == vma_keyid(vma) || newkeyid == NO_KEYID)) {
> +		*pprev = vma;
> +		return 0;
> +	}
> +	/* Flags match and Keyid changes */
>  	if (newflags == oldflags) {
> +		mprotect_set_encrypt(vma, newkeyid);
>  		*pprev = vma;
>  		return 0;
>  	}
> +	/* Flags and Keyids both change, continue. */
>  
>  	/*
>  	 * If we make a private mapping writable we increase our commit;
> @@ -373,6 +391,8 @@ mprotect_fixup(struct vm_area_struct *vma, struct
> vm_area_struct **pprev,
>  	}
>  
>  success:
> +	if (newkeyid != NO_KEYID)
> +		mprotect_set_encrypt(vma, newkeyid);
>  	/*
>  	 * vm_flags and vm_page_prot are protected by the mmap_sem
>  	 * held in write mode.
> @@ -404,10 +424,15 @@ mprotect_fixup(struct vm_area_struct *vma, struct
> vm_area_struct **pprev,
>  }
>  
>  /*
> - * When pkey==NO_PKEY we get legacy mprotect behavior here.
> + * do_mprotect_ext() supports the legacy mprotect behavior plus extensions
> + * for protection keys and memory encryption keys. These extensions are
> + * mutually exclusive and the behavior is:
> + *	(pkey==NO_PKEY && keyid==NO_KEYID) ==> legacy mprotect
> + *	(pkey is valid)  ==> legacy mprotect plus protection key extensions
> + *	(keyid is valid) ==> legacy mprotect plus encryption key extensions
>   */

The header does not follow

https://www.kernel.org/doc/Documentation/kernel-doc-nano-HOWTO.txt

>  static int do_mprotect_ext(unsigned long start, size_t len,
> -		unsigned long prot, int pkey)
> +			   unsigned long prot, int pkey, int keyid)
>  {
>  	unsigned long nstart, end, tmp, reqprot;
>  	struct vm_area_struct *vma, *prev;
> @@ -505,7 +530,8 @@ static int do_mprotect_ext(unsigned long start, size_t
> len,
>  		tmp = vma->vm_end;
>  		if (tmp > end)
>  			tmp = end;
> -		error = mprotect_fixup(vma, &prev, nstart, tmp, newflags);
> +		error = mprotect_fixup(vma, &prev, nstart, tmp, newflags,
> +				       keyid);
>  		if (error)
>  			goto out;
>  		nstart = tmp;
> @@ -530,7 +556,7 @@ static int do_mprotect_ext(unsigned long start, size_t
> len,
>  SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t, len,
>  		unsigned long, prot)
>  {
> -	return do_mprotect_ext(start, len, prot, NO_PKEY);
> +	return do_mprotect_ext(start, len, prot, NO_PKEY, NO_KEYID);
>  }
>  
>  #ifdef CONFIG_ARCH_HAS_PKEYS
> @@ -538,7 +564,7 @@ SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t,
> len,
>  SYSCALL_DEFINE4(pkey_mprotect, unsigned long, start, size_t, len,
>  		unsigned long, prot, int, pkey)
>  {
> -	return do_mprotect_ext(start, len, prot, pkey);
> +	return do_mprotect_ext(start, len, prot, pkey, NO_KEYID);
>  }
>  
>  SYSCALL_DEFINE2(pkey_alloc, unsigned long, flags, unsigned long, init_val)
> @@ -587,3 +613,32 @@ SYSCALL_DEFINE1(pkey_free, int, pkey)
>  }
>  
>  #endif /* CONFIG_ARCH_HAS_PKEYS */
> +
> +#ifdef CONFIG_X86_INTEL_MKTME
> +
> +SYSCALL_DEFINE4(encrypt_mprotect, unsigned long, start, size_t, len,
> +		unsigned long, prot, key_serial_t, serial)
> +{
> +	key_ref_t key_ref;
> +	int ret, keyid;
> +
> +	/* TODO MKTME key service must be initialized */
> +
> +	key_ref = lookup_user_key(serial, 0, KEY_NEED_VIEW);
> +	if (IS_ERR(key_ref))
> +		return PTR_ERR(key_ref);
> +
> +	mktme_map_lock();
> +	keyid = mktme_map_keyid_from_serial(serial);
> +	if (!keyid) {
> +		mktme_map_unlock();
> +		key_ref_put(key_ref);
> +		return -EINVAL;
> +	}
> +	ret = do_mprotect_ext(start, len, prot, NO_PKEY, keyid);
> +	mktme_map_unlock();
> +	key_ref_put(key_ref);
> +	return ret;
> +}
> +
> +#endif /* CONFIG_X86_INTEL_MKTME */

/Jarkko

Alison Schofield Sept. 10, 2018, 7:10 p.m. UTC | #7

On Sun, Sep 09, 2018 at 06:10:19PM -0700, Huang, Kai wrote:
> 
> > -----Original Message-----
> > From: [e-mail] [mailto:keyrings-
> > [e-mail]] On Behalf Of Alison Schofield
> > Sent: Saturday, September 8, 2018 10:23 AM
> > To: [e-mail]; [e-mail]
> > Cc: Huang, Kai <[e-mail]>; Nakajima, Jun
> > <[e-mail]>; Shutemov, Kirill <[e-mail]>;
> > Hansen, Dave <[e-mail]>; Sakkinen, Jarkko
> > <[e-mail]>; [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; [e-mail];
> > [e-mail]; [e-mail]
> > Subject: [RFC 00/12] Multi-Key Total Memory Encryption API (MKTME)
> > 
> > Seeking comments on the APIs supporting MKTME on future Intel platforms.
> > 
> > MKTME (Multi-Key Total Memory Encryption) is a technology supporting
> > memory encryption on upcoming Intel platforms. Whereas TME allows
> > encryption of the entire system memory using a single key, MKTME allows
> > mulitple encryption domains, each having their own key. While the main use
> > case for the feature is virtual machine isolation, the API needs the flexibility to
> > work for a wide range of use cases.
> > 
> > This RFC presents the 2 API additions that enable userspace to:
> >  1) Create Encryption Keys: Kernel Key Service type "mktme"
> >  2) Use the Encryption Keys: system call encrypt_mprotect()
> > 
> > In order to share between: the Kernel Key Service, the new system call, and the
> > existing mm code, helper functions were created in arch/x86/mktme
> 
> IMHO, we can separate this series into 2 parts, as you did above, and send out them separately. The reason is, in general I think adding new MKTME type to key retention services is not that related to memory management code, namely the encrypt_mprotect() API part.
> 
> So if we split the two parts and send them out separately, the first part can be reviewed by keyring and security guys, without involving mm guys, and the encrypt_mprotect() part can be more reviewed more by mm guys. 
> 

Kai,

That was the direction I had in mind at the onset: the MKTME key service
would be one patch(set) and the MKTME encrypt_mprotect() system call would
be delivered in another patch(set).

That separation falls apart when the shared structures and functions are
introduced. That 'mktme_map' (maps userspace keys to hardware keyid slots),
and the 'encrypt_count' array (counts vma's outstanding for each key) need
to be shared by both pieces. These mktme special shared structures and the
functions that operate on them are all defined in arch/x86/mm/mktme.h,.c.
From there they can be shared with the security/keys/mktme_keys.c

Once I made that separation, I stuck with it. Those structures, and any
functions that manipulate those structures live in arch/x86/mm/mktme.h,c

You noted that some of the functions that operate on the encrypt_count
might not need to be over in arch/x86/mm/mktme.c because they are not used
in the mm code. That is true.  But, then I'd be splitting up the definition
of the struct and the funcs that operate on it. So, I stuck with keeping it
all together in the arch specific mktme files.

Having said all the above, I do welcome other ideas on how to better organize
the code. 

Back to your request- to split it into smaller patchsets might look something
like:
1) the MKTME API helpers
2) the MKTME Key Service
3) the MKTME syscall encrypt_mprotect()

I'm not clear that would make anyones review life easier, than picking
the same pieces out of the greater patchset.

Suggestions welcome,
Alison

> And since encrypt_mprotect() is a new syscall, you may need to add more lists for the review, ie, linux-api, and maybe linux-kernel as well.

Got it. Will include theses in v1.

> 
> Thanks,
> -Kai
> 
> > 
> > This patchset is built upon Kirill Shutemov's patchset for the core MKTME
> > support. You can find that here:
> > git://git.kernel.org/pub/scm/linux/kernel/git/kas/linux.git mktme/wip
> > 
> > 
> > Alison Schofield (12):
> >   docs/x86: Document the Multi-Key Total Memory Encryption API
> >   mm: Generalize the mprotect implementation to support extensions
> >   syscall/x86: Wire up a new system call for memory encryption keys
> >   x86/mm: Add helper functions to manage memory encryption keys
> >   x86/mm: Add a helper function to set keyid bits in encrypted VMA's
> >   mm: Add the encrypt_mprotect() system call
> >   x86/mm: Add helper functions to track encrypted VMA's
> >   mm: Track VMA's in use for each memory encryption keyid
> >   mm: Restrict memory encryption to anonymous VMA's
> >   x86/pconfig: Program memory encryption keys on a system-wide basis
> >   keys/mktme: Add a new key service type for memory encryption keys
> >   keys/mktme: Do not revoke in use memory encryption keys
> > 
> >  Documentation/x86/mktme-keys.txt       | 153 ++++++++++++++++
> >  arch/x86/Kconfig                       |   1 +
> >  arch/x86/entry/syscalls/syscall_32.tbl |   1 +
> >  arch/x86/entry/syscalls/syscall_64.tbl |   1 +
> >  arch/x86/include/asm/intel_pconfig.h   |  42 ++++-
> >  arch/x86/include/asm/mktme.h           |  21 +++
> >  arch/x86/mm/mktme.c                    | 141 ++++++++++++++
> >  fs/exec.c                              |   4 +-
> >  include/keys/mktme-type.h              |  28 +++
> >  include/linux/key.h                    |   2 +
> >  include/linux/mm.h                     |   9 +-
> >  include/linux/syscalls.h               |   2 +
> >  include/uapi/asm-generic/unistd.h      |   4 +-
> >  kernel/fork.c                          |   2 +
> >  kernel/sys_ni.c                        |   2 +
> >  mm/mmap.c                              |  12 ++
> >  mm/mprotect.c                          |  93 +++++++++-
> >  mm/nommu.c                             |   4 +
> >  security/keys/Kconfig                  |  11 ++
> >  security/keys/Makefile                 |   1 +
> >  security/keys/internal.h               |   6 +
> >  security/keys/keyctl.c                 |   7 +
> >  security/keys/mktme_keys.c             | 325
> > +++++++++++++++++++++++++++++++++
> >  23 files changed, 855 insertions(+), 17 deletions(-)  create mode 100644
> > Documentation/x86/mktme-keys.txt  create mode 100644 include/keys/mktme-
> > type.h  create mode 100644 security/keys/mktme_keys.c
> > 
> > --
> > 2.14.1
>

Alison Schofield Sept. 11, 2018, 12:13 a.m. UTC | #8

On Sun, Sep 09, 2018 at 06:28:28PM -0700, Huang, Kai wrote:
> 
> > -----Original Message-----
> > From: [e-mail] [mailto:[e-mail]] On
> > Behalf Of Alison Schofield
> > Sent: Saturday, September 8, 2018 10:34 AM
> > To: [e-mail]; [e-mail]
> > Cc: Huang, Kai <[e-mail]>; Nakajima, Jun
> > <[e-mail]>; Shutemov, Kirill <[e-mail]>;
> > Hansen, Dave <[e-mail]>; Sakkinen, Jarkko
> > <[e-mail]>; [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; [e-mail];
> > [e-mail]; [e-mail]
> > Subject: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory
> > Encryption API
> > 
> > Document the API's used for MKTME on Intel platforms.
> > MKTME: Multi-KEY Total Memory Encryption
> > 
> > Signed-off-by: Alison Schofield <[e-mail]>
> > ---
> >  Documentation/x86/mktme-keys.txt | 153
> > +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 153 insertions(+)
> >  create mode 100644 Documentation/x86/mktme-keys.txt
> > 
> > diff --git a/Documentation/x86/mktme-keys.txt b/Documentation/x86/mktme-
> > keys.txt
> > new file mode 100644
> > index 000000000000..2dea7acd2a17
> > --- /dev/null
> > +++ b/Documentation/x86/mktme-keys.txt
> > @@ -0,0 +1,153 @@
> > +MKTME (Multi-Key Total Memory Encryption) is a technology that allows
> > +memory encryption on Intel platforms. Whereas TME (Total Memory
> > +Encryption) allows encryption of the entire system memory using a
> > +single key, MKTME allows multiple encryption domains, each having their
> > +own key. The main use case for the feature is virtual machine
> > +isolation. The API's introduced here are intended to offer flexibility to work in a
> > wide range of uses.
> > +
> > +The externally available Intel Architecture Spec:
> > +https://software.intel.com/sites/default/files/managed/a5/16/Multi-Key-
> > +Total-Memory-Encryption-Spec.pdf
> > +
> > +============================  API Overview
> > +============================
> > +
> > +There are 2 MKTME specific API's that enable userspace to create and
> > +use the memory encryption keys:
> > +
> > +1) Kernel Key Service: MKTME Type
> > +
> > +   MKTME is a new key type added to the existing Kernel Key Services
> > +   to support the memory encryption keys. The MKTME service manages
> > +   the addition and removal of MKTME keys. It maps userspace keys
> > +   to hardware keyids and programs the hardware with user requested
> > +   encryption parameters.
> > +
> > +   o An understanding of the Kernel Key Service is required in order
> > +     to use the MKTME key type as it is a subset of that service.
> > +
> > +   o MKTME keys are a limited resource. There is a single pool of
> > +     MKTME keys for a system and that pool can be from 3 to 63 keys.
> 
> Why 3 to 63 keys? Architecturally we are able to support up to 15-bit keyID, although in the first generation server we only support 6-bit keyID, which is 63 key/keyIDs (excluding keyID 0, which is TME's keyID).

My understanding is that low level SKU's could have as few as 3 bits 
available to hold the keyid, and that the max is 6 bits, hence 64.
I probably don't need to be stating that level of detail here, but
rather just iterate the important point that the resource is limited!

> 
> > +     With that in mind, userspace may take advantage of the kernel
> > +     key services sharing and permissions model for userspace keys.
> > +     One key can be shared as long as each user has the permission
> > +     of "KEY_NEED_VIEW" to use it.
> > +
> > +   o MKTME key type uses capabilities to restrict the allocation
> > +     of keys. It only requires CAP_SYS_RESOURCE, but will accept
> > +     the broader capability of CAP_SYS_ADMIN.  See capabilities(7).
> > +
> > +   o The MKTME key service blocks kernel key service commands that
> > +     could lead to reprogramming of in use keys, or loss of keys from
> > +     the pool. This means MKTME does not allow a key to be invalidated,
> > +     unlinked, or timed out. These operations are blocked by MKTME as
> > +     it creates all keys with the internal flag KEY_FLAG_KEEP.
> > +
> > +   o MKTME does not support the keyctl option of UPDATE. Userspace
> > +     may change the programming of a key by revoking it and adding
> > +     a new key with the updated encryption options (or vice-versa).
> > +
> > +2) System Call: encrypt_mprotect()
> > +
> > +   MKTME encryption is requested by calling encrypt_mprotect(). The
> > +   caller passes the serial number to a previously allocated and
> > +   programmed encryption key. That handle was created with the MKTME
> > +   Key Service.
> > +
> > +   o The caller must have KEY_NEED_VIEW permission on the key
> > +
> > +   o The range of memory that is to be protected must be mapped as
> > +     ANONYMOUS. If it is not, the entire encrypt_mprotect() request
> > +     fails with EINVAL.
> > +
> > +   o As an extension to the existing mprotect() system call,
> > +     encrypt_mprotect() supports the legacy mprotect behavior plus
> > +     the enabling of memory encryption. That means that in addition
> > +     to encrypting the memory, the protection flags will be updated
> > +     as requested in the call.
> > +
> > +   o Additional mprotect() calls to memory already protected with
> > +     MKTME will not alter the MKTME status.
> 
> I think it's better to separate encrypt_mprotect() into another doc so both parts can be reviewed easier.

I can do that.
Also, I do know I need man page for that too.
> 
> > +
> > +======================  Usage: MKTME Key Service
> > +======================
> > +
> > +MKTME is enabled on supported Intel platforms by selecting
> > +CONFIG_X86_INTEL_MKTME which selects CONFIG_MKTME_KEYS.
> > +
> > +Allocating MKTME Keys via command line or system call:
> > +    keyctl add mktme name "[options]" ring
> > +
> > +    key_serial_t add_key(const char *type, const char *description,
> > +                         const void *payload, size_t plen,
> > +                         key_serial_t keyring);
> > +
> > +Revoking MKTME Keys via command line or system call::
> > +   keyctl revoke <key>
> > +
> > +   long keyctl(KEYCTL_REVOKE, key_serial_t key);
> > +
> > +Options Field Definition:
> > +    userkey=      ASCII HEX value encryption key. Defaults to a CPU
> > +		  generated key if a userkey is not defined here.
> > +
> > +    algorithm=    Encryption algorithm name as a string.
> > +		  Valid algorithm: "aes-xts-128"
> > +
> > +    tweak=        ASCII HEX value tweak key. Tweak key will be added to the
> > +                  userkey...  (need to be clear here that this is being sent
> > +                  to the hardware - kernel not messing w it)
> > +
> > +    entropy=      ascii hex value entropy.
> > +                  This entropy will be used to generated the CPU key and
> > +		  the tweak key when CPU generated key is requested.
> > +
> > +Algorithm Dependencies:
> > +    AES-XTS 128 is the only supported algorithm.
> > +    There are only 2 ways that AES-XTS 128 may be used:
> > +
> > +    1) User specified encryption key
> > +	- The user specified encryption key must be exactly
> > +	  16 ASCII Hex bytes (128 bits).
> > +	- A tweak key must be specified and it must be exactly
> > +	  16 ASCII Hex bytes (128 bits).
> > +	- No entropy field is accepted.
> > +
> > +    2) CPU generated encryption key
> > +	- When no user specified encryption key is provided, the
> > +	  default encryption key will be CPU generated.
> > +	- User must specify 16 ASCII Hex bytes of entropy. This
> > +	  entropy will be used by the CPU to generate both the
> > +	  encryption key and the tweak key.
> > +	- No entropy field is accepted.
             ^^^^^^^ should be tweak

> 
> This is not true. The spec says in CPU generated random mode, both 'key' and 'tweak' part are used to generate the final key and tweak respectively.
> 
> Actually, simple 'XOR' is used to generate the final key:
> 
> case KEYID_SET_KEY_RANDOM:
> 	......
> 	(* Mix user supplied entropy to the data key and tweak key *)
> 	TMP_RND_DATA_KEY = TMP_RND_KEY XOR
> 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_1.BYTES[15:0];
> 	TMP_RND_TWEAK_KEY = TMP_RND_TWEAK_KEY XOR
> 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_2.BYTES[15:0];
> 
> So I think we can either just remove 'entropy' parameter, since we can use both 'userkey' and 'tweak' even for random key mode.
> 
> In fact, which might be better IMHO, we can simply disallow or ignore 'userkey' and 'tweak' part for random key mode, since if we allow user to specify both entropies, and if user passes value with all 1, we are effectively making the key and tweak to be all 1, which is not random anymore.
> 
> Instead, kernel can generate random for both entropies, or we can simply uses 0, ignoring user input.

Kai,
I think my typo above, threw you off. We have the same understanding of
the key fields.

Is this the structure you are suggesting?

	Options

	key_type=	"user" or "CPU"

	key=		If key_type == user
				key= is the data key
			If key_type == CPU
				key= is not required
				if key= is present
					it is entropy to be mixed with
					CPU generated data key

	tweak=		If key_type == user
				tweak= is the tweak key
			If key_type == CPU
				tweak= is not required
				if tweak= is present
					it is entropy to be mixed with
					CPU generated tweak key


Alison
> 
> Thanks,
> -Kai

........snip...........

Huang, Kai Sept. 11, 2018, 12:14 a.m. UTC | #9

> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]] On
> Behalf Of Huang, Kai
> Sent: Monday, September 10, 2018 1:28 PM
> To: Schofield, Alison <[e-mail]>; [e-mail];
> [e-mail]
> Cc: Nakajima, Jun <[e-mail]>; Shutemov, Kirill
> <[e-mail]>; Hansen, Dave <[e-mail]>;
> Sakkinen, Jarkko <[e-mail]>; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail]; [e-mail]
> Subject: RE: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory
> Encryption API
> 
> 
> > -----Original Message-----
> > From: [e-mail] [mailto:[e-mail]] On
> > Behalf Of Alison Schofield
> > Sent: Saturday, September 8, 2018 10:34 AM
> > To: [e-mail]; [e-mail]
> > Cc: Huang, Kai <[e-mail]>; Nakajima, Jun
> > <[e-mail]>; Shutemov, Kirill
> > <[e-mail]>; Hansen, Dave <[e-mail]>;
> > Sakkinen, Jarkko <[e-mail]>; [e-mail];
> > [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; [e-mail]; [e-mail]
> > Subject: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory
> > Encryption API
> >
> > Document the API's used for MKTME on Intel platforms.
> > MKTME: Multi-KEY Total Memory Encryption
> >
> > Signed-off-by: Alison Schofield <[e-mail]>
> > ---
> >  Documentation/x86/mktme-keys.txt | 153
> > +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 153 insertions(+)
> >  create mode 100644 Documentation/x86/mktme-keys.txt
> >
> > diff --git a/Documentation/x86/mktme-keys.txt
> > b/Documentation/x86/mktme- keys.txt new file mode 100644 index
> > 000000000000..2dea7acd2a17
> > --- /dev/null
> > +++ b/Documentation/x86/mktme-keys.txt
> > @@ -0,0 +1,153 @@
> > +MKTME (Multi-Key Total Memory Encryption) is a technology that allows
> > +memory encryption on Intel platforms. Whereas TME (Total Memory
> > +Encryption) allows encryption of the entire system memory using a
> > +single key, MKTME allows multiple encryption domains, each having
> > +their own key. The main use case for the feature is virtual machine
> > +isolation. The API's introduced here are intended to offer
> > +flexibility to work in a
> > wide range of uses.
> > +
> > +The externally available Intel Architecture Spec:
> > +https://software.intel.com/sites/default/files/managed/a5/16/Multi-Ke
> > +y-
> > +Total-Memory-Encryption-Spec.pdf
> > +
> > +============================  API Overview
> > +============================
> > +
> > +There are 2 MKTME specific API's that enable userspace to create and
> > +use the memory encryption keys:
> > +
> > +1) Kernel Key Service: MKTME Type
> > +
> > +   MKTME is a new key type added to the existing Kernel Key Services
> > +   to support the memory encryption keys. The MKTME service manages
> > +   the addition and removal of MKTME keys. It maps userspace keys
> > +   to hardware keyids and programs the hardware with user requested
> > +   encryption parameters.
> > +
> > +   o An understanding of the Kernel Key Service is required in order
> > +     to use the MKTME key type as it is a subset of that service.
> > +
> > +   o MKTME keys are a limited resource. There is a single pool of
> > +     MKTME keys for a system and that pool can be from 3 to 63 keys.
> 
> Why 3 to 63 keys? Architecturally we are able to support up to 15-bit keyID,
> although in the first generation server we only support 6-bit keyID, which is 63
> key/keyIDs (excluding keyID 0, which is TME's keyID).
> 
> > +     With that in mind, userspace may take advantage of the kernel
> > +     key services sharing and permissions model for userspace keys.
> > +     One key can be shared as long as each user has the permission
> > +     of "KEY_NEED_VIEW" to use it.
> > +
> > +   o MKTME key type uses capabilities to restrict the allocation
> > +     of keys. It only requires CAP_SYS_RESOURCE, but will accept
> > +     the broader capability of CAP_SYS_ADMIN.  See capabilities(7).
> > +
> > +   o The MKTME key service blocks kernel key service commands that
> > +     could lead to reprogramming of in use keys, or loss of keys from
> > +     the pool. This means MKTME does not allow a key to be invalidated,
> > +     unlinked, or timed out. These operations are blocked by MKTME as
> > +     it creates all keys with the internal flag KEY_FLAG_KEEP.
> > +
> > +   o MKTME does not support the keyctl option of UPDATE. Userspace
> > +     may change the programming of a key by revoking it and adding
> > +     a new key with the updated encryption options (or vice-versa).
> > +
> > +2) System Call: encrypt_mprotect()
> > +
> > +   MKTME encryption is requested by calling encrypt_mprotect(). The
> > +   caller passes the serial number to a previously allocated and
> > +   programmed encryption key. That handle was created with the MKTME
> > +   Key Service.
> > +
> > +   o The caller must have KEY_NEED_VIEW permission on the key
> > +
> > +   o The range of memory that is to be protected must be mapped as
> > +     ANONYMOUS. If it is not, the entire encrypt_mprotect() request
> > +     fails with EINVAL.
> > +
> > +   o As an extension to the existing mprotect() system call,
> > +     encrypt_mprotect() supports the legacy mprotect behavior plus
> > +     the enabling of memory encryption. That means that in addition
> > +     to encrypting the memory, the protection flags will be updated
> > +     as requested in the call.
> > +
> > +   o Additional mprotect() calls to memory already protected with
> > +     MKTME will not alter the MKTME status.
> 
> I think it's better to separate encrypt_mprotect() into another doc so both parts
> can be reviewed easier.
> 
> > +
> > +======================  Usage: MKTME Key Service
> > +======================
> > +
> > +MKTME is enabled on supported Intel platforms by selecting
> > +CONFIG_X86_INTEL_MKTME which selects CONFIG_MKTME_KEYS.
> > +
> > +Allocating MKTME Keys via command line or system call:
> > +    keyctl add mktme name "[options]" ring
> > +
> > +    key_serial_t add_key(const char *type, const char *description,
> > +                         const void *payload, size_t plen,
> > +                         key_serial_t keyring);
> > +
> > +Revoking MKTME Keys via command line or system call::
> > +   keyctl revoke <key>
> > +
> > +   long keyctl(KEYCTL_REVOKE, key_serial_t key);
> > +
> > +Options Field Definition:
> > +    userkey=      ASCII HEX value encryption key. Defaults to a CPU
> > +		  generated key if a userkey is not defined here.
> > +
> > +    algorithm=    Encryption algorithm name as a string.
> > +		  Valid algorithm: "aes-xts-128"
> > +
> > +    tweak=        ASCII HEX value tweak key. Tweak key will be added to the
> > +                  userkey...  (need to be clear here that this is being sent
> > +                  to the hardware - kernel not messing w it)
> > +
> > +    entropy=      ascii hex value entropy.
> > +                  This entropy will be used to generated the CPU key and
> > +		  the tweak key when CPU generated key is requested.
> > +
> > +Algorithm Dependencies:
> > +    AES-XTS 128 is the only supported algorithm.
> > +    There are only 2 ways that AES-XTS 128 may be used:
> > +
> > +    1) User specified encryption key
> > +	- The user specified encryption key must be exactly
> > +	  16 ASCII Hex bytes (128 bits).
> > +	- A tweak key must be specified and it must be exactly
> > +	  16 ASCII Hex bytes (128 bits).
> > +	- No entropy field is accepted.
> > +
> > +    2) CPU generated encryption key
> > +	- When no user specified encryption key is provided, the
> > +	  default encryption key will be CPU generated.
> > +	- User must specify 16 ASCII Hex bytes of entropy. This
> > +	  entropy will be used by the CPU to generate both the
> > +	  encryption key and the tweak key.
> > +	- No entropy field is accepted.
> 
> This is not true. The spec says in CPU generated random mode, both 'key' and
> 'tweak' part are used to generate the final key and tweak respectively.
> 
> Actually, simple 'XOR' is used to generate the final key:
> 
> case KEYID_SET_KEY_RANDOM:
> 	......
> 	(* Mix user supplied entropy to the data key and tweak key *)
> 	TMP_RND_DATA_KEY = TMP_RND_KEY XOR
> 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_1.BYTES[15:0];
> 	TMP_RND_TWEAK_KEY = TMP_RND_TWEAK_KEY XOR
> 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_2.BYTES[15:0];
> 
> So I think we can either just remove 'entropy' parameter, since we can use both
> 'userkey' and 'tweak' even for random key mode.
> 
> In fact, which might be better IMHO, we can simply disallow or ignore 'userkey'
> and 'tweak' part for random key mode, since if we allow user to specify both
> entropies, and if user passes value with all 1, we are effectively making the key
> and tweak to be all 1, which is not random anymore.
> 
> Instead, kernel can generate random for both entropies, or we can simply uses 0,
> ignoring user input.

Hmm.. I just realized that I confused 'OR' and 'XOR', so the last two paragraphs doesn't make sense. :)

But still I think we can simply use "userkey" and "tweak". "entropy" is not needed IMO.

Thanks,
-Kai
> 
> Thanks,
> -Kai
> > +
> > +======================  Usage: encrypt_mprotect()
> > +======================
> > +
> > +System Call encrypt_mprotect()::
> > +
> > +    This system call is an extension of the existing mprotect() system
> > +    call. It requires the same parameters as legary mprotect() plus
> > +    one additional parameter, the keyid. Userspace must provide the
> > +    key serial number assigned through the kernel key service.
> > +
> > +    int encrypt_mprotect(void *addr, size_t len, int prot, int
> > + keyid);
> > +
> > +======================  Usage: Sample Roundtrip
> > ======================
> > +
> > +Sample usage of MKTME Key Service API with encrypt_mprotect() API:
> > +
> > +  Add a key:
> > +        key = add_key(mktme, name, options, strlen(option), keyring);
> > +
> > +  Map memory:
> > +        ptr = mmap(NULL, size, prot, MAP_ANONYMOUS|MAP_PRIVATE, -1,
> > + 0);
> > +
> > +  Protect memory:
> > +        ret = syscall(sys_encrypt_mprotect, ptr, size, prot, keyid);
> > +
> > +  Use protected memory:
> > +        ................
> > +
> > +  Free memory:
> > +        ret = munmap(ptr, size);
> > +
> > +  Revoke key:
> > +        ret = keyctl(KEYCTL_REVOKE, key);
> > +
> > --
> > 2.14.1

Alison Schofield Sept. 11, 2018, 12:19 a.m. UTC | #10

On Mon, Sep 10, 2018 at 10:32:20AM -0700, Sakkinen, Jarkko wrote:
> On Fri, 2018-09-07 at 15:34 -0700, Alison Schofield wrote:
> > Document the API's used for MKTME on Intel platforms.
> > MKTME: Multi-KEY Total Memory Encryption
> > 
> > Signed-off-by: Alison Schofield <[e-mail]>
> > ---
> >  Documentation/x86/mktme-keys.txt | 153
> > +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 153 insertions(+)
> >  create mode 100644 Documentation/x86/mktme-keys.txt
> > 
> > diff --git a/Documentation/x86/mktme-keys.txt b/Documentation/x86/mktme-
> > keys.txt
> > new file mode 100644
> > index 000000000000..2dea7acd2a17
> > --- /dev/null
> > +++ b/Documentation/x86/mktme-keys.txt
> > @@ -0,0 +1,153 @@
> > +MKTME (Multi-Key Total Memory Encryption) is a technology that allows
> > +memory encryption on Intel platforms. Whereas TME (Total Memory Encryption)
> > +allows encryption of the entire system memory using a single key, MKTME
> > +allows multiple encryption domains, each having their own key. The main use
> > +case for the feature is virtual machine isolation. The API's introduced here
> > +are intended to offer flexibility to work in a wide range of uses.
> > +
> > +The externally available Intel Architecture Spec:
> > +https://software.intel.com/sites/default/files/managed/a5/16/Multi-Key-Total-
> > Memory-Encryption-Spec.pdf
> > +
> > +============================  API Overview  ============================
> > +
> > +There are 2 MKTME specific API's that enable userspace to create and use
> > +the memory encryption keys:
> 
> This is like saying that they are different APIs to do semantically the
> same exact thing. Is that so?

No. 
The API's used to create and use memory encryption keys are described below:

> 
> /Jarkko

Huang, Kai Sept. 11, 2018, 12:33 a.m. UTC | #11

> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]] On
> Behalf Of Alison Schofield
> Sent: Tuesday, September 11, 2018 12:13 PM
> To: Huang, Kai <[e-mail]>
> Cc: [e-mail]; [e-mail]; Nakajima, Jun
> <[e-mail]>; Shutemov, Kirill <[e-mail]>;
> Hansen, Dave <[e-mail]>; Sakkinen, Jarkko
> <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]
> Subject: Re: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory
> Encryption API
> 
> On Sun, Sep 09, 2018 at 06:28:28PM -0700, Huang, Kai wrote:
> >
> > > -----Original Message-----
> > > From: [e-mail] [mailto:[e-mail]] On
> > > Behalf Of Alison Schofield
> > > Sent: Saturday, September 8, 2018 10:34 AM
> > > To: [e-mail]; [e-mail]
> > > Cc: Huang, Kai <[e-mail]>; Nakajima, Jun
> > > <[e-mail]>; Shutemov, Kirill
> > > <[e-mail]>; Hansen, Dave <[e-mail]>;
> > > Sakkinen, Jarkko <[e-mail]>; [e-mail];
> > > [e-mail]; [e-mail];
> > > [e-mail]; [e-mail]; [e-mail]; linux-
> [e-mail]
> > > Subject: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory
> > > Encryption API
> > >
> > > Document the API's used for MKTME on Intel platforms.
> > > MKTME: Multi-KEY Total Memory Encryption
> > >
> > > Signed-off-by: Alison Schofield <[e-mail]>
> > > ---
> > >  Documentation/x86/mktme-keys.txt | 153
> > > +++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 153 insertions(+)
> > >  create mode 100644 Documentation/x86/mktme-keys.txt
> > >
> > > diff --git a/Documentation/x86/mktme-keys.txt
> > > b/Documentation/x86/mktme- keys.txt new file mode 100644 index
> > > 000000000000..2dea7acd2a17
> > > --- /dev/null
> > > +++ b/Documentation/x86/mktme-keys.txt
> > > @@ -0,0 +1,153 @@
> > > +MKTME (Multi-Key Total Memory Encryption) is a technology that
> > > +allows memory encryption on Intel platforms. Whereas TME (Total
> > > +Memory
> > > +Encryption) allows encryption of the entire system memory using a
> > > +single key, MKTME allows multiple encryption domains, each having
> > > +their own key. The main use case for the feature is virtual machine
> > > +isolation. The API's introduced here are intended to offer
> > > +flexibility to work in a
> > > wide range of uses.
> > > +
> > > +The externally available Intel Architecture Spec:
> > > +https://software.intel.com/sites/default/files/managed/a5/16/Multi-
> > > +Key-
> > > +Total-Memory-Encryption-Spec.pdf
> > > +
> > > +============================  API Overview
> > > +============================
> > > +
> > > +There are 2 MKTME specific API's that enable userspace to create
> > > +and use the memory encryption keys:
> > > +
> > > +1) Kernel Key Service: MKTME Type
> > > +
> > > +   MKTME is a new key type added to the existing Kernel Key Services
> > > +   to support the memory encryption keys. The MKTME service manages
> > > +   the addition and removal of MKTME keys. It maps userspace keys
> > > +   to hardware keyids and programs the hardware with user requested
> > > +   encryption parameters.
> > > +
> > > +   o An understanding of the Kernel Key Service is required in order
> > > +     to use the MKTME key type as it is a subset of that service.
> > > +
> > > +   o MKTME keys are a limited resource. There is a single pool of
> > > +     MKTME keys for a system and that pool can be from 3 to 63 keys.
> >
> > Why 3 to 63 keys? Architecturally we are able to support up to 15-bit keyID,
> although in the first generation server we only support 6-bit keyID, which is 63
> key/keyIDs (excluding keyID 0, which is TME's keyID).
> 
> My understanding is that low level SKU's could have as few as 3 bits available to
> hold the keyid, and that the max is 6 bits, hence 64.
> I probably don't need to be stating that level of detail here, but rather just
> iterate the important point that the resource is limited!
> 
> >
> > > +     With that in mind, userspace may take advantage of the kernel
> > > +     key services sharing and permissions model for userspace keys.
> > > +     One key can be shared as long as each user has the permission
> > > +     of "KEY_NEED_VIEW" to use it.
> > > +
> > > +   o MKTME key type uses capabilities to restrict the allocation
> > > +     of keys. It only requires CAP_SYS_RESOURCE, but will accept
> > > +     the broader capability of CAP_SYS_ADMIN.  See capabilities(7).
> > > +
> > > +   o The MKTME key service blocks kernel key service commands that
> > > +     could lead to reprogramming of in use keys, or loss of keys from
> > > +     the pool. This means MKTME does not allow a key to be invalidated,
> > > +     unlinked, or timed out. These operations are blocked by MKTME as
> > > +     it creates all keys with the internal flag KEY_FLAG_KEEP.
> > > +
> > > +   o MKTME does not support the keyctl option of UPDATE. Userspace
> > > +     may change the programming of a key by revoking it and adding
> > > +     a new key with the updated encryption options (or vice-versa).
> > > +
> > > +2) System Call: encrypt_mprotect()
> > > +
> > > +   MKTME encryption is requested by calling encrypt_mprotect(). The
> > > +   caller passes the serial number to a previously allocated and
> > > +   programmed encryption key. That handle was created with the MKTME
> > > +   Key Service.
> > > +
> > > +   o The caller must have KEY_NEED_VIEW permission on the key
> > > +
> > > +   o The range of memory that is to be protected must be mapped as
> > > +     ANONYMOUS. If it is not, the entire encrypt_mprotect() request
> > > +     fails with EINVAL.
> > > +
> > > +   o As an extension to the existing mprotect() system call,
> > > +     encrypt_mprotect() supports the legacy mprotect behavior plus
> > > +     the enabling of memory encryption. That means that in addition
> > > +     to encrypting the memory, the protection flags will be updated
> > > +     as requested in the call.
> > > +
> > > +   o Additional mprotect() calls to memory already protected with
> > > +     MKTME will not alter the MKTME status.
> >
> > I think it's better to separate encrypt_mprotect() into another doc so both
> parts can be reviewed easier.
> 
> I can do that.
> Also, I do know I need man page for that too.
> >
> > > +
> > > +======================  Usage: MKTME Key Service
> > > +======================
> > > +
> > > +MKTME is enabled on supported Intel platforms by selecting
> > > +CONFIG_X86_INTEL_MKTME which selects CONFIG_MKTME_KEYS.
> > > +
> > > +Allocating MKTME Keys via command line or system call:
> > > +    keyctl add mktme name "[options]" ring
> > > +
> > > +    key_serial_t add_key(const char *type, const char *description,
> > > +                         const void *payload, size_t plen,
> > > +                         key_serial_t keyring);
> > > +
> > > +Revoking MKTME Keys via command line or system call::
> > > +   keyctl revoke <key>
> > > +
> > > +   long keyctl(KEYCTL_REVOKE, key_serial_t key);
> > > +
> > > +Options Field Definition:
> > > +    userkey=      ASCII HEX value encryption key. Defaults to a CPU
> > > +		  generated key if a userkey is not defined here.
> > > +
> > > +    algorithm=    Encryption algorithm name as a string.
> > > +		  Valid algorithm: "aes-xts-128"
> > > +
> > > +    tweak=        ASCII HEX value tweak key. Tweak key will be added to the
> > > +                  userkey...  (need to be clear here that this is being sent
> > > +                  to the hardware - kernel not messing w it)
> > > +
> > > +    entropy=      ascii hex value entropy.
> > > +                  This entropy will be used to generated the CPU key and
> > > +		  the tweak key when CPU generated key is requested.
> > > +
> > > +Algorithm Dependencies:
> > > +    AES-XTS 128 is the only supported algorithm.
> > > +    There are only 2 ways that AES-XTS 128 may be used:
> > > +
> > > +    1) User specified encryption key
> > > +	- The user specified encryption key must be exactly
> > > +	  16 ASCII Hex bytes (128 bits).
> > > +	- A tweak key must be specified and it must be exactly
> > > +	  16 ASCII Hex bytes (128 bits).
> > > +	- No entropy field is accepted.
> > > +
> > > +    2) CPU generated encryption key
> > > +	- When no user specified encryption key is provided, the
> > > +	  default encryption key will be CPU generated.
> > > +	- User must specify 16 ASCII Hex bytes of entropy. This
> > > +	  entropy will be used by the CPU to generate both the
> > > +	  encryption key and the tweak key.
> > > +	- No entropy field is accepted.
>              ^^^^^^^ should be tweak
> 
> >
> > This is not true. The spec says in CPU generated random mode, both 'key' and
> 'tweak' part are used to generate the final key and tweak respectively.
> >
> > Actually, simple 'XOR' is used to generate the final key:
> >
> > case KEYID_SET_KEY_RANDOM:
> > 	......
> > 	(* Mix user supplied entropy to the data key and tweak key *)
> > 	TMP_RND_DATA_KEY = TMP_RND_KEY XOR
> > 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_1.BYTES[15:0];
> > 	TMP_RND_TWEAK_KEY = TMP_RND_TWEAK_KEY XOR
> > 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_2.BYTES[15:0];
> >
> > So I think we can either just remove 'entropy' parameter, since we can use
> both 'userkey' and 'tweak' even for random key mode.
> >
> > In fact, which might be better IMHO, we can simply disallow or ignore
> 'userkey' and 'tweak' part for random key mode, since if we allow user to specify
> both entropies, and if user passes value with all 1, we are effectively making the
> key and tweak to be all 1, which is not random anymore.
> >
> > Instead, kernel can generate random for both entropies, or we can simply uses
> 0, ignoring user input.
> 
> Kai,
> I think my typo above, threw you off. We have the same understanding of the
> key fields.
> 
> Is this the structure you are suggesting?
> 
> 	Options
> 
> 	key_type=	"user" or "CPU"
> 
> 	key=		If key_type == user
> 				key= is the data key
> 			If key_type == CPU
> 				key= is not required
> 				if key= is present
> 					it is entropy to be mixed with
> 					CPU generated data key
> 
> 	tweak=		If key_type == user
> 				tweak= is the tweak key
> 			If key_type == CPU
> 				tweak= is not required
> 				if tweak= is present
> 					it is entropy to be mixed with
> 					CPU generated tweak key

Exactly.

Although I am not sure whether we should support other 2 modes: Clear key  and  no encryption;

Thanks,
-Kai
> 
> 
> Alison
> >
> > Thanks,
> > -Kai
> 
> ........snip...........

Alison Schofield Sept. 11, 2018, 12:45 a.m. UTC | #12

On Mon, Sep 10, 2018 at 05:33:33PM -0700, Huang, Kai wrote:
> > -----Original Message-----
> > From: [e-mail] [mailto:[e-mail]] On
> > Behalf Of Alison Schofield
> > Sent: Tuesday, September 11, 2018 12:13 PM
> > To: Huang, Kai <[e-mail]>
> > Cc: [e-mail]; [e-mail]; Nakajima, Jun
> > <[e-mail]>; Shutemov, Kirill <[e-mail]>;
> > Hansen, Dave <[e-mail]>; Sakkinen, Jarkko
> > <[e-mail]>; [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; [e-mail];
> > [e-mail]; [e-mail]
> > Subject: Re: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory
> > Encryption API
> > 
> > On Sun, Sep 09, 2018 at 06:28:28PM -0700, Huang, Kai wrote:
> > >
> > > > -----Original Message-----
> > > > From: [e-mail] [mailto:[e-mail]] On
> > > > Behalf Of Alison Schofield
> > > > Sent: Saturday, September 8, 2018 10:34 AM
> > > > To: [e-mail]; [e-mail]
> > > > Cc: Huang, Kai <[e-mail]>; Nakajima, Jun
> > > > <[e-mail]>; Shutemov, Kirill
> > > > <[e-mail]>; Hansen, Dave <[e-mail]>;
> > > > Sakkinen, Jarkko <[e-mail]>; [e-mail];
> > > > [e-mail]; [e-mail];
> > > > [e-mail]; [e-mail]; [e-mail]; linux-
> > [e-mail]
> > > > Subject: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory
> > > > Encryption API
> > > >
> > > > Document the API's used for MKTME on Intel platforms.
> > > > MKTME: Multi-KEY Total Memory Encryption
> > > >
> > > > Signed-off-by: Alison Schofield <[e-mail]>
> > > > ---
> > > >  Documentation/x86/mktme-keys.txt | 153
> > > > +++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 153 insertions(+)
> > > >  create mode 100644 Documentation/x86/mktme-keys.txt
> > > >
> > > > diff --git a/Documentation/x86/mktme-keys.txt
> > > > b/Documentation/x86/mktme- keys.txt new file mode 100644 index
> > > > 000000000000..2dea7acd2a17
> > > > --- /dev/null
> > > > +++ b/Documentation/x86/mktme-keys.txt
> > > > @@ -0,0 +1,153 @@
> > > > +MKTME (Multi-Key Total Memory Encryption) is a technology that
> > > > +allows memory encryption on Intel platforms. Whereas TME (Total
> > > > +Memory
> > > > +Encryption) allows encryption of the entire system memory using a
> > > > +single key, MKTME allows multiple encryption domains, each having
> > > > +their own key. The main use case for the feature is virtual machine
> > > > +isolation. The API's introduced here are intended to offer
> > > > +flexibility to work in a
> > > > wide range of uses.
> > > > +
> > > > +The externally available Intel Architecture Spec:
> > > > +https://software.intel.com/sites/default/files/managed/a5/16/Multi-
> > > > +Key-
> > > > +Total-Memory-Encryption-Spec.pdf
> > > > +
> > > > +============================  API Overview
> > > > +============================
> > > > +
> > > > +There are 2 MKTME specific API's that enable userspace to create
> > > > +and use the memory encryption keys:
> > > > +
> > > > +1) Kernel Key Service: MKTME Type
> > > > +
> > > > +   MKTME is a new key type added to the existing Kernel Key Services
> > > > +   to support the memory encryption keys. The MKTME service manages
> > > > +   the addition and removal of MKTME keys. It maps userspace keys
> > > > +   to hardware keyids and programs the hardware with user requested
> > > > +   encryption parameters.
> > > > +
> > > > +   o An understanding of the Kernel Key Service is required in order
> > > > +     to use the MKTME key type as it is a subset of that service.
> > > > +
> > > > +   o MKTME keys are a limited resource. There is a single pool of
> > > > +     MKTME keys for a system and that pool can be from 3 to 63 keys.
> > >
> > > Why 3 to 63 keys? Architecturally we are able to support up to 15-bit keyID,
> > although in the first generation server we only support 6-bit keyID, which is 63
> > key/keyIDs (excluding keyID 0, which is TME's keyID).
> > 
> > My understanding is that low level SKU's could have as few as 3 bits available to
> > hold the keyid, and that the max is 6 bits, hence 64.
> > I probably don't need to be stating that level of detail here, but rather just
> > iterate the important point that the resource is limited!
> > 
> > >
> > > > +     With that in mind, userspace may take advantage of the kernel
> > > > +     key services sharing and permissions model for userspace keys.
> > > > +     One key can be shared as long as each user has the permission
> > > > +     of "KEY_NEED_VIEW" to use it.
> > > > +
> > > > +   o MKTME key type uses capabilities to restrict the allocation
> > > > +     of keys. It only requires CAP_SYS_RESOURCE, but will accept
> > > > +     the broader capability of CAP_SYS_ADMIN.  See capabilities(7).
> > > > +
> > > > +   o The MKTME key service blocks kernel key service commands that
> > > > +     could lead to reprogramming of in use keys, or loss of keys from
> > > > +     the pool. This means MKTME does not allow a key to be invalidated,
> > > > +     unlinked, or timed out. These operations are blocked by MKTME as
> > > > +     it creates all keys with the internal flag KEY_FLAG_KEEP.
> > > > +
> > > > +   o MKTME does not support the keyctl option of UPDATE. Userspace
> > > > +     may change the programming of a key by revoking it and adding
> > > > +     a new key with the updated encryption options (or vice-versa).
> > > > +
> > > > +2) System Call: encrypt_mprotect()
> > > > +
> > > > +   MKTME encryption is requested by calling encrypt_mprotect(). The
> > > > +   caller passes the serial number to a previously allocated and
> > > > +   programmed encryption key. That handle was created with the MKTME
> > > > +   Key Service.
> > > > +
> > > > +   o The caller must have KEY_NEED_VIEW permission on the key
> > > > +
> > > > +   o The range of memory that is to be protected must be mapped as
> > > > +     ANONYMOUS. If it is not, the entire encrypt_mprotect() request
> > > > +     fails with EINVAL.
> > > > +
> > > > +   o As an extension to the existing mprotect() system call,
> > > > +     encrypt_mprotect() supports the legacy mprotect behavior plus
> > > > +     the enabling of memory encryption. That means that in addition
> > > > +     to encrypting the memory, the protection flags will be updated
> > > > +     as requested in the call.
> > > > +
> > > > +   o Additional mprotect() calls to memory already protected with
> > > > +     MKTME will not alter the MKTME status.
> > >
> > > I think it's better to separate encrypt_mprotect() into another doc so both
> > parts can be reviewed easier.
> > 
> > I can do that.
> > Also, I do know I need man page for that too.
> > >
> > > > +
> > > > +======================  Usage: MKTME Key Service
> > > > +======================
> > > > +
> > > > +MKTME is enabled on supported Intel platforms by selecting
> > > > +CONFIG_X86_INTEL_MKTME which selects CONFIG_MKTME_KEYS.
> > > > +
> > > > +Allocating MKTME Keys via command line or system call:
> > > > +    keyctl add mktme name "[options]" ring
> > > > +
> > > > +    key_serial_t add_key(const char *type, const char *description,
> > > > +                         const void *payload, size_t plen,
> > > > +                         key_serial_t keyring);
> > > > +
> > > > +Revoking MKTME Keys via command line or system call::
> > > > +   keyctl revoke <key>
> > > > +
> > > > +   long keyctl(KEYCTL_REVOKE, key_serial_t key);
> > > > +
> > > > +Options Field Definition:
> > > > +    userkey=      ASCII HEX value encryption key. Defaults to a CPU
> > > > +		  generated key if a userkey is not defined here.
> > > > +
> > > > +    algorithm=    Encryption algorithm name as a string.
> > > > +		  Valid algorithm: "aes-xts-128"
> > > > +
> > > > +    tweak=        ASCII HEX value tweak key. Tweak key will be added to the
> > > > +                  userkey...  (need to be clear here that this is being sent
> > > > +                  to the hardware - kernel not messing w it)
> > > > +
> > > > +    entropy=      ascii hex value entropy.
> > > > +                  This entropy will be used to generated the CPU key and
> > > > +		  the tweak key when CPU generated key is requested.
> > > > +
> > > > +Algorithm Dependencies:
> > > > +    AES-XTS 128 is the only supported algorithm.
> > > > +    There are only 2 ways that AES-XTS 128 may be used:
> > > > +
> > > > +    1) User specified encryption key
> > > > +	- The user specified encryption key must be exactly
> > > > +	  16 ASCII Hex bytes (128 bits).
> > > > +	- A tweak key must be specified and it must be exactly
> > > > +	  16 ASCII Hex bytes (128 bits).
> > > > +	- No entropy field is accepted.
> > > > +
> > > > +    2) CPU generated encryption key
> > > > +	- When no user specified encryption key is provided, the
> > > > +	  default encryption key will be CPU generated.
> > > > +	- User must specify 16 ASCII Hex bytes of entropy. This
> > > > +	  entropy will be used by the CPU to generate both the
> > > > +	  encryption key and the tweak key.
> > > > +	- No entropy field is accepted.
> >              ^^^^^^^ should be tweak
> > 
> > >
> > > This is not true. The spec says in CPU generated random mode, both 'key' and
> > 'tweak' part are used to generate the final key and tweak respectively.
> > >
> > > Actually, simple 'XOR' is used to generate the final key:
> > >
> > > case KEYID_SET_KEY_RANDOM:
> > > 	......
> > > 	(* Mix user supplied entropy to the data key and tweak key *)
> > > 	TMP_RND_DATA_KEY = TMP_RND_KEY XOR
> > > 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_1.BYTES[15:0];
> > > 	TMP_RND_TWEAK_KEY = TMP_RND_TWEAK_KEY XOR
> > > 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_2.BYTES[15:0];
> > >
> > > So I think we can either just remove 'entropy' parameter, since we can use
> > both 'userkey' and 'tweak' even for random key mode.
> > >
> > > In fact, which might be better IMHO, we can simply disallow or ignore
> > 'userkey' and 'tweak' part for random key mode, since if we allow user to specify
> > both entropies, and if user passes value with all 1, we are effectively making the
> > key and tweak to be all 1, which is not random anymore.
> > >
> > > Instead, kernel can generate random for both entropies, or we can simply uses
> > 0, ignoring user input.
> > 
> > Kai,
> > I think my typo above, threw you off. We have the same understanding of the
> > key fields.
> > 
> > Is this the structure you are suggesting?
> > 
> > 	Options
> > 
> > 	key_type=	"user" or "CPU"
> > 
> > 	key=		If key_type == user
> > 				key= is the data key
> > 			If key_type == CPU
> > 				key= is not required
> > 				if key= is present
> > 					it is entropy to be mixed with
> > 					CPU generated data key
> > 
> > 	tweak=		If key_type == user
> > 				tweak= is the tweak key
> > 			If key_type == CPU
> > 				tweak= is not required
> > 				if tweak= is present
> > 					it is entropy to be mixed with
> > 					CPU generated tweak key
> 
> Exactly.
> 
> Although I am not sure whether we should support other 2 modes: Clear key  and  no encryption;

A hardware key does get CLEAR'ed when the userspace key is revoked.
I don't think we identified any other user directed need to clear a key.

The no encryption option is currently considered not a requirement.
That means, although you see it in the Intel HW Spec, we don't have
use case that is driving us to implement it.

For other's info - no encryption would be an option where the key
tells the hardware not to do any encryption at all on this piece of memory.
All of memory not encrypted with these MKTME keys, is by default, encrypted
with the system level TME, Total Memory Encryption algorithm. (OK - not
really *all*, there is also a BIOS settable exclusion zone for TME)

> 
> Thanks,
> -Kai
> > 
> > 
> > Alison
> > >
> > > Thanks,
> > > -Kai
> > 
> > ........snip...........
>

Huang, Kai Sept. 11, 2018, 1:14 a.m. UTC | #13

On Mon, 2018-09-10 at 17:45 -0700, Alison Schofield wrote:
> On Mon, Sep 10, 2018 at 05:33:33PM -0700, Huang, Kai wrote:
> > > -----Original Message-----
> > > From: [e-mail] [mailto:[e-mail]]
> > > On
> > > Behalf Of Alison Schofield
> > > Sent: Tuesday, September 11, 2018 12:13 PM
> > > To: Huang, Kai <[e-mail]>
> > > Cc: [e-mail]; [e-mail]; Nakajima, Jun
> > > <[e-mail]>; Shutemov, Kirill <kirill.shutemov@intel
> > > .com>;
> > > Hansen, Dave <[e-mail]>; Sakkinen, Jarkko
> > > <[e-mail]>; [e-mail]; [e-mail]
> > > nel.org;
> > > [e-mail]; [e-mail]; hpa@zyto
> > > r.com;
> > > [e-mail]; [e-mail]
> > > Subject: Re: [RFC 01/12] docs/x86: Document the Multi-Key Total
> > > Memory
> > > Encryption API
> > > 
> > > On Sun, Sep 09, 2018 at 06:28:28PM -0700, Huang, Kai wrote:
> > > > 
> > > > > -----Original Message-----
> > > > > From: [e-mail] [mailto:owner-linux-mm@kvack.o
> > > > > rg] On
> > > > > Behalf Of Alison Schofield
> > > > > Sent: Saturday, September 8, 2018 10:34 AM
> > > > > To: [e-mail]; [e-mail]
> > > > > Cc: Huang, Kai <[e-mail]>; Nakajima, Jun
> > > > > <[e-mail]>; Shutemov, Kirill
> > > > > <[e-mail]>; Hansen, Dave <dave.hansen@intel.
> > > > > com>;
> > > > > Sakkinen, Jarkko <[e-mail]>; jmorris@namei.o
> > > > > rg;
> > > > > [e-mail]; [e-mail].o
> > > > > rg;
> > > > > [e-mail]; [e-mail]; [e-mail]; linux-
> > > 
> > > [e-mail]
> > > > > Subject: [RFC 01/12] docs/x86: Document the Multi-Key Total
> > > > > Memory
> > > > > Encryption API
> > > > > 
> > > > > Document the API's used for MKTME on Intel platforms.
> > > > > MKTME: Multi-KEY Total Memory Encryption
> > > > > 
> > > > > Signed-off-by: Alison Schofield <[e-mail]>
> > > > > ---
> > > > >  Documentation/x86/mktme-keys.txt | 153
> > > > > +++++++++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 153 insertions(+)
> > > > >  create mode 100644 Documentation/x86/mktme-keys.txt
> > > > > 
> > > > > diff --git a/Documentation/x86/mktme-keys.txt
> > > > > b/Documentation/x86/mktme- keys.txt new file mode 100644
> > > > > index
> > > > > 000000000000..2dea7acd2a17
> > > > > --- /dev/null
> > > > > +++ b/Documentation/x86/mktme-keys.txt
> > > > > @@ -0,0 +1,153 @@
> > > > > +MKTME (Multi-Key Total Memory Encryption) is a technology
> > > > > that
> > > > > +allows memory encryption on Intel platforms. Whereas TME
> > > > > (Total
> > > > > +Memory
> > > > > +Encryption) allows encryption of the entire system memory
> > > > > using a
> > > > > +single key, MKTME allows multiple encryption domains, each
> > > > > having
> > > > > +their own key. The main use case for the feature is virtual
> > > > > machine
> > > > > +isolation. The API's introduced here are intended to offer
> > > > > +flexibility to work in a
> > > > > wide range of uses.
> > > > > +
> > > > > +The externally available Intel Architecture Spec:
> > > > > +https://software.intel.com/sites/default/files/managed/a5/16
> > > > > /Multi-
> > > > > +Key-
> > > > > +Total-Memory-Encryption-Spec.pdf
> > > > > +
> > > > > +============================  API Overview
> > > > > +============================
> > > > > +
> > > > > +There are 2 MKTME specific API's that enable userspace to
> > > > > create
> > > > > +and use the memory encryption keys:
> > > > > +
> > > > > +1) Kernel Key Service: MKTME Type
> > > > > +
> > > > > +   MKTME is a new key type added to the existing Kernel Key
> > > > > Services
> > > > > +   to support the memory encryption keys. The MKTME service
> > > > > manages
> > > > > +   the addition and removal of MKTME keys. It maps userspace
> > > > > keys
> > > > > +   to hardware keyids and programs the hardware with user
> > > > > requested
> > > > > +   encryption parameters.
> > > > > +
> > > > > +   o An understanding of the Kernel Key Service is required
> > > > > in order
> > > > > +     to use the MKTME key type as it is a subset of that
> > > > > service.
> > > > > +
> > > > > +   o MKTME keys are a limited resource. There is a single
> > > > > pool of
> > > > > +     MKTME keys for a system and that pool can be from 3 to
> > > > > 63 keys.
> > > > 
> > > > Why 3 to 63 keys? Architecturally we are able to support up to
> > > > 15-bit keyID,
> > > 
> > > although in the first generation server we only support 6-bit
> > > keyID, which is 63
> > > key/keyIDs (excluding keyID 0, which is TME's keyID).
> > > 
> > > My understanding is that low level SKU's could have as few as 3
> > > bits available to
> > > hold the keyid, and that the max is 6 bits, hence 64.
> > > I probably don't need to be stating that level of detail here,
> > > but rather just
> > > iterate the important point that the resource is limited!
> > > 
> > > > 
> > > > > +     With that in mind, userspace may take advantage of the
> > > > > kernel
> > > > > +     key services sharing and permissions model for
> > > > > userspace keys.
> > > > > +     One key can be shared as long as each user has the
> > > > > permission
> > > > > +     of "KEY_NEED_VIEW" to use it.
> > > > > +
> > > > > +   o MKTME key type uses capabilities to restrict the
> > > > > allocation
> > > > > +     of keys. It only requires CAP_SYS_RESOURCE, but will
> > > > > accept
> > > > > +     the broader capability of CAP_SYS_ADMIN.  See
> > > > > capabilities(7).
> > > > > +
> > > > > +   o The MKTME key service blocks kernel key service
> > > > > commands that
> > > > > +     could lead to reprogramming of in use keys, or loss of
> > > > > keys from
> > > > > +     the pool. This means MKTME does not allow a key to be
> > > > > invalidated,
> > > > > +     unlinked, or timed out. These operations are blocked by
> > > > > MKTME as
> > > > > +     it creates all keys with the internal flag
> > > > > KEY_FLAG_KEEP.
> > > > > +
> > > > > +   o MKTME does not support the keyctl option of UPDATE.
> > > > > Userspace
> > > > > +     may change the programming of a key by revoking it and
> > > > > adding
> > > > > +     a new key with the updated encryption options (or vice-
> > > > > versa).
> > > > > +
> > > > > +2) System Call: encrypt_mprotect()
> > > > > +
> > > > > +   MKTME encryption is requested by calling
> > > > > encrypt_mprotect(). The
> > > > > +   caller passes the serial number to a previously allocated
> > > > > and
> > > > > +   programmed encryption key. That handle was created with
> > > > > the MKTME
> > > > > +   Key Service.
> > > > > +
> > > > > +   o The caller must have KEY_NEED_VIEW permission on the
> > > > > key
> > > > > +
> > > > > +   o The range of memory that is to be protected must be
> > > > > mapped as
> > > > > +     ANONYMOUS. If it is not, the entire encrypt_mprotect()
> > > > > request
> > > > > +     fails with EINVAL.
> > > > > +
> > > > > +   o As an extension to the existing mprotect() system call,
> > > > > +     encrypt_mprotect() supports the legacy mprotect
> > > > > behavior plus
> > > > > +     the enabling of memory encryption. That means that in
> > > > > addition
> > > > > +     to encrypting the memory, the protection flags will be
> > > > > updated
> > > > > +     as requested in the call.
> > > > > +
> > > > > +   o Additional mprotect() calls to memory already protected
> > > > > with
> > > > > +     MKTME will not alter the MKTME status.
> > > > 
> > > > I think it's better to separate encrypt_mprotect() into another
> > > > doc so both
> > > 
> > > parts can be reviewed easier.
> > > 
> > > I can do that.
> > > Also, I do know I need man page for that too.
> > > > 
> > > > > +
> > > > > +======================  Usage: MKTME Key Service
> > > > > +======================
> > > > > +
> > > > > +MKTME is enabled on supported Intel platforms by selecting
> > > > > +CONFIG_X86_INTEL_MKTME which selects CONFIG_MKTME_KEYS.
> > > > > +
> > > > > +Allocating MKTME Keys via command line or system call:
> > > > > +    keyctl add mktme name "[options]" ring
> > > > > +
> > > > > +    key_serial_t add_key(const char *type, const char
> > > > > *description,
> > > > > +                         const void *payload, size_t plen,
> > > > > +                         key_serial_t keyring);
> > > > > +
> > > > > +Revoking MKTME Keys via command line or system call::
> > > > > +   keyctl revoke <key>
> > > > > +
> > > > > +   long keyctl(KEYCTL_REVOKE, key_serial_t key);
> > > > > +
> > > > > +Options Field Definition:
> > > > > +    userkey=      ASCII HEX value encryption key. Defaults
> > > > > to a CPU
> > > > > +		  generated key if a userkey is not defined
> > > > > here.
> > > > > +
> > > > > +    algorithm=    Encryption algorithm name as a string.
> > > > > +		  Valid algorithm: "aes-xts-128"
> > > > > +
> > > > > +    tweak=        ASCII HEX value tweak key. Tweak key will
> > > > > be added to the
> > > > > +                  userkey...  (need to be clear here that
> > > > > this is being sent
> > > > > +                  to the hardware - kernel not messing w it)
> > > > > +
> > > > > +    entropy=      ascii hex value entropy.
> > > > > +                  This entropy will be used to generated the
> > > > > CPU key and
> > > > > +		  the tweak key when CPU generated key is
> > > > > requested.
> > > > > +
> > > > > +Algorithm Dependencies:
> > > > > +    AES-XTS 128 is the only supported algorithm.
> > > > > +    There are only 2 ways that AES-XTS 128 may be used:
> > > > > +
> > > > > +    1) User specified encryption key
> > > > > +	- The user specified encryption key must be exactly
> > > > > +	  16 ASCII Hex bytes (128 bits).
> > > > > +	- A tweak key must be specified and it must be
> > > > > exactly
> > > > > +	  16 ASCII Hex bytes (128 bits).
> > > > > +	- No entropy field is accepted.
> > > > > +
> > > > > +    2) CPU generated encryption key
> > > > > +	- When no user specified encryption key is provided,
> > > > > the
> > > > > +	  default encryption key will be CPU generated.
> > > > > +	- User must specify 16 ASCII Hex bytes of entropy.
> > > > > This
> > > > > +	  entropy will be used by the CPU to generate both
> > > > > the
> > > > > +	  encryption key and the tweak key.
> > > > > +	- No entropy field is accepted.
> > > 
> > >              ^^^^^^^ should be tweak
> > > 
> > > > 
> > > > This is not true. The spec says in CPU generated random mode,
> > > > both 'key' and
> > > 
> > > 'tweak' part are used to generate the final key and tweak
> > > respectively.
> > > > 
> > > > Actually, simple 'XOR' is used to generate the final key:
> > > > 
> > > > case KEYID_SET_KEY_RANDOM:
> > > > 	......
> > > > 	(* Mix user supplied entropy to the data key and tweak
> > > > key *)
> > > > 	TMP_RND_DATA_KEY = TMP_RND_KEY XOR
> > > > 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_1.BYTES[15:0];
> > > > 	TMP_RND_TWEAK_KEY = TMP_RND_TWEAK_KEY XOR
> > > > 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_2.BYTES[15:0];
> > > > 
> > > > So I think we can either just remove 'entropy' parameter, since
> > > > we can use
> > > 
> > > both 'userkey' and 'tweak' even for random key mode.
> > > > 
> > > > In fact, which might be better IMHO, we can simply disallow or
> > > > ignore
> > > 
> > > 'userkey' and 'tweak' part for random key mode, since if we allow
> > > user to specify
> > > both entropies, and if user passes value with all 1, we are
> > > effectively making the
> > > key and tweak to be all 1, which is not random anymore.
> > > > 
> > > > Instead, kernel can generate random for both entropies, or we
> > > > can simply uses
> > > 
> > > 0, ignoring user input.
> > > 
> > > Kai,
> > > I think my typo above, threw you off. We have the same
> > > understanding of the
> > > key fields.
> > > 
> > > Is this the structure you are suggesting?
> > > 
> > > 	Options
> > > 
> > > 	key_type=	"user" or "CPU"
> > > 
> > > 	key=		If key_type == user
> > > 				key= is the data key
> > > 			If key_type == CPU
> > > 				key= is not required
> > > 				if key= is present
> > > 					it is entropy to be mixed with
> > > 					CPU generated data key
> > > 
> > > 	tweak=		If key_type == user
> > > 				tweak= is the tweak key
> > > 			If key_type == CPU
> > > 				tweak= is not required
> > > 				if tweak= is present
> > > 					it is entropy to be mixed with
> > > 					CPU generated tweak key
> > 
> > Exactly.
> > 
> > Although I am not sure whether we should support other 2 modes:
> > Clear key  and  no encryption;
> 
> A hardware key does get CLEAR'ed when the userspace key is revoked.
> I don't think we identified any other user directed need to clear a
> key.
> 
> The no encryption option is currently considered not a requirement.
> That means, although you see it in the Intel HW Spec, we don't have
> use case that is driving us to implement it.
> 
> For other's info - no encryption would be an option where the key
> tells the hardware not to do any encryption at all on this piece of
> memory.
> All of memory not encrypted with these MKTME keys, is by default,
> encrypted
> with the system level TME, Total Memory Encryption algorithm. (OK -
> not
> really *all*, there is also a BIOS settable exclusion zone for TME)

Agreed. Let's focus on user mode and CPU mode for now.

Thanks,
-Kai
> 
> > 
> > Thanks,
> > -Kai
> > > 
> > > 
> > > Alison
> > > > 
> > > > Thanks,
> > > > -Kai
> > > 
> > > ........snip...........

Alison Schofield Sept. 11, 2018, 2:15 a.m. UTC | #14

On Mon, Sep 10, 2018 at 09:02:43PM +0300, Jarkko Sakkinen wrote:
> On Fri, 2018-09-07 at 15:36 -0700, Alison Schofield wrote:
> > Implement memory encryption with a new system call that is an
> > extension of the legacy mprotect() system call.
> > 
> > In encrypt_mprotect the caller must pass a handle to a previously
> > allocated and programmed encryption key. Validate the key and store
> > the keyid bits in the vm_page_prot for each VMA in the protection
> > range.
> > 
> > Signed-off-by: Alison Schofield <[e-mail]>
> > ---
> >  fs/exec.c           |  4 ++--
> >  include/linux/key.h |  2 ++
> >  include/linux/mm.h  |  3 ++-
> >  mm/mprotect.c       | 67 ++++++++++++++++++++++++++++++++++++++++++++++++--
> > ---
> >  4 files changed, 67 insertions(+), 9 deletions(-)
> > 
> > diff --git a/fs/exec.c b/fs/exec.c
> > index a1a246062561..b681a413db9c 100644
> > --- a/fs/exec.c
> > +++ b/fs/exec.c
> > @@ -754,8 +754,8 @@ int setup_arg_pages(struct linux_binprm *bprm,
> >  	vm_flags |= mm->def_flags;
> >  	vm_flags |= VM_STACK_INCOMPLETE_SETUP;
> >  
> > -	ret = mprotect_fixup(vma, &prev, vma->vm_start, vma->vm_end,
> > -			vm_flags);
> > +	ret = mprotect_fixup(vma, &prev, vma->vm_start, vma->vm_end,
> > vm_flags,
> > +			     -1);
> 
> Why you pass a magic number here when you went the trouble having
> a named constant?

The named constant isn't available over here in fs/exec.c.
The special case is documented further down ... (scroll down)
That caller has no knowledge of proctection or encryption keys.

> 
> >  	if (ret)
> >  		goto out_unlock;
> >  	BUG_ON(prev != vma);
> > diff --git a/include/linux/key.h b/include/linux/key.h
> > index e58ee10f6e58..fb8a7d5f6149 100644
> > --- a/include/linux/key.h
> > +++ b/include/linux/key.h
> > @@ -346,6 +346,8 @@ static inline key_serial_t key_serial(const struct key
> > *key)
> >  
> >  extern void key_set_timeout(struct key *, unsigned);
> >  
> > +extern key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
> > +				 key_perm_t perm);
> >  /*
> >   * The permissions required on a key that we're looking up.
> >   */
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index ac85c0805761..0f9422c7841e 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -1579,7 +1579,8 @@ extern unsigned long change_protection(struct
> > vm_area_struct *vma, unsigned long
> >  			      int dirty_accountable, int prot_numa);
> >  extern int mprotect_fixup(struct vm_area_struct *vma,
> >  			  struct vm_area_struct **pprev, unsigned long start,
> > -			  unsigned long end, unsigned long newflags);
> > +			  unsigned long end, unsigned long newflags,
> > +			  int newkeyid);
> >  
> >  /*
> >   * doesn't attempt to fault and will return short.
> > diff --git a/mm/mprotect.c b/mm/mprotect.c
> > index 56e64ef7931e..6c2e1106525c 100644
> > --- a/mm/mprotect.c
> > +++ b/mm/mprotect.c
> > @@ -28,14 +28,17 @@
> >  #include <linux/ksm.h>
> >  #include <linux/uaccess.h>
> >  #include <linux/mm_inline.h>
> > +#include <linux/key.h>
> >  #include <asm/pgtable.h>
> >  #include <asm/cacheflush.h>
> >  #include <asm/mmu_context.h>
> >  #include <asm/tlbflush.h>
> > +#include <asm/mktme.h>
> >  
> >  #include "internal.h"
> >  
> >  #define NO_PKEY  -1
> > +#define NO_KEYID -1
> 
> Should have only single named constant IMHO. This ambiguity
> is worse than some reasonable constant name for both cases.
> Maybe NO_KEYID would be adequate?

Yes, this could be NO_KEY for both. I was worried about readability,
but most of the usages compare it to a well-named variable, as in 
(pkey == NO_KEY) or (keyid == NO_KEY) so it seems to work. 
Will do!

> 
> >  
> >  static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
> >  		unsigned long addr, unsigned long end, pgprot_t newprot,
> > @@ -310,7 +313,8 @@ unsigned long change_protection(struct vm_area_struct
> > *vma, unsigned long start,
> >  
> >  int
> >  mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
> > -	unsigned long start, unsigned long end, unsigned long newflags)
> > +	       unsigned long start, unsigned long end, unsigned long
> > newflags,
> > +	       int newkeyid)
> >  {
> >  	struct mm_struct *mm = vma->vm_mm;
> >  	unsigned long oldflags = vma->vm_flags;
> > @@ -320,10 +324,24 @@ mprotect_fixup(struct vm_area_struct *vma, struct
> > vm_area_struct **pprev,
> >  	int error;
> >  	int dirty_accountable = 0;
> >  
> > +	/*
> > +	 * Flags match and Keyids match or we have NO_KEYID.
> > +	 * This _fixup is usually called from do_mprotect_ext() except
> > +	 * for one special case: caller fs/exec.c/setup_arg_pages()
> > +	 * In that case, newkeyid is passed as -1 (NO_KEYID).
> > +	 */

Above is that special case explanation.

> > +	if (newflags == oldflags &&
> > +	    (newkeyid == vma_keyid(vma) || newkeyid == NO_KEYID)) {
> > +		*pprev = vma;
> > +		return 0;
> > +	}
> > +	/* Flags match and Keyid changes */
> >  	if (newflags == oldflags) {
> > +		mprotect_set_encrypt(vma, newkeyid);
> >  		*pprev = vma;
> >  		return 0;
> >  	}
> > +	/* Flags and Keyids both change, continue. */
> >  
> >  	/*
> >  	 * If we make a private mapping writable we increase our commit;
> > @@ -373,6 +391,8 @@ mprotect_fixup(struct vm_area_struct *vma, struct
> > vm_area_struct **pprev,
> >  	}
> >  
> >  success:
> > +	if (newkeyid != NO_KEYID)
> > +		mprotect_set_encrypt(vma, newkeyid);
> >  	/*
> >  	 * vm_flags and vm_page_prot are protected by the mmap_sem
> >  	 * held in write mode.
> > @@ -404,10 +424,15 @@ mprotect_fixup(struct vm_area_struct *vma, struct
> > vm_area_struct **pprev,
> >  }
> >  
> >  /*
> > - * When pkey==NO_PKEY we get legacy mprotect behavior here.
> > + * do_mprotect_ext() supports the legacy mprotect behavior plus extensions
> > + * for protection keys and memory encryption keys. These extensions are
> > + * mutually exclusive and the behavior is:
> > + *	(pkey==NO_PKEY && keyid==NO_KEYID) ==> legacy mprotect
> > + *	(pkey is valid)  ==> legacy mprotect plus protection key extensions
> > + *	(keyid is valid) ==> legacy mprotect plus encryption key extensions
> >   */
> 
> The header does not follow
> 
> https://www.kernel.org/doc/Documentation/kernel-doc-nano-HOWTO.txt

Jarkko,

I see the "how to" on kernel doc formatting, but I'm not getting the
when? When should it be done? I can imagine it for new code, like
the mktme_keys.c where new code is being added and the kernel docs
could give a complete narrative of the module.

Here, in mprotect.c, there are no kernel doc comments. I realize
sometime we just need to start somewhere ;), but I find it odd to
pluck out one function header to be extracted for kernel docs.

Alison

> 
> >  static int do_mprotect_ext(unsigned long start, size_t len,
> > -		unsigned long prot, int pkey)
> > +			   unsigned long prot, int pkey, int keyid)
> >  {
> >  	unsigned long nstart, end, tmp, reqprot;
> >  	struct vm_area_struct *vma, *prev;
> > @@ -505,7 +530,8 @@ static int do_mprotect_ext(unsigned long start, size_t
> > len,
> >  		tmp = vma->vm_end;
> >  		if (tmp > end)
> >  			tmp = end;
> > -		error = mprotect_fixup(vma, &prev, nstart, tmp, newflags);
> > +		error = mprotect_fixup(vma, &prev, nstart, tmp, newflags,
> > +				       keyid);
> >  		if (error)
> >  			goto out;
> >  		nstart = tmp;
> > @@ -530,7 +556,7 @@ static int do_mprotect_ext(unsigned long start, size_t
> > len,
> >  SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t, len,
> >  		unsigned long, prot)
> >  {
> > -	return do_mprotect_ext(start, len, prot, NO_PKEY);
> > +	return do_mprotect_ext(start, len, prot, NO_PKEY, NO_KEYID);
> >  }
> >  
> >  #ifdef CONFIG_ARCH_HAS_PKEYS
> > @@ -538,7 +564,7 @@ SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t,
> > len,
> >  SYSCALL_DEFINE4(pkey_mprotect, unsigned long, start, size_t, len,
> >  		unsigned long, prot, int, pkey)
> >  {
> > -	return do_mprotect_ext(start, len, prot, pkey);
> > +	return do_mprotect_ext(start, len, prot, pkey, NO_KEYID);
> >  }
> >  
> >  SYSCALL_DEFINE2(pkey_alloc, unsigned long, flags, unsigned long, init_val)
> > @@ -587,3 +613,32 @@ SYSCALL_DEFINE1(pkey_free, int, pkey)
> >  }
> >  
> >  #endif /* CONFIG_ARCH_HAS_PKEYS */
> > +
> > +#ifdef CONFIG_X86_INTEL_MKTME
> > +
> > +SYSCALL_DEFINE4(encrypt_mprotect, unsigned long, start, size_t, len,
> > +		unsigned long, prot, key_serial_t, serial)
> > +{
> > +	key_ref_t key_ref;
> > +	int ret, keyid;
> > +
> > +	/* TODO MKTME key service must be initialized */
> > +
> > +	key_ref = lookup_user_key(serial, 0, KEY_NEED_VIEW);
> > +	if (IS_ERR(key_ref))
> > +		return PTR_ERR(key_ref);
> > +
> > +	mktme_map_lock();
> > +	keyid = mktme_map_keyid_from_serial(serial);
> > +	if (!keyid) {
> > +		mktme_map_unlock();
> > +		key_ref_put(key_ref);
> > +		return -EINVAL;
> > +	}
> > +	ret = do_mprotect_ext(start, len, prot, NO_PKEY, keyid);
> > +	mktme_map_unlock();
> > +	key_ref_put(key_ref);
> > +	return ret;
> > +}
> > +
> > +#endif /* CONFIG_X86_INTEL_MKTME */
> 
> /Jarkko

Huang, Kai Sept. 11, 2018, 3:15 a.m. UTC | #15

On Mon, 2018-09-10 at 12:10 -0700, Alison Schofield wrote:
> On Sun, Sep 09, 2018 at 06:10:19PM -0700, Huang, Kai wrote:
> > 
> > > -----Original Message-----
> > > From: [e-mail] [mailto:keyrings-
> > > [e-mail]] On Behalf Of Alison Schofield
> > > Sent: Saturday, September 8, 2018 10:23 AM
> > > To: [e-mail]; [e-mail]
> > > Cc: Huang, Kai <[e-mail]>; Nakajima, Jun
> > > <[e-mail]>; Shutemov, Kirill <kirill.shutemov@intel
> > > .com>;
> > > Hansen, Dave <[e-mail]>; Sakkinen, Jarkko
> > > <[e-mail]>; [e-mail]; [e-mail]
> > > nel.org;
> > > [e-mail]; [e-mail]; hpa@zyto
> > > r.com;
> > > [e-mail]; [e-mail]
> > > Subject: [RFC 00/12] Multi-Key Total Memory Encryption API
> > > (MKTME)
> > > 
> > > Seeking comments on the APIs supporting MKTME on future Intel
> > > platforms.
> > > 
> > > MKTME (Multi-Key Total Memory Encryption) is a technology
> > > supporting
> > > memory encryption on upcoming Intel platforms. Whereas TME allows
> > > encryption of the entire system memory using a single key, MKTME
> > > allows
> > > mulitple encryption domains, each having their own key. While the
> > > main use
> > > case for the feature is virtual machine isolation, the API needs
> > > the flexibility to
> > > work for a wide range of use cases.
> > > 
> > > This RFC presents the 2 API additions that enable userspace to:
> > >  1) Create Encryption Keys: Kernel Key Service type "mktme"
> > >  2) Use the Encryption Keys: system call encrypt_mprotect()
> > > 
> > > In order to share between: the Kernel Key Service, the new system
> > > call, and the
> > > existing mm code, helper functions were created in arch/x86/mktme
> > 
> > IMHO, we can separate this series into 2 parts, as you did above,
> > and send out them separately. The reason is, in general I think
> > adding new MKTME type to key retention services is not that related
> > to memory management code, namely the encrypt_mprotect() API part.
> > 
> > So if we split the two parts and send them out separately, the
> > first part can be reviewed by keyring and security guys, without
> > involving mm guys, and the encrypt_mprotect() part can be more
> > reviewed more by mm guys. 
> > 
> 
> Kai,
> 
> That was the direction I had in mind at the onset: the MKTME key
> service
> would be one patch(set) and the MKTME encrypt_mprotect() system call
> would
> be delivered in another patch(set).
> 
> That separation falls apart when the shared structures and functions
> are
> introduced. That 'mktme_map' (maps userspace keys to hardware keyid
> slots),
> and the 'encrypt_count' array (counts vma's outstanding for each key)
> need
> to be shared by both pieces. These mktme special shared structures
> and the
> functions that operate on them are all defined in
> arch/x86/mm/mktme.h,.c.
> From there they can be shared with the security/keys/mktme_keys.c
> 
> Once I made that separation, I stuck with it. Those structures, and
> any
> functions that manipulate those structures live in
> arch/x86/mm/mktme.h,c
> 
> You noted that some of the functions that operate on the
> encrypt_count
> might not need to be over in arch/x86/mm/mktme.c because they are not
> used
> in the mm code. That is true.  

Yes IMO this is better.

> But, then I'd be splitting up the definition
> of the struct and the funcs that operate on it. So, I stuck with
> keeping it
> all together in the arch specific mktme files.

Definition can also be in include/keys/mktme-type.h, right?

> 
> Having said all the above, I do welcome other ideas on how to better
> organize
> the code. 
> 
> Back to your request- to split it into smaller patchsets might look
> something
> like:
> 1) the MKTME API helpers

Exactly what helpers?

> 2) the MKTME Key Service
> 3) the MKTME syscall encrypt_mprotect()
> 
> I'm not clear that would make anyones review life easier, than
> picking
> the same pieces out of the greater patchset.

Well, the reason I suggested we should split key retention services
from other staff and get it reviewed first is I think the
functionalities of MKTME key type has nothing to do w/ MM code. IMHO
what MKTME key type should provide includes:

1) Support add_key, keyctl (revoke, etc) obviously. THis also implies
keyID allocation, so you will need some info from MM or x86 code to get
 info such as number of keyIDs, etc. In fact I think number of keyIDs
is  the only info you need (0 keyIDs means MKTME is disabled).

2) You need to provide some APIs to munipulate key reference count.
Those functions can be declared in mktme-type.h, and implemented in
mktme_keys.c. I think this is more reasonable, as logically reference
count is part of MKTME key service.

I don't think you need to do anything else in MKTME key type?

I think you can even start upstreaming MKTME key type now w/o Kirill's
core-MKTME support, since 1) adding MKTME key type doesn't do any harm
w/o kirill's core-MKTME code; 2) logically MKTME key type has no true
dependency on core-MKTME code.

So IMHO, a better structure should be:

1) Current upstream code (maybe with some early detction code change,
ie, to disable MKTME for some SW reason, but I am not sure whether this
is truely needed, since when MKTME key type gets initialized, that part
should been done already).
2) MKTME key type
3) Kirill's core-MKTME
4) new syscall

And I think CONFIG_MKTME_KEYS doesn't really need to depend on
CONFIG_INTEL_MKTME at this point, although we can add such if needed
when kirill's core-MKTME code gets upstreamed.

Thanks,
-Kai
> 
> Suggestions welcome,
> Alison
> 
> > And since encrypt_mprotect() is a new syscall, you may need to add
> > more lists for the review, ie, linux-api, and maybe linux-kernel as
> > well.
> 
> Got it. Will include theses in v1.
> 
> > 
> > Thanks,
> > -Kai
> > 
> > > 
> > > This patchset is built upon Kirill Shutemov's patchset for the
> > > core MKTME
> > > support. You can find that here:
> > > git://git.kernel.org/pub/scm/linux/kernel/git/kas/linux.git
> > > mktme/wip
> > > 
> > > 
> > > Alison Schofield (12):
> > >   docs/x86: Document the Multi-Key Total Memory Encryption API
> > >   mm: Generalize the mprotect implementation to support
> > > extensions
> > >   syscall/x86: Wire up a new system call for memory encryption
> > > keys
> > >   x86/mm: Add helper functions to manage memory encryption keys
> > >   x86/mm: Add a helper function to set keyid bits in encrypted
> > > VMA's
> > >   mm: Add the encrypt_mprotect() system call
> > >   x86/mm: Add helper functions to track encrypted VMA's
> > >   mm: Track VMA's in use for each memory encryption keyid
> > >   mm: Restrict memory encryption to anonymous VMA's
> > >   x86/pconfig: Program memory encryption keys on a system-wide
> > > basis
> > >   keys/mktme: Add a new key service type for memory encryption
> > > keys
> > >   keys/mktme: Do not revoke in use memory encryption keys
> > > 
> > >  Documentation/x86/mktme-keys.txt       | 153 ++++++++++++++++
> > >  arch/x86/Kconfig                       |   1 +
> > >  arch/x86/entry/syscalls/syscall_32.tbl |   1 +
> > >  arch/x86/entry/syscalls/syscall_64.tbl |   1 +
> > >  arch/x86/include/asm/intel_pconfig.h   |  42 ++++-
> > >  arch/x86/include/asm/mktme.h           |  21 +++
> > >  arch/x86/mm/mktme.c                    | 141 ++++++++++++++
> > >  fs/exec.c                              |   4 +-
> > >  include/keys/mktme-type.h              |  28 +++
> > >  include/linux/key.h                    |   2 +
> > >  include/linux/mm.h                     |   9 +-
> > >  include/linux/syscalls.h               |   2 +
> > >  include/uapi/asm-generic/unistd.h      |   4 +-
> > >  kernel/fork.c                          |   2 +
> > >  kernel/sys_ni.c                        |   2 +
> > >  mm/mmap.c                              |  12 ++
> > >  mm/mprotect.c                          |  93 +++++++++-
> > >  mm/nommu.c                             |   4 +
> > >  security/keys/Kconfig                  |  11 ++
> > >  security/keys/Makefile                 |   1 +
> > >  security/keys/internal.h               |   6 +
> > >  security/keys/keyctl.c                 |   7 +
> > >  security/keys/mktme_keys.c             | 325
> > > +++++++++++++++++++++++++++++++++
> > >  23 files changed, 855 insertions(+), 17 deletions(-)  create
> > > mode 100644
> > > Documentation/x86/mktme-keys.txt  create mode 100644
> > > include/keys/mktme-
> > > type.h  create mode 100644 security/keys/mktme_keys.c
> > > 
> > > --
> > > 2.14.1