Update AES Encryption to 256bits for script + step package bootstrappers #1405
Conversation
flin-8
changed the title
| @@ -17,9 +32,10 @@ public class AesEncryption | |||
|
|
|||
| readonly byte[] key; | |||
|
|
|||
| public AesEncryption(string password) | |||
| public AesEncryption(string password, int keySizeBits) | |||
There was a problem hiding this comment.
Making the key size configurable allows us to upgrade the 3 components (scripts, step package, variables file) separately.
There was a problem hiding this comment.
The flexibility of this is nice, I wonder if this might open up the opportunity for the wrong one to be used in the wrong place.
It might be better to use inheritance here
keep AesEncryption abstract, and impliment the Bootstrapper and Script one passing through these key sizes required.
There was a problem hiding this comment.
Changed to factory methods for the scenarios and making the encryption key public on the encryptor
|
|
||
| readonly int keySizeBits; | ||
|
|
||
| const int BlockSizeBits = 128; |
There was a problem hiding this comment.
Block size can remain at 128
| public static readonly int ScriptBootstrapKeySize = 256; | ||
|
|
||
| //Key size used to decrypt the variables file sent by Octopus Server | ||
| public static readonly int VariablesFileKeySize = 128; |
There was a problem hiding this comment.
SaST will update this once they sort out variable encryption on the Server side. Currently it's encrypted by Tentacle which sounds wrong
| @@ -31,7 +31,7 @@ static PythonBootstrapper() | |||
|
|
|||
| public static string FormatCommandArguments(string bootstrapFile, string? scriptParameters) | |||
| { | |||
| var encryptionKey = ToHex(AesEncryption.GetEncryptionKey(SensitiveVariablePassword)); | |||
| var encryptionKey = ToHex(AesEncryption.GetEncryptionKey(SensitiveVariablePassword, AesEncryption.ScriptBootstrapKeySize)); | |||
There was a problem hiding this comment.
Python's decryption doens't specify bitness:
flin-8
changed the title
| @@ -17,9 +32,10 @@ public class AesEncryption | |||
|
|
|||
| readonly byte[] key; | |||
|
|
|||
| public AesEncryption(string password) | |||
| public AesEncryption(string password, int keySizeBits) | |||
There was a problem hiding this comment.
The flexibility of this is nice, I wonder if this might open up the opportunity for the wrong one to be used in the wrong place.
It might be better to use inheritance here
keep AesEncryption abstract, and impliment the Bootstrapper and Script one passing through these key sizes required.
[sc-98850]
Depends on https://github.com/OctopusDeploy/step-api/pull/569
Relates to: OctopusDeploy/Issues#9183
This PR updates the variable encryption from 128 to 256bits for: