From 45d1faaea8bd87c0cd386fba548033132ffdeefc Mon Sep 17 00:00:00 2001
From: Alan Stokes <alanstokes@google.com>
Date: Wed, 20 Dec 2023 16:19:51 +0000
Subject: [PATCH] Clarify Secretkeeper comments

Various small wording changes to attempt to clarify some of the
details of the Secretkeeper API.

While I'm here: fix error code naming inconsistency.

Bug: 291224769
Test: N/A
Change-Id: I956b549bc5bf4d2b964dde9867430cb4778e445b
---
 .../security/secretkeeper/ISecretkeeper.aidl        | 13 +++++++++----
 .../security/secretkeeper/SecretManagement.cddl     |  4 ++--
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ISecretkeeper.aidl b/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ISecretkeeper.aidl
index 49c3446265..b07dba8975 100644
--- a/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ISecretkeeper.aidl
+++ b/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ISecretkeeper.aidl
@@ -39,9 +39,14 @@ interface ISecretkeeper {
 
     /**
      * Retrieve the instance of the `IAuthGraphKeyExchange` HAL that should be used for shared
-     * session key establishment.  These keys are used to perform encryption of messages as
+     * session key establishment. These keys are used to perform encryption of messages as
      * described in SecretManagement.cddl, allowing the client and Secretkeeper to have a
-     * cryptographically secure channel.
+     * cryptographically secure channel. In the key exchange protocol the client acts as P1
+     * (source) and Secretkeeper as P2 (sink). The interface returned here can be used to invoke
+     * methods on the sink.
+     *
+     * The client's identity is its DICE chain; Secretkeeper's identity is a
+     * per-boot key pair.
      */
     IAuthGraphKeyExchange getAuthGraphKe();
 
@@ -56,8 +61,8 @@ interface ISecretkeeper {
      * ProtectedRequestPacket & ProtectedResponsePacket using symmetric keys agreed between
      * the client & service. This cryptographic protection is required because the messages are
      * ferried via Android, which is allowed to be outside the TCB of clients (for example protected
-     * Virtual Machines). For this, service (& client) must implement a key exchange protocol, which
-     * is critical for establishing the secure channel.
+     * Virtual Machines). For this, service (& client) must implement the AuthGraph key exchange
+     * protocol to establish a secure channel between them.
      *
      * If an encrypted response cannot be generated, then a service-specific Binder error using one
      * of the ERROR_ codes above will be returned.
diff --git a/security/secretkeeper/aidl/android/hardware/security/secretkeeper/SecretManagement.cddl b/security/secretkeeper/aidl/android/hardware/security/secretkeeper/SecretManagement.cddl
index 3d080789f1..a9e43519f1 100644
--- a/security/secretkeeper/aidl/android/hardware/security/secretkeeper/SecretManagement.cddl
+++ b/security/secretkeeper/aidl/android/hardware/security/secretkeeper/SecretManagement.cddl
@@ -82,7 +82,7 @@ ErrorCode = &(
     ; Requested Entry not found.
     ErrorCode_EntryNotFound: 3,
     ; Error happened while serialization or deserialization.
-    SerializationError: 4,
+    ErrorCode_SerializationError: 4,
     ; Indicates that Dice Policy matching did not succeed & hence access not granted.
     ErrorCode_DicePolicyError: 5,
 )
@@ -95,7 +95,7 @@ Result = &(
     GetSecretResult,
 )
 
-GetVersionResult = (version : uint)
+GetVersionResult = (1)
 
 StoreSecretResult = ()