Second draft

.gitattributes

@@ -0,0 +1,5 @@
+# Use unix line endings always, even on Windows
+*                            text=auto eol=lf
+
+# Allow comments in JSON in GitHub rendering
+*.json                       linguist-language=JSON-with-Comments

.github/workflows/no-merge.yml

@@ -0,0 +1,22 @@
+name: Check no-merge label
+
+on:
+  pull_request:
+    branches:
+      - main
+    types:
+      - opened
+      - reopened
+      - synchronize
+      - labeled
+      - unlabeled
+
+jobs:
+  check-no-merge:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Fail if PR is labeled no-merge
+        if: contains(github.event.pull_request.labels.*.name, 'no-merge')
+        run: |
+          echo "This PR is labeled no-merge and should not be merged."
+          exit 1

.github/workflows/validate.yml

@@ -18,16 +18,25 @@ jobs:
     - name: Setup dotnet
       uses: actions/setup-dotnet@v4
       with:
-        dotnet-version: 8.0.x
+        dotnet-version: 9.0.x
 
     - name: Show dotnet info
       run: dotnet --info
 
     - name: Restore
       run: dotnet restore src
 
-    - name: Build
+    - name: Check Formatting
+      run: dotnet format --verify-no-changes src
+
+    - name: Build (Debug)
+      run: dotnet build src -c Debug --no-restore
+
+    - name: Test (Debug)
+      run: dotnet test src -c Debug --no-build
+
+    - name: Build (Release)
       run: dotnet build src -c Release --no-restore
 
-    - name: Test
-      run: dotnet test src -c Release --no-build
+    - name: Test (Release)
+      run: dotnet test src -c Release --no-build

devdocs/GenerateKeyPseudoCode.md

@@ -7,7 +7,7 @@
     - Count of bytes of entropy (16 - 32) + 
     - 12 bytes (for fixed signatures + timestamp)
     - [0 - N] bytes for provider reserved data
-    - 3 bytes (for partial HMACSHA256 hash of key)
+    - 3 bytes (for partial CRC32 hash of key)
 1. Generate at least 128 bits of random data (256 recommended and required for MS)
 1. Copy random bytes to start of key
 1. Copy provider reserved bytes to key

devdocs/Questions.md

@@ -1,15 +1,42 @@
 # Questions
 1. MD5 vs. CRC32 (or something else)?
-1. Should we have a dedicated return type?
-1. Timing attacks in computed hash?
+   - 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?
+  - Best name for combined? Or better names (plural) if they should remain separate?
+  - Related: can we avoid repeating Cask like Cask.IsCask -> Cask.IsValid.
+
+1. Should we use a new timestamp in GenerateHash and mask it out in CompareHash?
+   - The first draft created a new timestamp, but didn't mask so it would fail if the month rolled over between GenerateHash and CompareHash.
+   - The second draft copies the timestamp from the secret when generating a hash.
+   - It feels wrong for something called "GenerateHash" to not be deterministic.
+
+1. Are the limits on entropy and provider data length reasonable? 
+   - Review Limits.cs.
+   - Keeping them small allows unconditional stackalloc.
+   - These can be increased later.
+
+1. We need to think about not overriding ToString() on CaskKey.
+   - Consider something like .SensitiveValue as the only way to get the string so people don't print it without thinking or try to use the default-initialed ToString() which musn't throw as a key.
 
 # TODOs
-1. Spanify everything in C#, Span<char> to clear up text vs. bytes
-1. Code is broken for dates in 2050 forward (need a test injection mechanism to unit test)
-1. Create a language neutral interface declaration
-1. CompareHash should use heavily optimized MemoryExtensions.SequenceEqual
-1. Tests should have their own root folder so they can have their own D.*.props
-1. Can we eliminate InternalsVisibleTo?
 1. Add hard-coded keys for testing.
-1. Fluent assertions vs. xunit assertions (we choose xunit for now)
 1. Stress, concurrency, performance, fuzzing, RNG behavior testing.
+1. Code coverage reporting in PR validation.
+1. Unit tests for generate/compare hash
+1. Test against base64 input with whitespace and padding, which we must disallow.
+1. Name regex captures and add more regex test coverage.
+1. Move magic numbers/chars to constants.
+1. Use named arguments or better variables with good names for all string arguments in tests.
+1. Use matrix to split debug/release tests onto different runs
+1. Reduce repetition between UTF-8 and UTF-16 code paths if performant. If not performant, comment why there is a bit of repetition. Also consider risk of misleading to use UTF-8 API on base64-decoded bytes before changing this.
+1. Run benchmarks somewhere on regular basis.
+1. Make tests for invalid keys more resilient to implementation changes.
+   - Produce keys using helpers that are only invalid in one way (e.g. just the length is wrong.)
+   - Return enough info from API (needs design) to assert that the reason the key is invalid is the reason we expect.