Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Runner.Common/HostContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public sealed class HostContext : EventListener, IObserver<DiagnosticListener>,
private static int[] _vssHttpCredentialEventIds = new int[] { 11, 13, 14, 15, 16, 17, 18, 20, 21, 22, 27, 29 };
private readonly ConcurrentDictionary<Type, object> _serviceInstances = new();
private readonly ConcurrentDictionary<Type, Type> _serviceTypes = new();
private readonly ISecretMasker _secretMasker = new SecretMasker();
private readonly ISecretMasker _secretMasker = new SecretMasker(new RunnerSecretRegistrationNotifier());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there might be a better approach for doing this, we probably just want to send over the data after we get them from the job message, so we don't have to touch secretmasker since its job is to mask.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a couple of different secrets I'm worried about, which I think come from different places:

I think we want to make sure we're getting context from all 3 sources, but also I'm very new to this codebase.

private readonly List<ProductInfoHeaderValue> _userAgents = new() { new ProductInfoHeaderValue($"GitHubActionsRunner-{BuildConstants.RunnerPackage.PackageName}", BuildConstants.RunnerPackage.Version) };
private CancellationTokenSource _runnerShutdownTokenSource = new();
private object _perfLock = new();
Expand Down
150 changes: 150 additions & 0 deletions src/Runner.Common/RunnerSecretRegistrationNotifier.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
using System;
using System.Collections.Generic;
using Microsoft.Win32.SafeHandles;
using System.Net;
using System.Net.Sockets;
using System.Runtime.InteropServices;
using System.Text;
using System.Text.Json;

namespace GitHub.Runner.Common
{
public sealed class RunnerSecretRegistrationNotifier : ISecretRegistrationNotifier
{
private const AddressFamily VsockAddressFamily = (AddressFamily)40;
internal bool IsLinux { get; }
private readonly Socket _vsock;
private readonly object _vsockSendLock = new object();

public RunnerSecretRegistrationNotifier()
{
IsLinux = RuntimeInformation.IsOSPlatform(OSPlatform.Linux);
_vsock = IsLinux ? TryCreateConnectedVsock() : null;
}

public void NotifySecretRegistration(List<string> secrets, List<string> secretRegexes)
{
if (_vsock == null)
{
return;
}

try
{
string jsonPayload = JsonSerializer.Serialize(new
{
RunnerSecrets = new
{
secrets = secrets ?? new List<string>(),
secretRegexes = secretRegexes ?? new List<string>()
}
});

byte[] payloadBytes = Encoding.UTF8.GetBytes(jsonPayload);
byte[] lengthPrefix = BitConverter.GetBytes(IPAddress.HostToNetworkOrder(payloadBytes.Length));

lock (_vsockSendLock)
{
SendAll(_vsock, lengthPrefix);
SendAll(_vsock, payloadBytes);
}
}
catch
{
// Notification delivery is best-effort and must never break secret masking.
}
}

private static Socket TryCreateConnectedVsock()
{
Socket socket = null;
try
{
SafeSocketHandle nativeSocket = NativeSocket((int)VsockAddressFamily, (int)SocketType.Stream, 0);
if (nativeSocket.IsInvalid)
{
int error = Marshal.GetLastPInvokeError();
nativeSocket.Dispose();
throw new SocketException(error);
}

socket = new Socket(nativeSocket);
socket.Connect(new HostVsockEndPoint(2, 9999));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we probably want to use some input to provide the value, so it's not hardcoded.

return socket;
}
catch
{
socket?.Dispose();
return null;
}
}

[DllImport("libc", SetLastError = true, EntryPoint = "socket")]
private static extern SafeSocketHandle NativeSocket(int domain, int type, int protocol);

private static void SendAll(Socket socket, byte[] buffer)
{
int offset = 0;
while (offset < buffer.Length)
{
int bytesSent = socket.Send(buffer, offset, buffer.Length - offset, SocketFlags.None);
if (bytesSent <= 0)
{
throw new SocketException((int)SocketError.ConnectionReset);
}

offset += bytesSent;
}
}

private sealed class HostVsockEndPoint : EndPoint
{
private const int SocketAddressSize = 16;
private readonly uint _cid;
private readonly uint _port;

public HostVsockEndPoint(uint cid, uint port)
{
_cid = cid;
_port = port;
}

public override AddressFamily AddressFamily => VsockAddressFamily;

public override SocketAddress Serialize()
{
SocketAddress socketAddress = new SocketAddress(AddressFamily.Unspecified, SocketAddressSize);
// sockaddr_vm layout: family(0-1), reserved1(2-3), port(4-7), cid(8-11)
ushort family = (ushort)AddressFamily;
socketAddress[0] = (byte)(family & 0xFF);
socketAddress[1] = (byte)((family >> 8) & 0xFF);
socketAddress[2] = 0;
socketAddress[3] = 0;
socketAddress[4] = (byte)(_port & 0xFF);
socketAddress[5] = (byte)((_port >> 8) & 0xFF);
socketAddress[6] = (byte)((_port >> 16) & 0xFF);
socketAddress[7] = (byte)((_port >> 24) & 0xFF);
socketAddress[8] = (byte)(_cid & 0xFF);
socketAddress[9] = (byte)((_cid >> 8) & 0xFF);
socketAddress[10] = (byte)((_cid >> 16) & 0xFF);
socketAddress[11] = (byte)((_cid >> 24) & 0xFF);
return socketAddress;
}

public override EndPoint Create(SocketAddress socketAddress)
{
uint port = (uint)socketAddress[4]
| ((uint)socketAddress[5] << 8)
| ((uint)socketAddress[6] << 16)
| ((uint)socketAddress[7] << 24);

uint cid = (uint)socketAddress[8]
| ((uint)socketAddress[9] << 8)
| ((uint)socketAddress[10] << 16)
| ((uint)socketAddress[11] << 24);

return new HostVsockEndPoint(cid, port);
}
}
}
}
23 changes: 23 additions & 0 deletions src/Sdk/DTLogging/Logging/ISecretRegistrationNotifier.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
using System.Collections.Generic;

namespace GitHub.DistributedTask.Logging
{
public interface ISecretRegistrationNotifier
{
void NotifySecretRegistration(List<string> secretValues, List<string> secretRegexes);
}

public sealed class NoOpSecretRegistrationNotifier : ISecretRegistrationNotifier
{
public static readonly NoOpSecretRegistrationNotifier Instance = new NoOpSecretRegistrationNotifier();

private NoOpSecretRegistrationNotifier()
{
}

public void NotifySecretRegistration(List<string> secretValues, List<string> secretRegexes)
{
// Intentionally empty. A concrete API caller can be wired later.
}
}
}
24 changes: 24 additions & 0 deletions src/Sdk/DTLogging/Logging/SecretMasker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,17 @@ namespace GitHub.DistributedTask.Logging
public sealed class SecretMasker : ISecretMasker, IDisposable
{
public SecretMasker()
: this(NoOpSecretRegistrationNotifier.Instance)
{
}

public SecretMasker(ISecretRegistrationNotifier secretRegistrationNotifier)
{
m_originalValueSecrets = new HashSet<ValueSecret>();
m_regexSecrets = new HashSet<RegexSecret>();
m_valueEncoders = new HashSet<ValueEncoder>();
m_valueSecrets = new HashSet<ValueSecret>();
m_secretRegistrationNotifier = secretRegistrationNotifier ?? NoOpSecretRegistrationNotifier.Instance;
}

private SecretMasker(SecretMasker copy)
Expand All @@ -30,6 +36,7 @@ private SecretMasker(SecretMasker copy)
m_regexSecrets = new HashSet<RegexSecret>(copy.m_regexSecrets);
m_valueEncoders = new HashSet<ValueEncoder>(copy.m_valueEncoders);
m_valueSecrets = new HashSet<ValueSecret>(copy.m_valueSecrets);
m_secretRegistrationNotifier = copy.m_secretRegistrationNotifier;
}
finally
{
Expand Down Expand Up @@ -66,6 +73,8 @@ public void AddRegex(String pattern)
m_lock.ExitWriteLock();
}
}

NotifySecretRegistration(new List<string>(), new List<string> { pattern });
}
Comment on lines 74 to 78

/// <summary>
Expand Down Expand Up @@ -133,6 +142,8 @@ public void AddValue(String value)
m_lock.ExitWriteLock();
}
}

NotifySecretRegistration(new List<string> { value }, new List<string>());
}

/// <summary>
Expand Down Expand Up @@ -293,6 +304,19 @@ public String MaskSecrets(String input)
private readonly HashSet<RegexSecret> m_regexSecrets;
private readonly HashSet<ValueEncoder> m_valueEncoders;
private readonly HashSet<ValueSecret> m_valueSecrets;
private readonly ISecretRegistrationNotifier m_secretRegistrationNotifier;
private ReaderWriterLockSlim m_lock = new ReaderWriterLockSlim(LockRecursionPolicy.NoRecursion);

private void NotifySecretRegistration(List<string> secretValues, List<string> secretRegexes)
{
try
{
m_secretRegistrationNotifier.NotifySecretRegistration(secretValues, secretRegexes);
Comment on lines +310 to +314
}
catch
{
// Notification failures must never break masking behavior.
}
}
}
}
Loading