fix: Changed exception for invalid characters in Base32 encodings to …

…FormatException. chore: Clearer structure of decoding of encrypted string.
dbsystel · May 12, 2021 · 49eb77a · 49eb77a
1 parent 8c280e4
commit 49eb77a
Show file tree

Hide file tree

Showing 7 changed files with 80 additions and 46 deletions.
diff --git a/.gitignore b/.gitignore
@@ -342,3 +342,7 @@ ASALocalRun/
 
 # BeatPulse healthcheck temp database
 healthchecksdb
+
+# No nuget data
+*.nuget
+*.nuspec
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,16 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [2.0.6] - 2021-05-12
+
+### Changed
+- Changed exception for invalid characters in Base32 encodings to FormatException.
+
+## [2.0.5] - 2021-05-11
+
+### Changed
+- Clearer structure of getting encryption data from string.
+
 ## [2.0.4] - 2021-01-04
 
 ### Changed

diff --git a/TUPWLib/Arrays/Base32Encoding.vb b/TUPWLib/Arrays/Base32Encoding.vb
@@ -1,5 +1,5 @@
 '
-' SPDX-FileCopyrightText: 2020 DB Systel GmbH
+' SPDX-FileCopyrightText: 2021 DB Systel GmbH
 '
 ' SPDX-License-Identifier: Apache-2.0
 '
@@ -18,12 +18,13 @@
 '
 ' Author: Frank Schwab, DB Systel GmbH
 '
-' Version: 1.1.0
+' Version: 1.2.0
 '
 ' Change history:
 '    2020-11-12: V1.0.0: Created.
 '    2020-11-13: V1.1.0: Use unified method for mapping tables.
 '    2021-01-04: V1.1.1: Corrected typo.
+'    2021-05-12: V1.2.0: Throw correct exception on invalid character.
 '
 
 ''' <summary>
@@ -131,8 +132,9 @@ Public NotInheritable Class Base32Encoding
    ''' </summary>
    ''' <param name="encodedValue">The Base32 string to decode.</param>
    ''' <returns>The decoded Base32 string as a byte array.</returns>
-   ''' <exception cref="ArgumentException">Thrown if <paramref name="encodedValue"/> contains an invalid Base32 character, or has an invalid length.</exception>
+   ''' <exception cref="ArgumentException">Thrown if <paramref name="encodedValue"/> has an invalid length.</exception>
    ''' <exception cref="ArgumentNullException">Thrown if <paramref name="encodedValue"/> is <c>Nothing</c>.</exception>
+   ''' <exception cref="FormatException">Thrown if <paramref name="encodedValue"/> contains an invalid character.</exception>
    Public Shared Function Decode(encodedValue As String) As Byte()
       Return DecodeNewBufferWithMapping(encodedValue, RFC_4648_CHAR_TO_VALUE)
    End Function
@@ -143,8 +145,9 @@ Public NotInheritable Class Base32Encoding
    ''' <param name="encodedValue">The Base32 string to decode.</param>
    ''' <param name="destinationBuffer">Byte array where the decoded values are placed.</param>
    ''' <returns>The length of the bytes written into the destination buffer.</returns>
-   ''' <exception cref="ArgumentException">Thrown if <paramref name="encodedValue"/> contains an invalid Base32 character, or has an invalid length.</exception>
+   ''' <exception cref="ArgumentException">Thrown if <paramref name="encodedValue"/> has an invalid length.</exception>
    ''' <exception cref="ArgumentNullException">Thrown if <paramref name="encodedValue"/> is <c>Nothing</c>.</exception>
+   ''' <exception cref="FormatException">Thrown if <paramref name="encodedValue"/> contains an invalid character.</exception>
    Public Shared Function Decode(encodedValue As String, destinationBuffer As Byte()) As Integer
       Return DecodeExistingBufferWithMapping(encodedValue, destinationBuffer, RFC_4648_CHAR_TO_VALUE)   ' destinationBuffer is checked in the called method
    End Function
@@ -156,6 +159,7 @@ Public NotInheritable Class Base32Encoding
    ''' <returns>The decoded spell-safe Base32 string as a byte array.</returns>
    ''' <exception cref="ArgumentException">Thrown if <paramref name="encodedValue"/> contains an invalid spell-safe Base32 character, or has an invalid length.</exception>
    ''' <exception cref="ArgumentNullException">Thrown if <paramref name="encodedValue"/> is <c>Nothing</c>.</exception>
+   ''' <exception cref="FormatException">Thrown if <paramref name="encodedValue"/> contains an invalid character.</exception>
    Public Shared Function DecodeSpellSafe(encodedValue As String) As Byte()
       Return DecodeNewBufferWithMapping(encodedValue, SPELL_SAFE_CHAR_TO_VALUE)
    End Function
@@ -168,6 +172,7 @@ Public NotInheritable Class Base32Encoding
    ''' <returns>The length of the bytes written into the destination buffer.</returns>
    ''' <exception cref="ArgumentException">Thrown if <paramref name="encodedValue"/> contains an invalid Base32 character, or has an invalid length.</exception>
    ''' <exception cref="ArgumentNullException">Thrown if <paramref name="encodedValue"/> is <c>Nothing</c>.</exception>
+   ''' <exception cref="FormatException">Thrown if <paramref name="encodedValue"/> contains an invalid character.</exception>
    Public Shared Function DecodeSpellSafe(encodedValue As String, destinationBuffer As Byte()) As Integer
       Return DecodeExistingBufferWithMapping(encodedValue, destinationBuffer, SPELL_SAFE_CHAR_TO_VALUE)   ' destinationBuffer is checked in the called method
    End Function
@@ -224,6 +229,9 @@ Public NotInheritable Class Base32Encoding
    ''' </summary>
    ''' <param name="encodedValue">Encoded value to decode.</param>
    ''' <param name="mapCharToByte">Mapping table to use.</param>
+   ''' <exception cref="ArgumentException">Thrown if <paramref name="encodedValue"/> has an invalid length.</exception>
+   ''' <exception cref="ArgumentNullException">Thrown if <paramref name="encodedValue"/> is <c>Nothing</c>.</exception>
+   ''' <exception cref="FormatException">Thrown if <paramref name="encodedValue"/> contains an invalid character.</exception>
    ''' <returns>Newly created byte array with the decoded bytes.</returns>
    Private Shared Function DecodeNewBufferWithMapping(encodedValue As String, mapCharToByte As Byte()) As Byte()
       Dim byteCount As Integer = CheckEncodedValue(encodedValue)
@@ -242,6 +250,9 @@ Public NotInheritable Class Base32Encoding
    ''' <param name="encodedValue">Encoded value to decode.</param>
    ''' <param name="destinationBuffer">Byte array where the decoded values are placed.</param>
    ''' <param name="mapCharToByte">Mapping table to use.</param>
+   ''' <exception cref="ArgumentException">Thrown if <paramref name="encodedValue"/> has an invalid length.</exception>
+   ''' <exception cref="ArgumentNullException">Thrown if <paramref name="encodedValue"/> is <c>Nothing</c>.</exception>
+   ''' <exception cref="FormatException">Thrown if <paramref name="encodedValue"/> contains an invalid character.</exception>
    ''' <returns>Number of bytes in the <paramref name="destinationBuffer"/> that are filled.</returns>
    Private Shared Function DecodeExistingBufferWithMapping(encodedValue As String, destinationBuffer As Byte(), mapCharToByte As Byte()) As Integer
       Dim byteCount As Integer = CheckEncodedValue(encodedValue)
@@ -261,8 +272,7 @@ Public NotInheritable Class Base32Encoding
    ''' </summary>
    ''' <param name="encodedValue">The Base32 string to decode.</param>
    ''' <param name="mapCharToByte">Array with mappings from the character to the corresponding byte.</param>
-   ''' <exception cref="ArgumentException">Thrown if <paramref name="encodedValue"/> contains an invalid Base32 character, or has an invalid length.</exception>
-   ''' <exception cref="ArgumentNullException">Thrown if <paramref name="encodedValue"/> is <c>Nothing</c>.</exception>
+   ''' <exception cref="FormatException">Thrown if <paramref name="encodedValue"/> contains an invalid character.</exception>
    Private Shared Sub DecodeWorker(encodedValue As String, destinationBuffer As Byte(), byteCount As Integer, mapCharToByte As Byte())
       Dim actByte As Byte = 0
       Dim bitsRemaining As Byte = BITS_PER_BYTE
@@ -279,7 +289,11 @@ Public NotInheritable Class Base32Encoding
 
          If (bitsRemaining > BITS_PER_CHARACTER) Then
             mask = charValue << (bitsRemaining - BITS_PER_CHARACTER)
+            ' This is *not* a silly bit operation
+            ' SonarLint is silly in that it does not consider that this is done in a loop
+#Disable Warning S2437 ' Silly bit operations should not be performed
             actByte = actByte Or mask
+#Enable Warning S2437 ' Silly bit operations should not be performed
             bitsRemaining -= BITS_PER_CHARACTER
          Else
             mask = charValue >> (BITS_PER_CHARACTER - bitsRemaining)
@@ -310,6 +324,8 @@ Public NotInheritable Class Base32Encoding
    ''' <param name="aByteArray">The byte array to encode.</param>
    ''' <param name="mapByteToChar">Array with mappings from the byte to the corresponding character.</param>
    ''' <param name="withPadding"><c>True</c>: Result will be padded, <c>False</c>: Result will not be padded</param>
+   ''' <exception cref="ArgumentNullException">Thrown if <paramref name="aByteArray"/> is <c>Nothing</c>.</exception>
+   ''' <exception cref="InvalidOperationException">Thrown when there is a bug in the processing of the bytes.</exception>
    ''' <returns>The Base32 representation of the bytes in <paramref name="aByteArray"/>.</returns>
    Private Shared Function EncodeWorker(aByteArray As Byte(), mapByteToChar As Char(), withPadding As Boolean) As String
       Dim lastIndex As Integer
@@ -333,6 +349,7 @@ Public NotInheritable Class Base32Encoding
    ''' <param name="mapByteToChar">Array with mappings from the byte to the corresponding character.</param>
    ''' <returns>The encoded bytes as a string. Note that <paramref name="lastIndex"/> is also a return parameter.</returns>
    ''' <exception cref="ArgumentNullException">Thrown if <paramref name="aByteArray"/> is <c>Nothing</c>.</exception>
+   ''' <exception cref="InvalidOperationException">Thrown when there is a bug in the processing of the bytes.</exception>
    Private Shared Function EncodeInternal(aByteArray As Byte(), ByRef lastIndex As Integer, mapByteToChar As Char()) As Char()
       If aByteArray Is Nothing Then _
          Throw New ArgumentNullException(NameOf(aByteArray))
@@ -346,7 +363,11 @@ Public NotInheritable Class Base32Encoding
          Dim arrayIndex As Integer = 0
 
          For Each b As Byte In aByteArray
+            ' This is *not* a silly bit operation
+            ' SonarLint is silly in that it does not consider that this is done in a loop
+#Disable Warning S2437 ' Silly bit operations should not be performed
             actValue = actValue Or (b >> (BITS_PER_BYTE - bitsRemaining))
+#Enable Warning S2437 ' Silly bit operations should not be performed
             result(arrayIndex) = ValueToChar(actValue, mapByteToChar)
             arrayIndex += 1
 
@@ -383,6 +404,7 @@ Public NotInheritable Class Base32Encoding
    ''' </summary>
    ''' <param name="c">Character to convert.</param>
    ''' <param name="mapCharToByte">Map table for conversion.</param>
+   ''' <exception cref="FormatException">Thrown when the character <paramref name="c"/> is not a valid character for the mapping <paramref name="mapCharToByte"/>.</exception>
    ''' <returns>Value corresponding to character <paramref name="c"/>.</returns>
    Private Shared Function CharToValue(c As Char, mapCharToByte As Byte()) As Byte
       Dim index As Integer = Asc(c) - CODEPOINT_ZERO
@@ -393,11 +415,10 @@ Public NotInheritable Class Base32Encoding
          If result <> INVALID_CHARACTER_VALUE Then
             Return result
          Else
-            Throw New ArgumentException(ERROR_TEXT_INVALID_CHARACTER, NameOf(c))
+            Throw New FormatException(ERROR_TEXT_INVALID_CHARACTER)
          End If
-
       Else
-         Throw New ArgumentException(ERROR_TEXT_INVALID_CHARACTER, NameOf(c))
+         Throw New FormatException(ERROR_TEXT_INVALID_CHARACTER)
       End If
    End Function
 
@@ -406,12 +427,13 @@ Public NotInheritable Class Base32Encoding
    ''' </summary>
    ''' <param name="b">Value to map</param>
    ''' <param name="mapByteToChar">Map table for conversion.</param>
+   ''' <exception cref="InvalidOperationException">Thrown when the byte <paramref name="b"/> is not a valid byte for the mapping <paramref name="mapByteToChar"/>.</exception>
    ''' <returns>Character corresponding to value <paramref name="b"/>.</returns>
    Private Shared Function ValueToChar(b As Byte, mapByteToChar As Char()) As Char
       If b < mapByteToChar.Length Then
          Return mapByteToChar(b)
       Else
-         Throw New ArgumentException(ERROR_TEXT_INVALID_BYTE_VALUE, NameOf(b))
+         Throw New InvalidOperationException(ERROR_TEXT_INVALID_BYTE_VALUE)
       End If
    End Function
 #End Region
@@ -421,9 +443,9 @@ Public NotInheritable Class Base32Encoding
    ''' Checks if <paramref name="encodedValue"/> has a valid length and returns it, if it has one.
    ''' </summary>
    ''' <param name="encodedValue">The encoded value to check.</param>
-   ''' <returns>The number of decoded bytes in the encdoed value.</returns>
    ''' <exception cref="ArgumentException">Thrown if <paramref name="encodedValue"/> has an invalid length.</exception>
    ''' <exception cref="ArgumentNullException">Thrown if <paramref name="encodedValue"/> is <c>Nothing</c>.</exception>
+   ''' <returns>The number of decoded bytes in the encoded value.</returns>
    Private Shared Function CheckEncodedValue(encodedValue As String) As Integer
       If encodedValue Is Nothing Then _
          Throw New ArgumentNullException(NameOf(encodedValue))

diff --git a/TUPWLib/Crypto/SplitKeyEncryption.vb b/TUPWLib/Crypto/SplitKeyEncryption.vb
@@ -18,7 +18,7 @@
 '
 ' Author: Frank Schwab, DB Systel GmbH
 '
-' Version: 2.0.4
+' Version: 2.0.6
 '
 ' Change history:
 '    2020-05-05: V1.0.0: Created.
@@ -37,14 +37,16 @@
 '    2020-12-16: V2.0.2: Made usage of SyncLock for disposal consistent and changed some message creations.
 '    2021-01-04: V2.0.3: Fixed some error messages.
 '    2021-01-04: V2.0.4: Corrected naming of some methods and improved error handling.
+'    2021-05-11: V2.0.5: Clearer structure of getting the encryption parts from string.
+'    2021-05-11: V2.0.6: Corrected exception for Base32Encoding.
 '
 
 Imports System.IO
 Imports System.Security.Cryptography
 Imports System.Text
 
 ''' <summary>
-''' SplitKeyEncryption
+''' Encryption with key splitting
 ''' </summary>
 Public Class SplitKeyEncryption : Implements IDisposable
 #Region "Private constants"
@@ -662,39 +664,33 @@ Public Class SplitKeyEncryption : Implements IDisposable
 
       Dim paddedSourceBytes As Byte() = Nothing
 
-      Dim aesCipher As AesCng = New AesCng() With {
-         .Mode = CipherMode.CBC,
-         .Padding = PaddingMode.None   ' Never use any of the standard paddings!!!! They are all susceptible to a padding oracle.
-         }
-
-      Dim blockSizeInBytes As Integer = aesCipher.BlockSize >> 3
-
-      result.iv = GetInitializationVector(blockSizeInBytes)
-
-      '
-      ' This is a "try-catch" block instead of a "using" construct because we need to clear the contents of
-      ' an array before handling the exception. A "using" construct would only dispose of the "aesCipher".
       '
       ' There is no "finally" statement as this will only be executed if there is a wrapping "try-catch" block.
       ' To ensure that the array is cleared the corresponding statements are duplicated in the "try"
       ' and the "catch" part.
       '
       Try
-         paddedSourceBytes = PadSourceBytes(sourceBytes, blockSizeInBytes)
+         Using aesCipher As AesCng = New AesCng() With {
+            .Mode = CipherMode.CBC,
+            .Padding = PaddingMode.None   ' Never use any of the standard paddings!!!! They are all susceptible to a padding oracle.
+            }
+
+            Dim blockSizeInBytes As Integer = aesCipher.BlockSize >> 3
 
-         ' Encrypt the source string with the iv
-         result.encryptedData = GetEncryptedBytes(aesCipher, paddedSourceBytes, key, result.iv)
+            result.iv = GetInitializationVector(blockSizeInBytes)
 
-         ArrayHelper.Clear(paddedSourceBytes)   ' Clear sensitive data
+            paddedSourceBytes = PadSourceBytes(sourceBytes, blockSizeInBytes)
 
-         aesCipher.Dispose()
+            ' Encrypt the source string with the iv
+            result.encryptedData = GetEncryptedBytes(aesCipher, paddedSourceBytes, key, result.iv)
+
+            ArrayHelper.Clear(paddedSourceBytes)   ' Clear sensitive data
+         End Using
 
       Catch ex As Exception
          If paddedSourceBytes IsNot Nothing Then _
             ArrayHelper.Clear(paddedSourceBytes)   ' Clear sensitive data
 
-         aesCipher.Dispose()
-
          Throw
       End Try
 
@@ -814,7 +810,7 @@ Public Class SplitKeyEncryption : Implements IDisposable
          If ep IsNot Nothing Then _
             ep.Zap()
 
-         Throw New ArgumentException("Invalid Base64 encoding", ex)
+         Throw New ArgumentException("Invalid Base32/Base64 encoding", ex)
 
       Catch ex As Exception
          ' Ensure sensitive data are cleared

diff --git a/TUPWLib/My Project/AssemblyInfo.vb b/TUPWLib/My Project/AssemblyInfo.vb
@@ -12,7 +12,7 @@ Imports System.Runtime.InteropServices
 <Assembly: AssemblyDescription(".Net implementation of the Technical User PassWord library (TUPW)")>
 <Assembly: AssemblyCompany("DB Systel GmbH")>
 <Assembly: AssemblyProduct("TUPW")>
-<Assembly: AssemblyCopyright("Copyright © 2020 DB Systel GmbH")>
+<Assembly: AssemblyCopyright("Copyright © 2021 DB Systel GmbH")>
 <Assembly: AssemblyTrademark("")>
 
 <Assembly: ComVisible(False)>
@@ -30,5 +30,5 @@ Imports System.Runtime.InteropServices
 ' You can specify all the values or you can default the Build and Revision Numbers 
 ' by using the '*' as shown below:
 
-<Assembly: AssemblyVersion("2.0.1.0")>
-<Assembly: AssemblyFileVersion("2.0.1.0")>
+<Assembly: AssemblyVersion("2.0.6.0")>
+<Assembly: AssemblyFileVersion("2.0.6.0")>
diff --git a/TUPWLibTest/Arrays/Base32EncodingTest.vb b/TUPWLibTest/Arrays/Base32EncodingTest.vb
@@ -18,10 +18,11 @@
 '
 ' Author: Frank Schwab, DB Systel GmbH
 '
-' Version: 1.0.0
+' Version: 1.1.0
 '
 ' Change history:
 '    2020-04-29: V1.0.0: Created.
+'    2021-05-12: V1.1.0: Adapt for changed exceptions.
 '
 
 ' Imports Microsoft.VisualStudio.TestTools.UnitTesting
@@ -419,7 +420,7 @@ Imports DB.BCM.TUPW
 
          Assert.Fail("Expected exception ArgumentException not thrown")
 
-      Catch ex As ArgumentException
+      Catch ex As FormatException
          ' Expected exception
 
       Catch ex As Exception
@@ -509,7 +510,7 @@ Imports DB.BCM.TUPW
 
          Assert.Fail("Expected exception ArgumentException not thrown")
 
-      Catch ex As ArgumentException
+      Catch ex As FormatException
          ' Expected exception
 
       Catch ex As Exception
@@ -525,7 +526,7 @@ Imports DB.BCM.TUPW
 
          Assert.Fail("Expected exception ArgumentException not thrown")
 
-      Catch ex As ArgumentException
+      Catch ex As FormatException
          ' Expected exception
 
       Catch ex As Exception
@@ -633,7 +634,7 @@ Imports DB.BCM.TUPW
 
          Assert.Fail("Expected exception ArgumentException not thrown")
 
-      Catch ex As ArgumentException
+      Catch ex As FormatException
          ' Expected exception
 
       Catch ex As Exception
@@ -705,7 +706,7 @@ Imports DB.BCM.TUPW
 
          Assert.Fail("Expected exception ArgumentException not thrown")
 
-      Catch ex As ArgumentException
+      Catch ex As FormatException
          ' Expected exception
 
       Catch ex As Exception
@@ -721,7 +722,7 @@ Imports DB.BCM.TUPW
 
          Assert.Fail("Expected exception ArgumentException not thrown")
 
-      Catch ex As ArgumentException
+      Catch ex As FormatException
          ' Expected exception
 
       Catch ex As Exception

diff --git a/TUPWLibTest/TUPWLibTest.vbproj b/TUPWLibTest/TUPWLibTest.vbproj
@@ -94,9 +94,6 @@
   <ItemGroup>
     <Reference Include="System" />
     <Reference Include="System.Core" />
-    <Reference Include="TUPWLib">
-      <HintPath>..\TUPWLib\bin\Debug\TUPWLib.dll</HintPath>
-    </Reference>
   </ItemGroup>
   <ItemGroup>
     <Import Include="Microsoft.VisualBasic" />
@@ -166,6 +163,10 @@
       <Project>{f3305e3a-bad3-4ca3-8431-2c96e346ef81}</Project>
       <Name>TUPWLib</Name>
     </ProjectReference>
+    <ProjectReference Include="..\TUPWLib\TUPWLib.vbproj">
+      <Project>{2e46a307-075e-4dc3-9d8a-f5fa7ba6acd4}</Project>
+      <Name>TUPWLib</Name>
+    </ProjectReference>
   </ItemGroup>
   <ItemGroup>
     <PackageReference Include="MSTest.TestAdapter">