PR feedback

devdocs/Questions.md

@@ -1,11 +1,4 @@
 # Questions
-1. MD5 vs. CRC32 (or something else)?
-   - CRC32 is a good choice, we're using it for its intended purpose.
-   - Can we make room for all four bytes? There isn't a lot of prior art for truncating CRC-32 like SHAs.
-   - Reserving a byte or two for future use by us might be a good idea anyway?
-   - Possible uses:
-     - Record secret entropy length. Right now, it's a bit of a footgun that you have to pass the correct length to Generate/CompareHash.
-
 1. Should provider data be allowed to be unaligned. Should we pad it instead of throwing?
 
 1. Should we combine Cask and CaskKey, putting the statics as helpers on the struct?

docs/PrimaryKey.md

@@ -63,8 +63,8 @@
 |decodedKey[^15..^12]| 37, 4, 9  |0x25, 0x04, 0x09| 00100101b, 00000100b, 00001001b | Decoded 'JQQJ' signature.
 |decodedKey[^12..^9]|0...255|0x0...0xFF|00000000b...11111111b| Provider identifier, e.g. , '0x4c', '0x44', '0x93' (base64 encoded as 'TEST')
 |decodedKey[^9..^6]||||Time stamp data encoded in 4 six-bit blocks for YMDH.
-|decodedKey[^6]|0, 28, 32|0x00, 0x1c, 0x20|00000000b, 00011100b, 00100000b| Leading 6 bits comprises kind enum followed by 2 bits of reserved zero padding.
-|decodedKey[^5]|0|0x00|00000000b| Leading 4 bits comprise version number (currenty 0 = Cask 1.0), followed by 4 bits of reserved zero padding.
+|decodedKey[^6]|0, 28, 32|0x00, 0x1c, 0x20|00000000b, 00011100b, 00100000b | Leading 6 bits comprises kind enum followed by 2 bits of reserved zero padding.
+|decodedKey[^5]|0|0x00|00000000b| Reserved. Must be zero.
 |decodedKey[^4..]|0...255|0x0...0xFF|00000000b..11111111b|CRC32(key[..^4])
 
 ## Primary 256-bit Key Base64-Encoded Rendering
@@ -82,7 +82,7 @@
 |encodedKey[^10]|'A'...'Z'\|'a'..'e'|Represents the day of key allocation, 'A' (0) to 'e' (31)|
 |encodedKey[^9]|'A'...'X'|Represents the hour of key allocation, 'A' (hour 0 or midnight) to 'X' (hour 23). [TBD: This value could be used for half hour increments instead].
 |encodedKey[^8]|'A', 'H', 'I'|Represents the key kind, a 256-bit primary key, HMAC256 or HMAC384.
-|encodedKey[^7]|'A'|Cask version 1.0
-|encodedKey[^6]| 'A'...'D'| Encoded character of four leading zero bits followed by the first two bits of the CRC32 checksum.
+|encodedKey[^7]|'A'| Reserved. Must be 'A'.
+|encodedKey[^6]|'A'...'D'| Encoded character of four leading zero bits followed by the first two bits of the CRC32 checksum.
 |encodedKey[^5..]|'A'...'_'|The final five encoded checksum characters representing the remaining 30 bits of the CRC32 checksum.
 ```

src/Cask/Cask.cs

@@ -35,6 +35,7 @@ public static bool IsCask(ReadOnlySpan<char> key)
             return false;
         }
 
+        // Check for CASK signature, "JQQJ".
         if (!key[CaskSignatureCharRange].SequenceEqual(CaskSignature))
         {
             return false;
@@ -74,6 +75,7 @@ public static bool IsCaskUtf8(ReadOnlySpan<byte> keyUtf8)
             return false;
         }
 
+        // Check for CASK signature, "JQQJ" UTF-8 encoded.
         if (!keyUtf8[CaskSignatureCharRange].SequenceEqual(CaskSignatureUtf8))
         {
             return false;
@@ -108,26 +110,27 @@ public static bool IsCaskUtf8(ReadOnlySpan<byte> keyUtf8)
     /// </summary>
     public static bool IsCaskBytes(ReadOnlySpan<byte> keyBytes)
     {
-        // Check length is within limits and 3-byte aligned.
         if (keyBytes.Length < MinKeyLengthInBytes || keyBytes.Length > MaxKeyLengthInBytes || !Is3ByteAligned(keyBytes.Length))
         {
             return false;
         }
 
-        // Check CASK signature
+        // Check for CASK signature. "JQQJ" base64-decoded.
         if (!keyBytes[CaskSignatureByteRange].SequenceEqual(CaskSignatureBytes))
         {
             return false;
         }
 
-        // Check kind. NOTE: Hash384Bit is not implemented yet.
+        // Check that kind is valid. NOTE: 'Hash384Bit' is not implemented yet
+        // and is therefore treated as invalid here for now.
         if (!TryByteToKind(keyBytes[KindByteIndex], out KeyKind kind) || kind is not KeyKind.Key256Bit and not KeyKind.Hash256Bit)
         {
             return false;
         }
 
-        // Check version: must be 1.0.0 currently.
-        if (!TryByteToVersion(keyBytes[VersionByteIndex], out CaskVersion version) || version is not CaskVersion.OneZeroZero)
+        // Check that reserved version byte is zeroed out. If not, this might be
+        // a CASK key from a future version that we do not support.
+        if (keyBytes[ReservedVersionByteIndex] != 0)
         {
             return false;
         }
@@ -257,7 +260,7 @@ private static void FinalizeKey(Span<byte> key, KeyKind kind, ReadOnlySpan<byte>
             timestamp.CopyTo(key[TimestampByteRange]);
         }
 
-        key[VersionByteIndex] = VersionToByte(CaskVersion.OneZeroZero);
+        key[ReservedVersionByteIndex] = 0;
         key[KindByteIndex] = KindToByte(kind);
         Crc32.Hash(key[..Crc32ByteRange.Start], key[Crc32ByteRange]);
     }

src/Cask/CaskKey.cs

@@ -7,6 +7,8 @@
 using System.Text;
 using System.Text.RegularExpressions;
 
+using static System.Text.RegularExpressions.RegexOptions;
+
 namespace CommonAnnotatedSecurityKeys;
 
 /// <summary>
@@ -36,15 +38,6 @@ public KeyKind Kind
         }
     }
 
-    public CaskVersion CaskVersion
-    {
-        get
-        {
-            ThrowIfUnitialized();
-            return CharToVersion(_key[VersionCharIndex]);
-        }
-    }
-
     public int SizeInBytes
     {
         get
@@ -186,7 +179,7 @@ private void ThrowIfUnitialized()
 
     // language=regex
     private const string RegexPattern = """(^|[^A-Za-z0-9+/-_])([A-Za-z0-9-_]{4}){6,}JQQJ[A-Za-z0-9-_]{16}($|[^A-Za-z0-9+/-_])""";
-    private const RegexOptions RegexFlags = RegexOptions.Compiled | RegexOptions.ExplicitCapture | RegexOptions.CultureInvariant;
+    private const RegexOptions RegexFlags = Compiled | ExplicitCapture | CultureInvariant | NonBacktracking;
 
     [GeneratedRegex(RegexPattern, RegexFlags)]
     private static partial Regex CompiledRegex();

src/Cask/Helpers.cs

@@ -46,8 +46,7 @@ public static bool Is4CharAligned(int charLength)
     public static int GetKeyLengthInBytes(int providerDataLengthInBytes)
     {
         Debug.Assert(Is3ByteAligned(providerDataLengthInBytes), $"{nameof(providerDataLengthInBytes)} should have been validated to 3-byte aligned already.");
-        int paddedEntropyInBytes = RoundUpTo3ByteAlignment(SecretEntropyInBytes);
-        int keyLengthInBytes = paddedEntropyInBytes + providerDataLengthInBytes + FixedKeyComponentSizeInBytes;
+        int keyLengthInBytes = PaddedSecretEntropyInBytes + providerDataLengthInBytes + FixedKeyComponentSizeInBytes;
         Debug.Assert(Is3ByteAligned(keyLengthInBytes));
         return keyLengthInBytes;
     }
@@ -63,40 +62,15 @@ public static int GetHashLengthInBytes(int secretSizeInBytes, out int providerDa
 
     public static KeyKind CharToKind(char kindChar)
     {
+        Debug.Assert(kindChar == 'A' || kindChar == 'H' || kindChar == 'I', "This is only meant to be called using the kind char of a known valid key.");
         return (KeyKind)(kindChar - 'A');
     }
 
-    public static CaskVersion CharToVersion(char versionChar)
-    {
-        return (CaskVersion)(versionChar - 'A');
-    }
-
     public static byte KindToByte(KeyKind kind)
     {
         return (byte)((int)kind << KindReservedBits);
     }
 
-    public static byte VersionToByte(CaskVersion version)
-    {
-        return (byte)((int)version << VersionReservedBits);
-    }
-
-    /// <summary>
-    /// Converts a byte that encodes the CASK version to the CaskVersion enum.
-    /// Returns false if the reserved bits in that byte are non-zero.
-    /// </summary>
-    public static bool TryByteToVersion(byte value, out CaskVersion version)
-    {
-        if ((value & VersionReservedMask) != 0)
-        {
-            version = default;
-            return false;
-        }
-
-        version = (CaskVersion)(value >> VersionReservedBits);
-        return true;
-    }
-
     /// <summary>
     /// Converts a byte that encodeds the key kind to the KeyKind enum.
     /// Returns false if the reserved bits in that byte are non-zero.
@@ -151,7 +125,8 @@ private static int RoundUpToMultipleOf(int value, int multiple)
         return (value + multiple - 1) / multiple * multiple;
     }
 
-    public static void ThrowIfUnitialized<T>(T value, [CallerArgumentExpression(nameof(value))] string? paramName = null) where T : struct, IIsInitialized
+    public static void ThrowIfUnitialized<T>(T value, [CallerArgumentExpression(nameof(value))] string? paramName = null)
+        where T : struct, IIsInitialized
     {
         if (!value.IsInitialized)
         {
@@ -174,6 +149,7 @@ public static void ThrowIfNotHash(CaskKey value, [CallerArgumentExpression(nameo
             ThrowNotHash(paramName);
         }
     }
+
     public static void ThrowIfDestinationTooSmall<T>(Span<T> destination, int requiredSize, [CallerArgumentExpression(nameof(destination))] string? paramName = null)
     {
         if (destination.Length < requiredSize)

src/Cask/InternalConstants.cs

@@ -9,20 +9,22 @@ namespace CommonAnnotatedSecurityKeys;
 /// Move things elsewhere if/when they need to be made public, and avoid `const` in 
 /// public API in favor of static readonly properties.
 /// </remarks>
-internal static class InternalConstants
+internal static partial class InternalConstants
 {
+    /// <summary>
+    /// The base64-encoded CASK signature "JQQJ" in UTF-16.)
+    /// </summary>
     public static ReadOnlySpan<char> CaskSignature => "JQQJ".AsSpan();
-    public static ReadOnlySpan<byte> CaskSignatureUtf8 => "JQQJ"u8;
 
     /// <summary>
-    /// The bytes that make up the CASK signature. "JQQJ", base64-decoded.
+    /// The base64-encoded CASK signature "JQQJ" in UTF-8.
     /// </summary>
-    public static ReadOnlySpan<byte> CaskSignatureBytes => [0x25, 0x04, 0x09];
+    public static ReadOnlySpan<byte> CaskSignatureUtf8 => "JQQJ"u8;
 
     /// <summary>
-    /// Current version of CASK spec that we implement.
+    /// The base64-decoded CASK signature "JQQJ" in bytes.
     /// </summary>
-    public const int CaskVersion = 0;
+    public static ReadOnlySpan<byte> CaskSignatureBytes => [0x25, 0x04, 0x09];
 
     /// <summary>
     /// The number of bytes in a CASK signature
@@ -35,12 +37,12 @@ internal static class InternalConstants
     public const int TimestampSizeInBytes = 3;
 
     /// <summary>
-    /// The number of bytes  the provider signature.
+    /// The number of bytes in a provider signature.
     /// </summary>
     public const int ProviderSignatureSizeInBytes = 3;
 
     /// <summary>
-    /// The nubmer of bytes reserved in the key footer for future use.
+    /// The number of bytes reserved in the key footer for future use.
     /// </summary>
     public const int VersionAndKindSizeInBytes = 2;
 
@@ -60,84 +62,79 @@ internal static class InternalConstants
         Crc32SizeInBytes;
 
     /// <summary>
-    /// The number of bytes in the fixed componet of a hash key, from the C3ID to the end of the key.
+    /// The number of bytes in the fixed components of a hash key, from the C3ID to the end of the key.
     /// </summary>
     public const int FixedHashComponentSizeInBytes = FixedKeyComponentSizeInBytes + CrossCompanyCorrelatingId.RawSizeInBytes;
 
     /// <summary>
-    /// The number of bytes of entropy in a primary key.
-    /// Currently fixed, but may become configurable in future versions.
+    /// The number of bytes of entropy in a primary key. 32-bytes (256 bits) of
+    /// entropy generated by a cryptographically secure RNG are currently deemed
+    /// unbreakable even in a post-quantum world. 
+    /// 
+    /// Currently this value is fixed, but may become configurable in future versions.
     /// </summary>
     public const int SecretEntropyInBytes = 32;
 
     /// <summary>
     /// The size of the entropy in a primary after padding to 3-byte alignment.
     /// </summary>
-    public const int PaddedSecretEntropyInBytes = 33;
+    public static int PaddedSecretEntropyInBytes { get; } = RoundUpTo3ByteAlignment(SecretEntropyInBytes);
 
     /// <summary>
     /// The size of the HMAC-SHA256 hash after padding to 3-byte alignment.
     /// </summary>
-    public const int PaddedHmacSha256SizeInBytes = 33;
+    public static int PaddedHmacSha256SizeInBytes { get; } = RoundUpTo3ByteAlignment(PaddedSecretEntropyInBytes);
 
     /// <summary>
     /// The maximum amount of bytes that the implementation will stackalloc.
     /// </summary>
     public const int MaxStackAlloc = 256;
 
     /// <summary>
-    /// The range of byte indices in a key for the bytes that contain the CASK signature.
+    /// The number of least significant bits reserved in the key kind byte.
     /// </summary>
-    public static Range CaskSignatureByteRange => ^15..^12;
-
-    /// <summary>
-    /// The range of byte indices in a key for the bytes that contain the provider signature.
-    /// </summary>
-    public static Range ProviderSignatureByteRange => ^12..^9;
+    public const int KindReservedBits = 2;
 
     /// <summary>
-    /// The range of byte indices in a key for the bytes that contain the timestamp.
+    /// A bit mask to obtain the reserved bits from the key kind.
     /// </summary>
-    public static Range TimestampByteRange => ^9..^6;
+    public const int KindReservedMask = (1 << KindReservedBits) - 1;
 
     /// <summary>
-    /// The index of the byte in a key that contains the key kind.
+    /// The range of byte indices in a hash for the bytes that contain the C3ID of the secret.
     /// </summary>
-    public static Index KindByteIndex = ^6;
+    public static Range C3IdByteRange => ^27..^15;
 
     /// <summary>
-    /// The index of the byte in a key that contains the CASK version.
-    /// </summary>
-    public static Index VersionByteIndex => ^5;
-    /// <summary>
-    /// The range of byte indices in a key for the bytes that contain the CRC32 of the key.
+    /// The range of byte indices in a key for the bytes that contain the CASK signature.
     /// </summary>
-    public static Range Crc32ByteRange => ^4..;
+    public static Range CaskSignatureByteRange => ^15..^12;
 
     /// <summary>
-    /// The range of byte indices in a hash for the bytes that contain the C3ID of the secret.
+    /// The range of byte indices in a key for the bytes that contain the provider signature.
     /// </summary>
-    public static Range C3IdByteRange => ^27..^15;
+    public static Range ProviderSignatureByteRange => ^12..^9;
 
     /// <summary>
-    /// The number of least significant bits reserved in the version byte.
+    /// The range of byte indices in a key for the bytes that contain the timestamp.
     /// </summary>
-    public const int VersionReservedBits = 4;
+    public static Range TimestampByteRange => ^9..^6;
 
     /// <summary>
-    /// A bit mask to obtain the reserved bits from the version byte
+    /// The index of the byte in a key that contains the key kind.
     /// </summary>
-    public const int VersionReservedMask = (1 << VersionReservedBits) - 1;
+    public static Index KindByteIndex = ^6;
 
     /// <summary>
-    /// The number of least significant bits reserved in the key kind
+    /// The index of the byte in a key that is currently reserved to be zeroed
+    /// out, and may be used in the future to indicate a format change.
     /// </summary>
-    public const int KindReservedBits = 2;
+    public static Index ReservedVersionByteIndex => ^5;
 
     /// <summary>
-    /// A bit mask to obtain the reserved bits from the key kind.
+    /// The range of byte indices in a key for the bytes that contain the CRC32 of the key.
     /// </summary>
-    public const int KindReservedMask = (1 << KindReservedBits) - 1;
+    public static Range Crc32ByteRange => ^4..;
 
     /// <summary>
     /// The range of chars in a base64-encoded key that hold the Cask signature.

src/Cask/KeyKind.cs

@@ -10,6 +10,7 @@ public enum KeyKind
     /// </summary>
     Key256Bit = ('A' - 'A'), // Base64: index 0 == 'A'
                              // Bytewise: << 2 == 0, 0x00, 0b0000_0000
+
     /// <summary>
     /// Specifies a CASK hashed signature that incorporates a Message
     /// Authentication code of a derivation input generated by SHA-256.

src/Tests/Cask.Benchmarks/CompareHashBenchmarks.cs

@@ -29,7 +29,7 @@ public bool CompareHash_Floor()
         Span<byte> candidateHash = stackalloc byte[HMACSHA256.HashSizeInBytes];
         Base64Url.DecodeFromChars(TestNonIdentifiableHash.AsSpan(), candidateHash);
 
-        Span<byte> secret = stackalloc byte[32];
+        Span<byte> secret = stackalloc byte[TestSecretEntropyInBytes];
         Base64Url.DecodeFromChars(TestNonIdentifiableSecret.AsSpan(), secret);
 
         Span<byte> hash = stackalloc byte[HMACSHA256.HashSizeInBytes];

src/Tests/Cask.Benchmarks/GenerateHashBenchmarks.cs

@@ -26,7 +26,7 @@ public string GenerateHash_Cask()
     [Benchmark]
     public string GenerateHash_Floor()
     {
-        Span<byte> secret = stackalloc byte[32];
+        Span<byte> secret = stackalloc byte[TestSecretEntropyInBytes];
         Base64Url.DecodeFromChars(TestNonIdentifiableSecret.AsSpan(), secret);
 
         Span<byte> hash = stackalloc byte[HMACSHA256.HashSizeInBytes];

src/Tests/Cask.Benchmarks/GenerateKeyBenchmarks.cs

@@ -27,7 +27,7 @@ public string GenerateKey_Cask()
     [Benchmark]
     public string GenerateKey_Floor()
     {
-        Span<byte> bytes = stackalloc byte[32];
+        Span<byte> bytes = stackalloc byte[TestSecretEntropyInBytes];
         RandomNumberGenerator.Fill(bytes);
         return Base64Url.EncodeToString(bytes);
     }