-
Notifications
You must be signed in to change notification settings - Fork 116
Add a docker credential helper to avoid scary security warnings #1981
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
matt-richardson
wants to merge
90
commits into
main
Choose a base branch
from
mattr/docker-credential-helper-refactor
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
90 commits
Select commit
Hold shift + click to select a range
2bb8b15
Add a credential helper to avoid plain-text storage warnings
matt-richardson 4a48588
Improve debugability
matt-richardson 91e8425
Make it works
matt-richardson 5a456b9
Try fixing DockerCredentialScriptsFixture
matt-richardson 5f7d88b
Update DockerCredentialScriptsFixture.cs
matt-richardson d12b8c6
Merge branch 'main' into mattr/docker-credential-helper
matt-richardson aeb2bf8
Update DockerCredentialScriptsFixture.cs
matt-richardson 55abbf0
Update docker-credential-octopus.ps1
matt-richardson 2a3111b
Update DockerCredentialScriptsFixture.cs
matt-richardson 341a9e4
Update DockerCredentialScriptsFixture.cs
matt-richardson ca0e60e
Update docker-credential-octopus.ps1
matt-richardson 2c585bd
Merge branch 'main' into mattr/docker-credential-helper
matt-richardson 6f6a96c
Update Build.cs
matt-richardson f9dda25
Update Build.cs
matt-richardson 0a64a30
Update KubernetesContextScriptWrapperLiveFixture.cs
matt-richardson d021330
Update InstallTools.cs
matt-richardson 3f555d8
Ignore on CloudMac & AmazonLinux agent
matt-richardson 70f2b0d
Use vars from password store
matt-richardson af2d579
Merge branch 'main' into mattr/docker-credential-helper
matt-richardson 0e5ca6d
Fix bad merge
matt-richardson a233717
Fix bad merge
matt-richardson 09645a4
Undo changes to RequiresNonMacAttribute
matt-richardson ede2e57
Discard changes to source/Calamari.Testing/EnvironmentVariables.cs
matt-richardson f5c7961
Discard changes to source/Calamari.Testing/Requirements/RequiresNonMa…
matt-richardson bd67658
Discard changes to source/Calamari.Tests/Fixtures/Integration/Package…
matt-richardson 2b617bb
Discard changes to source/Calamari.Testing/EnvironmentVariables.cs
matt-richardson 5998e99
Discard changes to source/Calamari.Tests/KubernetesFixtures/Helm3Upgr…
matt-richardson e120f7c
Discard changes to source/Calamari.Tests/Fixtures/Integration/Package…
matt-richardson a483bda
Merge branch 'main' into mattr/docker-credential-helper
matt-richardson a13c75b
undo changes we dont want
matt-richardson d6c49a4
Discard changes to source/Calamari.Tests/KubernetesFixtures/InstallTo…
matt-richardson 2b2b59d
Restore BOM encoding for HelmChartPackageDownloaderFixture.cs
matt-richardson 9bb5fd8
Restore BOM encoding for Helm3UpgradeFixture.cs
matt-richardson 2186487
Restore BOM encoding for EnvironmentVariables.cs
matt-richardson 12155ad
Update DockerImagePackageDownloader.cs
matt-richardson cdc71f7
Merge branch 'main' into mattr/docker-credential-helper
matt-richardson 943282c
Merge branch 'main' into mattr/docker-credential-helper
matt-richardson a81fb15
Fix merge
matt-richardson f9133ad
Only use inMemorySink for this specific case
matt-richardson 0544cca
Update CommandResult.cs
matt-richardson e24ed46
Cleanup
matt-richardson 8e9d725
Discard changes to source/Calamari.Common/Features/Processes/CommandR…
matt-richardson ba83937
Discard changes to source/Calamari.Common/Features/Processes/CommandR…
matt-richardson 16899d2
Discard changes to source/Calamari.Shared/Integration/Processes/Libra…
matt-richardson b8ad51d
Discard changes to source/Calamari.Common/Features/Processes/CommandR…
matt-richardson 2640069
Discard changes to source/Calamari.Tests/Fixtures/Deployment/Conventi…
matt-richardson 9f959e5
Merge branch 'main' into mattr/docker-credential-helper
matt-richardson f6e9332
Add design doc for standalone Docker credential helper app
matt-richardson 913eeec
Refine fallback: string match is diagnostic, retain output capture
matt-richardson fc13d64
Add implementation plan for standalone Docker credential helper app
matt-richardson ff7084e
Extract DockerCredentialStore into Calamari.Common
matt-richardson ac0cf21
Clarify DockerCredentialStore.Get catch intent; add corrupt-file test
matt-richardson 129d6ec
Add Calamari.DockerCredentialHelper project and protocol handler
matt-richardson 47c6283
Handle malformed store input in DockerCredentialProtocol; add tests
matt-richardson 9e65f87
Add docker-credential-octopus entry point
matt-richardson 38b1675
Document why docker-credential-octopus requires DOCKER_CONFIG and OCT…
matt-richardson 62f17c0
Rewire downloader to use standalone credential helper binary
matt-richardson 77cc20d
Clear credential env on cleanup; capture stderr for login fallback di…
matt-richardson 732f14f
Revert invasive DeferredLogger plumbing; remove docker-credential sub…
matt-richardson 160e3df
Overlay docker-credential-octopus into Calamari publish output
matt-richardson 39defa4
Fail build if helper project missing; sign docker-credential-octopus …
matt-richardson a00dd7f
Clean up Docker credential helper test fixtures
matt-richardson 5902f90
Remove implementation plan doc from PR
matt-richardson 4ea35a2
Remove design spec doc from PR
matt-richardson 38ae180
Revert unrelated BOM change to CommandResult.cs
matt-richardson 1ded4df
Merge remote-tracking branch 'origin/main' into mattr/docker-credenti…
matt-richardson 994bd40
Revert unrelated BOM change to PackagedScriptConventionFixture.cs
matt-richardson 1d6a487
Clean up Calamari.Tests.csproj merge artifacts; keep only the helper …
matt-richardson ab67070
Strip merge/whitespace artifacts from DockerImagePackageDownloaderFix…
matt-richardson 89f6ff6
Use an ephemeral random password for Docker credential encryption
matt-richardson 6178c81
Fix self-contained helper overlay; restore cleanup log; align test as…
matt-richardson 82d9397
Drop redundant credential pre-store in SetupCredentialHelper
matt-richardson 920223d
Always clean up Docker credential artifacts via try/finally
matt-richardson 1c8a6da
Address remaining review findings (list verb, login fallback, verifie…
matt-richardson a865fb1
Compare overlay files by version, not raw bytes
matt-richardson 19f9853
Stamp the helper publish with Calamari's version
matt-richardson 74c58fe
Include both versions in the overlay divergence error message
matt-richardson f110631
Make docker-credential-octopus a standalone trimmed binary in its own…
matt-richardson 6dd6380
Ship docker-credential-octopus in Calamari's top-level folder
matt-richardson 878dbc5
Fix credential-helper integration test: apply trim/single-file at pub…
matt-richardson d449406
Restore the executable bit on docker-credential-octopus before login
matt-richardson e80b41b
Adversarial-review hardening: UTF-8 helper IO, warn on plaintext fall…
matt-richardson 5669603
Revert credential-helper fallback log to Verbose
matt-richardson a83bb87
Restrict credential files to the owner (0600 file, 0700 dir)
matt-richardson 2f6ffd1
Refactor credential-file permission helpers
matt-richardson b722a66
Cleanup
matt-richardson 44ea463
Merge remote-tracking branch 'origin/main' into mattr/docker-credenti…
matt-richardson 8830f20
Address PR review: convert credential DTOs to records, trim comments
matt-richardson d1d402d
Remove unused param
matt-richardson aa8cb03
Merge remote-tracking branch 'origin/main' into mattr/docker-credenti…
matt-richardson File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
24 changes: 24 additions & 0 deletions
24
source/Calamari.Common/Features/Processes/InMemoryCommandOutputSink.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| using System; | ||
| using System.Text; | ||
| using Calamari.Common.Plumbing.Commands; | ||
|
|
||
| namespace Calamari.Common.Features.Processes | ||
| { | ||
| public class InMemoryCommandOutputSink : ICommandInvocationOutputSink | ||
| { | ||
| readonly StringBuilder stdOut = new StringBuilder(); | ||
| readonly StringBuilder stdErr = new StringBuilder(); | ||
| public string StdOut => stdOut.ToString(); | ||
| public string StdErr => stdErr.ToString(); | ||
|
|
||
| public void WriteInfo(string line) | ||
| { | ||
| stdOut.AppendLine(line); | ||
| } | ||
|
|
||
| public void WriteError(string line) | ||
| { | ||
| stdErr.AppendLine(line); | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| using System; | ||
| using System.IO; | ||
| using System.Security.Cryptography; | ||
| using System.Text; | ||
|
|
||
| namespace Calamari.DockerCredentialHelper | ||
| { | ||
| // Self-contained AES used only by docker-credential-octopus to protect the short-lived credential | ||
| // files it writes during a package acquisition. The helper both writes (on `docker login`) and | ||
| // reads (on `docker pull`) these files, so this does not need to interoperate with Calamari's AesEncryption | ||
| public class AesEncryption | ||
| { | ||
| const int KeySizeBits = 256; | ||
| const int BlockSizeBits = 128; | ||
| const int PasswordSaltIterations = 1000; | ||
| static readonly byte[] PasswordPaddingSalt = Encoding.UTF8.GetBytes("Octopuss"); | ||
| static readonly byte[] IvPrefix = Encoding.UTF8.GetBytes("IV__"); | ||
|
|
||
| readonly byte[] encryptionKey; | ||
|
|
||
| AesEncryption(string password) | ||
| { | ||
| encryptionKey = Rfc2898DeriveBytes.Pbkdf2(password, PasswordPaddingSalt, PasswordSaltIterations, HashAlgorithmName.SHA1, KeySizeBits / 8); | ||
| } | ||
|
|
||
| public static AesEncryption ForScripts(string password) => new AesEncryption(password); | ||
|
|
||
| public byte[] Encrypt(string plaintext) | ||
| { | ||
| var plainTextBytes = Encoding.UTF8.GetBytes(plaintext); | ||
| using var algorithm = CreateAlgorithm(); | ||
| using var encryptor = algorithm.CreateEncryptor(); | ||
| using var stream = new MemoryStream(); | ||
|
|
||
| // The IV is random per-encrypt and prepended (after a marker) so Decrypt can recover it. | ||
| stream.Write(IvPrefix, 0, IvPrefix.Length); | ||
| stream.Write(algorithm.IV, 0, algorithm.IV.Length); | ||
| using (var cryptoStream = new CryptoStream(stream, encryptor, CryptoStreamMode.Write)) | ||
| cryptoStream.Write(plainTextBytes, 0, plainTextBytes.Length); | ||
|
|
||
| return stream.ToArray(); | ||
| } | ||
|
|
||
| public string Decrypt(byte[] encrypted) | ||
| { | ||
| var aesBytes = ExtractIV(encrypted, out var iv); | ||
| using var algorithm = CreateAlgorithm(); | ||
| algorithm.IV = iv; | ||
| using var decryptor = algorithm.CreateDecryptor(); | ||
| using var memoryStream = new MemoryStream(aesBytes); | ||
| using var cryptoStream = new CryptoStream(memoryStream, decryptor, CryptoStreamMode.Read); | ||
| using var reader = new StreamReader(cryptoStream, Encoding.UTF8); | ||
| return reader.ReadToEnd(); | ||
| } | ||
|
|
||
| Aes CreateAlgorithm() | ||
| { | ||
| var algorithm = Aes.Create(); | ||
| algorithm.Mode = CipherMode.CBC; | ||
| algorithm.Padding = PaddingMode.PKCS7; | ||
| algorithm.KeySize = KeySizeBits; | ||
| algorithm.BlockSize = BlockSizeBits; | ||
| algorithm.Key = encryptionKey; | ||
| return algorithm; | ||
| } | ||
|
|
||
| static byte[] ExtractIV(byte[] encrypted, out byte[] iv) | ||
| { | ||
| var ivLength = BlockSizeBits / 8; | ||
| iv = new byte[ivLength]; | ||
| Buffer.BlockCopy(encrypted, IvPrefix.Length, iv, 0, ivLength); | ||
|
|
||
| var ivDataLength = IvPrefix.Length + ivLength; | ||
| var aesDataLength = encrypted.Length - ivDataLength; | ||
| var aesData = new byte[aesDataLength]; | ||
| Buffer.BlockCopy(encrypted, ivDataLength, aesData, 0, aesDataLength); | ||
| return aesData; | ||
| } | ||
| } | ||
| } |
23 changes: 23 additions & 0 deletions
23
source/Calamari.DockerCredentialHelper/Calamari.DockerCredentialHelper.csproj
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
|
|
||
| <PropertyGroup> | ||
| <OutputType>Exe</OutputType> | ||
| <AssemblyName>docker-credential-octopus</AssemblyName> | ||
|
APErebus marked this conversation as resolved.
|
||
| <RootNamespace>Calamari.DockerCredentialHelper</RootNamespace> | ||
| <TargetFramework>net8.0</TargetFramework> | ||
| <RuntimeIdentifiers>win-x64;linux-x64;osx-x64;linux-arm;linux-arm64</RuntimeIdentifiers> | ||
| <Nullable>enable</Nullable> | ||
| <TreatWarningsAsErrors>true</TreatWarningsAsErrors> | ||
| <IsPackable>false</IsPackable> | ||
| <!-- NOTE: trimming/single-file are applied at publish time by build/Build.PackageCalamariProjects.cs, | ||
| NOT here. Setting them as project properties leaks into how Calamari.Tests consumes this | ||
| project reference and produces a non-runnable apphost in the test output. --> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <!-- Match the System.Text.Json version the rest of the solution uses, so the assemblies in a | ||
| shared output directory (e.g. Calamari.Tests) agree with this project's deps.json. --> | ||
| <PackageReference Include="System.Text.Json" Version="9.0.16" /> | ||
| </ItemGroup> | ||
|
|
||
| </Project> | ||
33 changes: 33 additions & 0 deletions
33
source/Calamari.DockerCredentialHelper/CredentialModels.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| using System.Text.Json.Serialization; | ||
|
|
||
| namespace Calamari.DockerCredentialHelper | ||
| { | ||
| public record DockerCredential | ||
| { | ||
| public string Username { get; init; } = string.Empty; | ||
| public string Secret { get; init; } = string.Empty; | ||
| } | ||
|
|
||
| public record StoreRequest | ||
| { | ||
| public string ServerURL { get; init; } = string.Empty; | ||
| public string Username { get; init; } = string.Empty; | ||
| public string Secret { get; init; } = string.Empty; | ||
| } | ||
|
|
||
| public record GetResponse | ||
| { | ||
| public string ServerURL { get; init; } = string.Empty; | ||
| public string Username { get; init; } = string.Empty; | ||
| public string Secret { get; init; } = string.Empty; | ||
| } | ||
|
|
||
| // Source-generated serialization keeps System.Text.Json trim-safe (no reflection), so the | ||
| // published binary can be trimmed without losing (de)serialization of these types. | ||
| [JsonSerializable(typeof(DockerCredential))] | ||
| [JsonSerializable(typeof(StoreRequest))] | ||
| [JsonSerializable(typeof(GetResponse))] | ||
| internal partial class CredentialJsonContext : JsonSerializerContext | ||
| { | ||
| } | ||
| } |
96 changes: 96 additions & 0 deletions
96
source/Calamari.DockerCredentialHelper/DockerCredentialProtocol.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| using System; | ||
| using System.IO; | ||
| using System.Text.Json; | ||
|
|
||
| namespace Calamari.DockerCredentialHelper | ||
| { | ||
| public class DockerCredentialProtocol | ||
| { | ||
| readonly DockerCredentialStore store; | ||
|
|
||
| public DockerCredentialProtocol(DockerCredentialStore store) | ||
| { | ||
| this.store = store; | ||
| } | ||
|
|
||
| public int Run(string operation, TextReader input, TextWriter output, TextWriter error, string encryptionPassword, string dockerConfigPath) | ||
| { | ||
| switch (operation.ToLowerInvariant()) | ||
| { | ||
| case "store": | ||
| return Store(input, error, encryptionPassword, dockerConfigPath); | ||
| case "get": | ||
| return Get(input, output, error, encryptionPassword, dockerConfigPath); | ||
| case "erase": | ||
| return Erase(input, dockerConfigPath); | ||
| case "list": | ||
| return List(output); | ||
| default: | ||
| error.WriteLine($"Invalid operation: {operation}. Valid operations are: store, get, erase, list"); | ||
| return 1; | ||
| } | ||
| } | ||
|
|
||
| // Docker's 'list' expects a JSON map of ServerURL -> Username. We don't enumerate stored | ||
| // credentials (they're short-lived, per-acquisition), so we report none. | ||
| int List(TextWriter output) | ||
| { | ||
| output.WriteLine("{}"); | ||
| return 0; | ||
| } | ||
|
|
||
| // Docker sends a JSON object on stdin for 'store'. | ||
| int Store(TextReader input, TextWriter error, string encryptionPassword, string dockerConfigPath) | ||
| { | ||
| StoreRequest? request; | ||
| try | ||
| { | ||
| request = JsonSerializer.Deserialize(input.ReadToEnd(), CredentialJsonContext.Default.StoreRequest); | ||
| } | ||
| catch (Exception) | ||
| { | ||
| error.WriteLine("Invalid store request"); | ||
| return 1; | ||
| } | ||
|
|
||
| if (request == null || string.IsNullOrEmpty(request.ServerURL)) | ||
| { | ||
| error.WriteLine("Invalid store request"); | ||
| return 1; | ||
| } | ||
|
|
||
| store.Store(request.ServerURL, request.Username, request.Secret, encryptionPassword, dockerConfigPath); | ||
| return 0; | ||
| } | ||
|
|
||
| // Docker sends a bare server URL line on stdin for 'get' and 'erase'. | ||
| int Get(TextReader input, TextWriter output, TextWriter error, string encryptionPassword, string dockerConfigPath) | ||
| { | ||
| var serverUrl = input.ReadLine()?.Trim(); | ||
| if (string.IsNullOrEmpty(serverUrl)) | ||
| { | ||
| error.WriteLine("No server URL provided"); | ||
| return 1; | ||
| } | ||
|
|
||
| var credential = store.Get(serverUrl, encryptionPassword, dockerConfigPath); | ||
| if (credential == null) | ||
| { | ||
| error.WriteLine("credentials not found in native keychain"); | ||
| return 1; | ||
| } | ||
|
|
||
| var response = new GetResponse { ServerURL = serverUrl, Username = credential.Username, Secret = credential.Secret }; | ||
| output.WriteLine(JsonSerializer.Serialize(response, CredentialJsonContext.Default.GetResponse)); | ||
| return 0; | ||
| } | ||
|
|
||
| int Erase(TextReader input, string dockerConfigPath) | ||
| { | ||
| var serverUrl = input.ReadLine()?.Trim(); | ||
| if (!string.IsNullOrEmpty(serverUrl)) | ||
| store.Erase(serverUrl, dockerConfigPath); | ||
| return 0; | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.