From de8b39065ec6d56d6784ce3b704f00432b41e6fb Mon Sep 17 00:00:00 2001
From: Neil Alexander <neilalexander@users.noreply.github.com>
Date: Wed, 23 Sep 2020 11:07:57 +0100
Subject: [PATCH] Enforce valid key IDs (#1437)

* Enforce valid key IDs

* Don't use key_id from dendrite.yaml as it is in matrix_key.pem
---
 dendrite-config.yaml             | 3 ---
 internal/config/config.go        | 6 ++++++
 internal/config/config_global.go | 2 +-
 internal/test/config.go          | 7 ++++++-
 4 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/dendrite-config.yaml b/dendrite-config.yaml
index be0972e4..8c737692 100644
--- a/dendrite-config.yaml
+++ b/dendrite-config.yaml
@@ -38,9 +38,6 @@ global:
   # The path to the signing private key file, used to sign requests and events.
   private_key: matrix_key.pem
 
-  # A unique identifier for this private key. Must start with the prefix "ed25519:".
-  key_id: ed25519:auto
-
   # How long a remote server can cache our server signing key before requesting it
   # again. Increasing this number will reduce the number of requests made by other
   # servers for our key but increases the period that a compromised key will be
diff --git a/internal/config/config.go b/internal/config/config.go
index d7470f87..d75500db 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -36,6 +36,9 @@ import (
 	jaegermetrics "github.com/uber/jaeger-lib/metrics"
 )
 
+// keyIDRegexp defines allowable characters in Key IDs.
+var keyIDRegexp = regexp.MustCompile("^ed25519:[a-zA-Z0-9_]+$")
+
 // Version is the current version of the config format.
 // This will change whenever we make breaking changes to the config format.
 const Version = 1
@@ -459,6 +462,9 @@ func readKeyPEM(path string, data []byte) (gomatrixserverlib.KeyID, ed25519.Priv
 			if !strings.HasPrefix(keyID, "ed25519:") {
 				return "", nil, fmt.Errorf("key ID %q doesn't start with \"ed25519:\" in %q", keyID, path)
 			}
+			if !keyIDRegexp.MatchString(keyID) {
+				return "", nil, fmt.Errorf("key ID %q in %q contains illegal characters (use a-z, A-Z, 0-9 and _ only)", keyID, path)
+			}
 			_, privKey, err := ed25519.GenerateKey(bytes.NewReader(keyBlock.Bytes))
 			if err != nil {
 				return "", nil, err
diff --git a/internal/config/config_global.go b/internal/config/config_global.go
index 2b36da2f..03f522be 100644
--- a/internal/config/config_global.go
+++ b/internal/config/config_global.go
@@ -20,7 +20,7 @@ type Global struct {
 
 	// An arbitrary string used to uniquely identify the PrivateKey. Must start with the
 	// prefix "ed25519:".
-	KeyID gomatrixserverlib.KeyID `yaml:"key_id"`
+	KeyID gomatrixserverlib.KeyID `yaml:"-"`
 
 	// How long a remote server can cache our server key for before requesting it again.
 	// Increasing this number will reduce the number of requests made by remote servers
diff --git a/internal/test/config.go b/internal/test/config.go
index 72cd0e6e..8080988f 100644
--- a/internal/test/config.go
+++ b/internal/test/config.go
@@ -25,6 +25,7 @@ import (
 	"math/big"
 	"os"
 	"path/filepath"
+	"strings"
 	"time"
 
 	"github.com/matrix-org/dendrite/internal/config"
@@ -146,10 +147,14 @@ func NewMatrixKey(matrixKeyPath string) (err error) {
 		err = keyOut.Close()
 	})()
 
+	keyID := base64.RawURLEncoding.EncodeToString(data[:])
+	keyID = strings.ReplaceAll(keyID, "-", "")
+	keyID = strings.ReplaceAll(keyID, "_", "")
+
 	err = pem.Encode(keyOut, &pem.Block{
 		Type: "MATRIX PRIVATE KEY",
 		Headers: map[string]string{
-			"Key-ID": "ed25519:" + base64.RawStdEncoding.EncodeToString(data[:3]),
+			"Key-ID": fmt.Sprintf("ed25519:%s", keyID[:6]),
 		},
 		Bytes: data[3:],
 	})