CCM: harden DeclarePayloadLength (follow-up to #106)

Source/DECAuthenticatedCipherModesBase.pas

@@ -220,9 +220,11 @@   TAuthenticatedCipherModesBase = class(TObject)
     /// <summary>
     ///   Declares the total payload length in bytes. Required by CCM before
     ///   the first Encode/Decode when the message will be supplied in several
-    ///   chunks. Ignored by GCM. A later call is ignored once a length has
-    ///   been set or processing has started. One-shot Encode/Decode still
-    ///   works without this: the first call's Size is treated as the total.
+    ///   chunks. Ignored by GCM. For CCM, repeating the same length is
+    ///   idempotent; a different length, a call after Encode/Decode has
+    ///   started, or a call after Done raises EDECCipherException. One-shot
+    ///   Encode/Decode still works without this: the first call's Size is
+    ///   treated as the total.
     /// </summary>
     /// <param name="AByteLength">
     ///   Total plaintext/ciphertext length in bytes (not including the tag)

Source/DECCipherFormats.pas

@@ -780,7 +780,10 @@ procedure TDECFormattedCipher.DoEncodeDecodeStream(const Source, Dest: TStream;
   if DataSize < 0 then
     DataSize := Source.Size - Pos;
 
-  if Assigned(FAuthObj) then
+  // One-shot authenticated streams declare DataSize as l(m) for CCM. Skip when
+  // a non-zero length is already set so multi-chunk EncodeStream can follow
+  // AuthenticatedPayloadLength without re-declaring the chunk size.
+  if Assigned(FAuthObj) and (FAuthObj.GetDeclaredPayloadLength = 0) then
   begin
     FAuthObj.DeclarePayloadLength(UInt64(DataSize));
   end;

Source/DECCipherModes.pas

@@ -445,8 +445,10 @@   TDECCipherModes = class(TDECCipher, IDECAuthenticatedCipher)
     /// <summary>
     ///   Total payload length in bytes for authenticated modes that need it
     ///   before processing (CCM). Ignored by GCM. Set this before the first
-    ///   Encode/Decode when feeding CCM in several chunks. One EncodeStream
-    ///   of the full message sets it automatically from DataSize.
+    ///   Encode/Decode when feeding CCM in several chunks. Repeating the same
+    ///   length is allowed; a different value, a set after Encode/Decode has
+    ///   started, or a set after Done raises EDECCipherException. One
+    ///   EncodeStream of the full message sets it automatically from DataSize.
     /// </summary>
     property AuthenticatedPayloadLength: UInt64
       read   GetAuthenticatedPayloadLength

Source/DECCipherModesCCM.pas

@@ -231,11 +231,17 @@   TCCM = class(TAuthenticatedCipherModesBase)
     function SupportsMultiChunk: Boolean; override;
     /// <summary>
     ///   Declares the total payload length in bytes before the first
-    ///   Encode/Decode. Ignored if a length is already set or processing started.
+    ///   Encode/Decode. Repeating the same length is idempotent. A different
+    ///   length, a call after Encode/Decode has started, or a call after Done
+    ///   raises EDECCipherException.
     /// </summary>
     /// <param name="AByteLength">
     ///   Total plaintext/ciphertext length in bytes
     /// </param>
+    /// <exception cref="EDECCipherException">
+    ///   Raised after Done, after processing has started, or when a different
+    ///   length is declared.
+    /// </exception>
     procedure DeclarePayloadLength(const AByteLength: UInt64); override;
     /// <summary>
     ///   Returns the payload length declared for this CCM instance
@@ -268,6 +274,10 @@ implementation
     'CCM payload is shorter than the declared length';
   sCCMAADLocked =
     'CCM DataToAuthenticate cannot be changed after Encode/Decode has started or after Done';
+  sCCMPayloadLengthAlreadyDeclared =
+    'CCM payload length already declared as a different value';
+  sCCMPayloadLengthLocked =
+    'CCM payload length cannot be declared after Encode/Decode has started';
 
 procedure TCCM.Decode(Source, Dest: PUInt8Array; Size: Integer);
 begin
@@ -313,8 +323,16 @@ procedure TCCM.DeclarePayloadLength(const AByteLength: UInt64);
 begin
   CheckNotFinalized;
 
-  if FStarted or FPayloadLengthDeclared then
+  if FStarted then
+    raise EDECCipherException.CreateRes(@sCCMPayloadLengthLocked);
+
+  if FPayloadLengthDeclared then
+  begin
+    if AByteLength <> FExpectedPayloadLength then
+      raise EDECCipherException.CreateRes(@sCCMPayloadLengthAlreadyDeclared);
+
     Exit;
+  end;
 
   FExpectedPayloadLength := AByteLength;
   FPayloadLengthDeclared := True;

Unit Tests/Tests/TestDECCipherModesCCM.pas

@@ -47,6 +47,7 @@   TestTDECCCM = class(TTestCase)
     FTestDataList  : TAuthenticatedTestDataList;
     FCipherAES     : TCipher_AES;
     FTestBitLength : Integer; // AuthenticationBitLength for test for wring lengths
+    FTestPayloadLength : UInt64; // payload length used by CheckException helpers
   private
     function IsEqual(const a, b: TBytes): Boolean;
     procedure DoTestEncodeStream_LoadAndTestCAVSData(const aMaxChunkSize: Int64);
@@ -58,6 +59,7 @@   TestTDECCCM = class(TTestCase)
     procedure DoTestAuthenticationBitLengthWrong;
     procedure DoReadTagBeforeDone;
     procedure DoEncodeAfterDone;
+    procedure DoSetAuthenticatedPayloadLength;
   public
     procedure SetUp; override;
     procedure TearDown; override;
@@ -119,6 +121,26 @@   TestTDECCCM = class(TTestCase)
     ///   Several EncodeStream calls covering the message after declaring length.
     /// </summary>
     procedure TestEncodeStreamMultiChunk;
+    /// <summary>
+    ///   Re-declaring the same payload length is idempotent.
+    /// </summary>
+    procedure TestDeclarePayloadLengthIdempotent;
+    /// <summary>
+    ///   Re-declaring a different payload length must raise.
+    /// </summary>
+    procedure TestDeclarePayloadLengthDifferentRaises;
+    /// <summary>
+    ///   Declaring payload length after Encode has started must raise.
+    /// </summary>
+    procedure TestDeclarePayloadLengthAfterEncodeRaises;
+    /// <summary>
+    ///   Declaring payload length after Decode has started must raise.
+    /// </summary>
+    procedure TestDeclarePayloadLengthAfterDecodeRaises;
+    /// <summary>
+    ///   Declaring payload length after Done must raise via CheckNotFinalized.
+    /// </summary>
+    procedure TestDeclarePayloadLengthAfterDoneRaises;
   end;
 
 
@@ -966,6 +988,115 @@ procedure TestTDECCCM.TestEncodeStreamMultiChunk;
   DoTestEncodeStream_TestSingleSet(0, 0, 8);
 end;
 
+procedure TestTDECCCM.DoSetAuthenticatedPayloadLength;
+begin
+  FCipherAES.AuthenticatedPayloadLength := FTestPayloadLength;
+end;
+
+procedure TestTDECCCM.TestDeclarePayloadLengthIdempotent;
+var
+  TestData: TSingleAuthenticatedTestData;
+  PT: TBytes;
+begin
+  TestData := FTestDataList[0].TestData[0];
+  PT := TFormat_HexL.Decode(BytesOf(TestData.PT));
+
+  FCipherAES.Init(BytesOf(TFormat_HexL.Decode(TestData.CryptKey)),
+                   BytesOf(TFormat_HexL.Decode(TestData.InitVector)),
+                   $FF);
+  FCipherAES.AuthenticationResultBitLength := FTestDataList[0].Taglen;
+  FCipherAES.DataToAuthenticate := TFormat_HexL.Decode(BytesOf(TestData.AAD));
+
+  FCipherAES.AuthenticatedPayloadLength := UInt64(Length(PT));
+  FCipherAES.AuthenticatedPayloadLength := UInt64(Length(PT));
+
+  CheckEquals(Length(PT), Integer(FCipherAES.AuthenticatedPayloadLength),
+              'Re-declaring the same payload length must keep the declared value');
+end;
+
+procedure TestTDECCCM.TestDeclarePayloadLengthDifferentRaises;
+var
+  TestData: TSingleAuthenticatedTestData;
+  PT: TBytes;
+begin
+  TestData := FTestDataList[0].TestData[0];
+  PT := TFormat_HexL.Decode(BytesOf(TestData.PT));
+
+  FCipherAES.Init(BytesOf(TFormat_HexL.Decode(TestData.CryptKey)),
+                   BytesOf(TFormat_HexL.Decode(TestData.InitVector)),
+                   $FF);
+  FCipherAES.AuthenticationResultBitLength := FTestDataList[0].Taglen;
+  FCipherAES.DataToAuthenticate := TFormat_HexL.Decode(BytesOf(TestData.AAD));
+
+  FCipherAES.AuthenticatedPayloadLength := UInt64(Length(PT));
+  FTestPayloadLength := UInt64(Length(PT) + 1);
+  CheckException(DoSetAuthenticatedPayloadLength, EDECCipherException,
+                 'Re-declaring a different payload length must raise EDECCipherException');
+end;
+
+procedure TestTDECCCM.TestDeclarePayloadLengthAfterEncodeRaises;
+var
+  TestData: TSingleAuthenticatedTestData;
+  PT: TBytes;
+begin
+  TestData := FTestDataList[0].TestData[0];
+  PT := TFormat_HexL.Decode(BytesOf(TestData.PT));
+
+  FCipherAES.Init(BytesOf(TFormat_HexL.Decode(TestData.CryptKey)),
+                   BytesOf(TFormat_HexL.Decode(TestData.InitVector)),
+                   $FF);
+  FCipherAES.AuthenticationResultBitLength := FTestDataList[0].Taglen;
+  FCipherAES.DataToAuthenticate := TFormat_HexL.Decode(BytesOf(TestData.AAD));
+  FCipherAES.EncodeBytes(PT);
+
+  FTestPayloadLength := UInt64(Length(PT));
+  CheckException(DoSetAuthenticatedPayloadLength, EDECCipherException,
+                 'Declaring payload length after Encode has started must raise EDECCipherException');
+end;
+
+procedure TestTDECCCM.TestDeclarePayloadLengthAfterDecodeRaises;
+var
+  TestData: TSingleAuthenticatedTestData;
+  CT: TBytes;
+begin
+  TestData := FTestDataList[0].TestData[0];
+  CT := TFormat_HexL.Decode(BytesOf(TestData.CT));
+
+  FCipherAES.Init(BytesOf(TFormat_HexL.Decode(TestData.CryptKey)),
+                   BytesOf(TFormat_HexL.Decode(TestData.InitVector)),
+                   $FF);
+  FCipherAES.AuthenticationResultBitLength := FTestDataList[0].Taglen;
+  FCipherAES.DataToAuthenticate := TFormat_HexL.Decode(BytesOf(TestData.AAD));
+  FCipherAES.ExpectedAuthenticationResult :=
+    TFormat_HexL.Decode(BytesOf(TestData.TagResult));
+  FCipherAES.DecodeBytes(CT);
+
+  FTestPayloadLength := UInt64(Length(CT));
+  CheckException(DoSetAuthenticatedPayloadLength, EDECCipherException,
+                 'Declaring payload length after Decode has started must raise EDECCipherException');
+end;
+
+procedure TestTDECCCM.TestDeclarePayloadLengthAfterDoneRaises;
+var
+  TestData: TSingleAuthenticatedTestData;
+  PT: TBytes;
+begin
+  TestData := FTestDataList[0].TestData[0];
+  PT := TFormat_HexL.Decode(BytesOf(TestData.PT));
+
+  FCipherAES.Init(BytesOf(TFormat_HexL.Decode(TestData.CryptKey)),
+                   BytesOf(TFormat_HexL.Decode(TestData.InitVector)),
+                   $FF);
+  FCipherAES.AuthenticationResultBitLength := FTestDataList[0].Taglen;
+  FCipherAES.DataToAuthenticate := TFormat_HexL.Decode(BytesOf(TestData.AAD));
+  FCipherAES.EncodeBytes(PT);
+  FCipherAES.Done;
+
+  FTestPayloadLength := UInt64(Length(PT));
+  CheckException(DoSetAuthenticatedPayloadLength, EDECCipherException,
+                 'Declaring payload length after Done must raise EDECCipherException');
+end;
+
 initialization
   // Register all test cases to be run
   {$IFDEF DUnitX}