]> git.ipfire.org Git - thirdparty/chrony.git/commitdiff
nts: construct key exporter context
authorMiroslav Lichvar <mlichvar@redhat.com>
Mon, 16 Sep 2024 12:15:38 +0000 (14:15 +0200)
committerMiroslav Lichvar <mlichvar@redhat.com>
Thu, 26 Sep 2024 10:45:44 +0000 (12:45 +0200)
When the NTS client and server negotiated use of AES-128-GCM-SIV keys,
the keys exported from the TLS session and used for authentication and
encryption of NTP messages do not comply to RFC8915. The exporter
context value specified in the section 5.1 of RFC8915 function is
incorrect. It is a hardcoded string which contains 15 (AES-SIV-CMAC-256)
instead of 30 (AES-128-GCM-SIV). This causes chrony to not interoperate
with NTS implementations that follow RFC8915 correctly. (At this time,
there doesn't seem to be another implementation with AES-128-GCM-SIV
support yet.)

Replace the string with a proper construction of the exporter context
from a specified AEAD ID and next protocol.

Keep using the incorrect AEAD ID for AES-128-GCM-SIV to not break
compatibility with existing chrony servers and clients. A new NTS-KE
record will be added to negotiate the compliant exporter context.

Reported-by: Martin Mayer <martin.mayer@m2-it-solutions.de>
nts_ke.h
nts_ke_client.c
nts_ke_server.c
nts_ke_session.c
nts_ke_session.h
test/unit/nts_ke_server.c
test/unit/nts_ke_session.c

index e7497af00f779e4bc956f0500071514bc5d89df5..3ecadd9d752c469386ae5ecd07919cdfedd40b17 100644 (file)
--- a/nts_ke.h
+++ b/nts_ke.h
@@ -49,8 +49,6 @@
 
 #define NKE_ALPN_NAME                   "ntske/1"
 #define NKE_EXPORTER_LABEL              "EXPORTER-network-time-security"
-#define NKE_EXPORTER_CONTEXT_C2S        "\x0\x0\x0\xf\x0"
-#define NKE_EXPORTER_CONTEXT_S2C        "\x0\x0\x0\xf\x1"
 
 #define NKE_MAX_MESSAGE_LENGTH          16384
 #define NKE_MAX_RECORD_BODY_LENGTH      256
index 3891f71293467620a3211c6b05d0fb6fb161f515..f10aed5506cca0f571eb78068d840f8c8b4a3a00 100644 (file)
@@ -255,6 +255,7 @@ process_response(NKC_Instance inst)
 static int
 handle_message(void *arg)
 {
+  SIV_Algorithm exporter_algorithm;
   NKC_Instance inst = arg;
 
   if (!process_response(inst)) {
@@ -262,8 +263,15 @@ handle_message(void *arg)
     return 0;
   }
 
-  if (!NKSN_GetKeys(inst->session, inst->context.algorithm,
-                    &inst->context.c2s, &inst->context.s2c))
+  exporter_algorithm = inst->context.algorithm;
+
+  /* With AES-128-GCM-SIV, set the algorithm ID in the RFC5705 key exporter
+     context incorrectly for compatibility with older chrony servers */
+  if (exporter_algorithm == AEAD_AES_128_GCM_SIV)
+    exporter_algorithm = AEAD_AES_SIV_CMAC_256;
+
+  if (!NKSN_GetKeys(inst->session, inst->context.algorithm, exporter_algorithm,
+                    NKE_NEXT_PROTOCOL_NTPV4, &inst->context.c2s, &inst->context.s2c))
     return 0;
 
   if (inst->server_name[0] != '\0') {
index 6d617b47827780b71b9c679e9143e175c6517a88..d5b240ba7d0025ca537d27c6e7602e36d7d5fc54 100644 (file)
@@ -339,6 +339,7 @@ helper_signal(int x)
 static int
 prepare_response(NKSN_Instance session, int error, int next_protocol, int aead_algorithm)
 {
+  SIV_Algorithm exporter_algorithm;
   NKE_Context context;
   NKE_Cookie cookie;
   char *ntp_server;
@@ -385,8 +386,15 @@ prepare_response(NKSN_Instance session, int error, int next_protocol, int aead_a
     }
 
     context.algorithm = aead_algorithm;
+    exporter_algorithm = aead_algorithm;
 
-    if (!NKSN_GetKeys(session, aead_algorithm, &context.c2s, &context.s2c))
+    /* With AES-128-GCM-SIV, set the algorithm ID in the RFC5705 key exporter
+       context incorrectly for compatibility with older chrony clients */
+    if (exporter_algorithm == AEAD_AES_128_GCM_SIV)
+      exporter_algorithm = AEAD_AES_SIV_CMAC_256;
+
+    if (!NKSN_GetKeys(session, aead_algorithm, exporter_algorithm,
+                      NKE_NEXT_PROTOCOL_NTPV4, &context.c2s, &context.s2c))
       return 0;
 
     for (i = 0; i < NKE_MAX_COOKIES; i++) {
index 2ae1e9155adc2970594b754d9d37ea06ed4e05ab..6ad662a405d20161d4d7101dc187c3c2472a0097 100644 (file)
@@ -877,22 +877,39 @@ NKSN_GetRecord(NKSN_Instance inst, int *critical, int *type, int *body_length,
 /* ================================================== */
 
 int
-NKSN_GetKeys(NKSN_Instance inst, SIV_Algorithm siv, NKE_Key *c2s, NKE_Key *s2c)
+NKSN_GetKeys(NKSN_Instance inst, SIV_Algorithm algorithm, SIV_Algorithm exporter_algorithm,
+             int next_protocol, NKE_Key *c2s, NKE_Key *s2c)
 {
-  int length = SIV_GetKeyLength(siv);
+  int length = SIV_GetKeyLength(algorithm);
+  struct {
+    uint16_t next_protocol;
+    uint16_t algorithm;
+    uint8_t is_s2c;
+    uint8_t _pad;
+  } context;
 
   if (length <= 0 || length > sizeof (c2s->key) || length > sizeof (s2c->key)) {
     DEBUG_LOG("Invalid algorithm");
     return 0;
   }
 
+  assert(sizeof (context) == 6);
+  context.next_protocol = htons(next_protocol);
+  context.algorithm = htons(exporter_algorithm);
+
+  context.is_s2c = 0;
   if (gnutls_prf_rfc5705(inst->tls_session,
                          sizeof (NKE_EXPORTER_LABEL) - 1, NKE_EXPORTER_LABEL,
-                         sizeof (NKE_EXPORTER_CONTEXT_C2S) - 1, NKE_EXPORTER_CONTEXT_C2S,
-                         length, (char *)c2s->key) < 0 ||
-      gnutls_prf_rfc5705(inst->tls_session,
+                         sizeof (context) - 1, (char *)&context,
+                         length, (char *)c2s->key) < 0) {
+    DEBUG_LOG("Could not export key");
+    return 0;
+  }
+
+  context.is_s2c = 1;
+  if (gnutls_prf_rfc5705(inst->tls_session,
                          sizeof (NKE_EXPORTER_LABEL) - 1, NKE_EXPORTER_LABEL,
-                         sizeof (NKE_EXPORTER_CONTEXT_S2C) - 1, NKE_EXPORTER_CONTEXT_S2C,
+                         sizeof (context) - 1, (char *)&context,
                          length, (char *)s2c->key) < 0) {
     DEBUG_LOG("Could not export key");
     return 0;
index 2735e04cdac644f8451e22ce9cc8819fc6ad62ff..b046144faf19c0dd2ca71ecab772ddc9d7809b61 100644 (file)
@@ -77,8 +77,11 @@ extern int NKSN_EndMessage(NKSN_Instance inst);
 extern int NKSN_GetRecord(NKSN_Instance inst, int *critical, int *type, int *body_length,
                           void *body, int buffer_length);
 
-/* Export NTS keys for a specified algorithm */
-extern int NKSN_GetKeys(NKSN_Instance inst, SIV_Algorithm siv, NKE_Key *c2s, NKE_Key *s2c);
+/* Export NTS keys for a specified algorithm (for compatibility reasons the
+   RFC5705 exporter context is allowed to have a different algorithm) */
+extern int NKSN_GetKeys(NKSN_Instance inst, SIV_Algorithm algorithm,
+                        SIV_Algorithm exporter_algorithm,
+                        int next_protocol, NKE_Key *c2s, NKE_Key *s2c);
 
 /* Check if the session has stopped */
 extern int NKSN_IsStopped(NKSN_Instance inst);
index 3d2f295405d2dbbdab4ab422b45868bc49162190..be3f6d805726e03767001f8fb34e6fc1d267308d 100644 (file)
 #define NKSN_GetKeys get_keys
 
 static int
-get_keys(NKSN_Instance session, SIV_Algorithm siv, NKE_Key *c2s, NKE_Key *s2c)
+get_keys(NKSN_Instance session, SIV_Algorithm algorithm, SIV_Algorithm exporter_algorithm,
+         int next_protocol, NKE_Key *c2s, NKE_Key *s2c)
 {
-  c2s->length = SIV_GetKeyLength(siv);
+  c2s->length = SIV_GetKeyLength(algorithm);
   UTI_GetRandomBytes(c2s->key, c2s->length);
-  s2c->length = SIV_GetKeyLength(siv);
+  s2c->length = SIV_GetKeyLength(algorithm);
   UTI_GetRandomBytes(s2c->key, s2c->length);
   return 1;
 }
@@ -174,7 +175,8 @@ test_unit(void)
 
   for (i = 0; i < 10000; i++) {
     context.algorithm = AEAD_AES_SIV_CMAC_256;
-    get_keys(session, context.algorithm, &context.c2s, &context.s2c);
+    get_keys(session, context.algorithm, random() % 100, NKE_NEXT_PROTOCOL_NTPV4,
+             &context.c2s, &context.s2c);
     memset(&cookie, 0, sizeof (cookie));
     TEST_CHECK(NKS_GenerateCookie(&context, &cookie));
     TEST_CHECK(NKS_DecodeCookie(&cookie, &context2));
index d0e72c7af15d0294b9d58bea1c3dee58bfd14d81..c2db4a2544a4c2f318cb51a07d2588aeccb8fad5 100644 (file)
@@ -116,9 +116,19 @@ verify_message(NKSN_Instance inst)
 
   TEST_CHECK(!NKSN_GetRecord(inst, &critical, &t, &length, buffer, sizeof (buffer)));
 
-  TEST_CHECK(NKSN_GetKeys(inst, AEAD_AES_SIV_CMAC_256, &c2s, &s2c));
-  TEST_CHECK(c2s.length == SIV_GetKeyLength(AEAD_AES_SIV_CMAC_256));
-  TEST_CHECK(s2c.length == SIV_GetKeyLength(AEAD_AES_SIV_CMAC_256));
+  for (i = 0; i < 10; i++) {
+    TEST_CHECK(NKSN_GetKeys(inst, AEAD_AES_SIV_CMAC_256, random(), random(), &c2s, &s2c));
+    TEST_CHECK(c2s.length == SIV_GetKeyLength(AEAD_AES_SIV_CMAC_256));
+    TEST_CHECK(s2c.length == SIV_GetKeyLength(AEAD_AES_SIV_CMAC_256));
+
+    if (SIV_GetKeyLength(AEAD_AES_128_GCM_SIV) > 0) {
+      TEST_CHECK(NKSN_GetKeys(inst, AEAD_AES_128_GCM_SIV, random(), random(), &c2s, &s2c));
+      TEST_CHECK(c2s.length == SIV_GetKeyLength(AEAD_AES_128_GCM_SIV));
+      TEST_CHECK(s2c.length == SIV_GetKeyLength(AEAD_AES_128_GCM_SIV));
+    } else {
+      TEST_CHECK(!NKSN_GetKeys(inst, AEAD_AES_128_GCM_SIV, random(), random(), &c2s, &s2c));
+    }
+  }
 }
 
 static int