From fad32ea2c703ff64b6a7d7a5b092edc9e527d591 Mon Sep 17 00:00:00 2001
From: ale <ale@incal.net>
Date: Sat, 4 Dec 2021 16:01:34 +0000
Subject: [PATCH] Improve error checking in U2F key deserialization

---
 ldap/compositetypes/composite_types.go | 42 ++++++++++++++++++--------
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/ldap/compositetypes/composite_types.go b/ldap/compositetypes/composite_types.go
index 263f6d6..25b1097 100644
--- a/ldap/compositetypes/composite_types.go
+++ b/ldap/compositetypes/composite_types.go
@@ -106,17 +106,12 @@ func UnmarshalEncryptedKey(s string) (*EncryptedKey, error) {
 // U2FRegistration stores information on a single WebAuthN/U2F device
 // registration.
 //
-// This type supports both legacy U2F registrations, as well as newer
-// WebAuthN registrations. These have different serialization formats.
-// The legacy U2F serialized format follows part of the U2F standard
-// and just stores 64 bytes of the public key immediately followed by
-// the key handle data, with no encoding. Note that the public key
-// itself is a serialization of the elliptic curve parameters. The
-// newer serialization format is JSON, for extensibility. Marshaling
-// always happens in JSON, legacy format is read-only.
+// The public key is expected to be in raw COSE format. Note that on
+// the wire (i.e. when serialized as JSON) both the public key and the
+// key handle are base64-encoded.
 //
-// The data in U2FRegistration is still encoded, but it can be turned
-// into a usable run-time form by calling Decode().
+// It is possible to obtain a usable webauthn.Credential object at
+// run-time by calling Decode().
 type U2FRegistration struct {
 	KeyHandle []byte `json:"key_handle"`
 	PublicKey []byte `json:"public_key"`
@@ -133,6 +128,11 @@ func (r *U2FRegistration) Marshal() string {
 	return string(data)
 }
 
+const (
+	legacySerializedU2FKeySize = 65
+	minU2FKeySize              = 64
+)
+
 // UnmarshalU2FRegistration parses a U2FRegistration from its serialized format.
 func UnmarshalU2FRegistration(s string) (*U2FRegistration, error) {
 	// Try JSON first.
@@ -143,13 +143,13 @@ func UnmarshalU2FRegistration(s string) (*U2FRegistration, error) {
 
 	// Deserialize legacy format, and perform a conversion of the
 	// public key to COSE format.
-	if len(s) < 65 {
+	if len(s) < legacySerializedU2FKeySize {
 		return nil, errors.New("badly encoded u2f registration")
 	}
 	b := []byte(s)
 	return &U2FRegistration{
-		PublicKey: u2fToCOSE(b[:65]),
-		KeyHandle: b[65:],
+		PublicKey: u2fToCOSE(b[:legacySerializedU2FKeySize]),
+		KeyHandle: b[legacySerializedU2FKeySize:],
 		Legacy:    true,
 	}, nil
 }
@@ -171,6 +171,14 @@ func ParseLegacyU2FRegistrationFromStrings(keyHandle, publicKey string) (*U2FReg
 		return nil, fmt.Errorf("error decoding public key: %w", err)
 	}
 
+	// Simple sanity check for non-empty fields.
+	if len(kh) == 0 {
+		return nil, errors.New("missing key handle")
+	}
+	if len(pk) < minU2FKeySize {
+		return nil, errors.New("public key missing or too short")
+	}
+
 	return &U2FRegistration{
 		PublicKey: u2fToCOSE(pk),
 		KeyHandle: kh,
@@ -192,6 +200,14 @@ func ParseU2FRegistrationFromStrings(keyHandle, publicKey string) (*U2FRegistrat
 		return nil, fmt.Errorf("error decoding public key: %w", err)
 	}
 
+	// Simple sanity check for non-empty fields.
+	if len(kh) == 0 {
+		return nil, errors.New("missing key handle")
+	}
+	if len(pk) < minU2FKeySize {
+		return nil, errors.New("public key missing or too short")
+	}
+
 	return &U2FRegistration{
 		PublicKey: pk,
 		KeyHandle: kh,
-- 
GitLab