Skip to content

Commit d538534

Browse files
BornToBeRootclaude
andcommitted
Fix: Address review feedback for RDAP (#3607)
- Referrals: pin the connection to the checked addresses (SocketsHttpHandler ConnectCallback) to prevent DNS rebinding to local/private addresses, fail closed if the host cannot be resolved; proxied connections (CONNECT) are checked before the request. - Run command / redirected data: detect the query type from the input instead of using the last selected type. - Notice links: use a keyboard accessible Hyperlink (Tab + Enter). - Clarify why zone IDs are removed from IPv6 input (RFC 9082 3.1.1). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 7cec5d8 commit d538534

4 files changed

Lines changed: 131 additions & 17 deletions

File tree

‎Source/NETworkManager.Models/RDAP/RDAPClient.cs‎

Lines changed: 71 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,13 @@ public class RDAPClient : SingletonBase<RDAPClient>
4343

4444
private readonly HttpClient _client;
4545

46+
/// <summary>
47+
/// Client for registrar referrals. The URL is sent by the registry and therefore untrusted. Direct connections
48+
/// are only made to the addresses that were checked when connecting (no second DNS lookup, see
49+
/// <see cref="ConnectToPublicAddressAsync" />), which prevents DNS rebinding to local or private addresses.
50+
/// </summary>
51+
private readonly HttpClient _referralClient;
52+
4653
#endregion
4754

4855
#region Constructor
@@ -59,9 +66,22 @@ public RDAPClient()
5966
Timeout = Timeout.InfiniteTimeSpan
6067
};
6168

62-
_client.DefaultRequestHeaders.UserAgent.ParseAdd("NETworkManager");
63-
_client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue(RDAPMediaType));
64-
_client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
69+
_referralClient = new HttpClient(new SocketsHttpHandler
70+
{
71+
AllowAutoRedirect = false,
72+
AutomaticDecompression = DecompressionMethods.All,
73+
ConnectCallback = ConnectToPublicAddressAsync
74+
})
75+
{
76+
Timeout = Timeout.InfiniteTimeSpan
77+
};
78+
79+
foreach (var client in new[] { _client, _referralClient })
80+
{
81+
client.DefaultRequestHeaders.UserAgent.ParseAdd("NETworkManager");
82+
client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue(RDAPMediaType));
83+
client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
84+
}
6585
}
6686

6787
#endregion
@@ -196,13 +216,19 @@ private async Task<RDAPResult> GetAsync(RDAPQuery query, string url, TimeSpan ti
196216

197217
try
198218
{
199-
response = await _client.GetAsync(currentUrl, timeoutSource.Token).ConfigureAwait(false);
219+
response = await (isReferral ? _referralClient : _client).GetAsync(currentUrl, timeoutSource.Token)
220+
.ConfigureAwait(false);
200221
body = await response.Content.ReadAsStringAsync(timeoutSource.Token).ConfigureAwait(false);
201222
}
202223
catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
203224
{
204225
throw;
205226
}
227+
catch (HttpRequestException ex) when (ex.InnerException is RDAPException inner)
228+
{
229+
// Blocked by ConnectToPublicAddressAsync.
230+
throw new RDAPException(inner.Kind, inner.Message, ex) { RequestUrl = currentUrl.AbsoluteUri };
231+
}
206232
catch (Exception ex) when (ex is HttpRequestException or OperationCanceledException)
207233
{
208234
var message = ex is OperationCanceledException ? $"Timeout after {timeout.TotalSeconds} s." : ex.Message;
@@ -261,17 +287,55 @@ private static async Task ValidateReferralTargetAsync(Uri url, CancellationToken
261287
{
262288
addresses = await Dns.GetHostAddressesAsync(url.DnsSafeHost, cancellationToken).ConfigureAwait(false);
263289
}
264-
catch (SocketException)
290+
catch (SocketException ex)
265291
{
266-
// The request itself fails with a proper network error.
267-
return;
292+
// Fail closed: a referral is only followed if its address could be checked.
293+
throw new RDAPException(RDAPErrorKind.Network, ex.Message, ex) { RequestUrl = url.AbsoluteUri };
268294
}
269295
}
270296

271297
if (addresses.Any(IsLocalOrPrivateAddress))
272298
throw new RDAPException(RDAPErrorKind.PrivateAddress, url.AbsoluteUri) { RequestUrl = url.AbsoluteUri };
273299
}
274300

301+
/// <summary>
302+
/// Connect callback of the referral client: resolves the host once, checks all addresses and connects to exactly
303+
/// these addresses, so a second DNS answer (DNS rebinding) cannot redirect the connection to a local or private
304+
/// address. TLS still validates the certificate against the host name. If a proxy is used, the connection goes to
305+
/// the proxy, which resolves the target itself; in this case the check in
306+
/// <see cref="ValidateReferralTargetAsync" /> applies.
307+
/// </summary>
308+
private static async ValueTask<Stream> ConnectToPublicAddressAsync(SocketsHttpConnectionContext context,
309+
CancellationToken cancellationToken)
310+
{
311+
var endPoint = context.DnsEndPoint;
312+
313+
// A tunnel (CONNECT) to the configured proxy. Referrals only use HTTPS, so a proxied connection is always a
314+
// tunnel.
315+
var isProxy = context.InitialRequestMessage.Method == HttpMethod.Connect;
316+
317+
var addresses = IPAddress.TryParse(endPoint.Host, out var ipAddress)
318+
? [ipAddress]
319+
: await Dns.GetHostAddressesAsync(endPoint.Host, cancellationToken).ConfigureAwait(false);
320+
321+
if (!isProxy && (addresses.Length == 0 || addresses.Any(IsLocalOrPrivateAddress)))
322+
throw new RDAPException(RDAPErrorKind.PrivateAddress, endPoint.Host);
323+
324+
var socket = new Socket(SocketType.Stream, ProtocolType.Tcp) { NoDelay = true };
325+
326+
try
327+
{
328+
await socket.ConnectAsync(addresses, endPoint.Port, cancellationToken).ConfigureAwait(false);
329+
330+
return new NetworkStream(socket, true);
331+
}
332+
catch
333+
{
334+
socket.Dispose();
335+
throw;
336+
}
337+
}
338+
275339
/// <summary>
276340
/// Loopback, private (RFC 1918, RFC 4193), shared (RFC 6598), link-local, unspecified and multicast addresses.
277341
/// </summary>

‎Source/NETworkManager.Models/RDAP/RDAPQueryParser.cs‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,34 @@ public static RDAPQueryParseError TryParse(RDAPQueryType type, string input, out
4949
return query == null ? GetError(type) : RDAPQueryParseError.None;
5050
}
5151

52+
/// <summary>
53+
/// Detects the query type of an input without a type (e.g. data redirected from another tool): IP address or
54+
/// CIDR, AS number ("AS15169", "15169", "1.10"), TLD (single label), entity handle (single label with a hyphen,
55+
/// e.g. "ORG-RIEN1-RIPE"), otherwise domain.
56+
/// </summary>
57+
/// <param name="input">User input.</param>
58+
/// <returns>Detected query type.</returns>
59+
public static RDAPQueryType DetectType(string input)
60+
{
61+
input = input?.Trim() ?? string.Empty;
62+
63+
if (ParseIPAddress(input) != null)
64+
return RDAPQueryType.IPAddress;
65+
66+
// Digits only or with "AS" prefix. Asdot ("1.10") is also numeric, a TLD is never numeric.
67+
if (ASNRegex().IsMatch(input))
68+
return RDAPQueryType.ASN;
69+
70+
var label = input.Trim('.');
71+
72+
if (!label.Contains('.'))
73+
return label.Contains('-') && !label.StartsWith("xn--", StringComparison.OrdinalIgnoreCase)
74+
? RDAPQueryType.Entity
75+
: RDAPQueryType.TLD;
76+
77+
return RDAPQueryType.Domain;
78+
}
79+
5280
private static RDAPQueryParseError GetError(RDAPQueryType type)
5381
{
5482
return type switch
@@ -126,7 +154,9 @@ private static RDAPQuery ParseIPAddress(string input)
126154
if (parts.Length > 2)
127155
return null;
128156

129-
// Zone IDs (e.g. fe80::1%eth0) must not be used in RDAP queries (RFC 9082 3.1.1).
157+
// Zone IDs (e.g. fe80::1%eth0) must not be sent in RDAP queries (RFC 9082 3.1.1). They are only meaningful on
158+
// the local host and irrelevant for the registration data, so they are removed instead of rejecting the input
159+
// (link-local addresses are often copied with a zone ID, e.g. from ipconfig).
130160
var addressPart = parts[0].Split('%')[0];
131161

132162
// IPAddress.TryParse also accepts shortened IPv4 forms like "1" or "1.2", which are not valid here.

‎Source/NETworkManager/ViewModels/RDAPHostViewModel.cs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,15 @@ private static void CloseItemAction(ItemActionCallbackArgs<TabablzControl> args)
113113
/// Adds a new tab and starts the query, if one is given.
114114
/// </summary>
115115
/// <param name="query">Query (e.g. domain, IP address, AS number).</param>
116-
/// <param name="queryType">Type of the query. Uses the last selected type if null.</param>
116+
/// <param name="queryType">
117+
/// Type of the query. If null, the type is detected from the query (e.g. data redirected from another tool or a
118+
/// run command), or the last selected type is used for an empty tab.
119+
/// </param>
117120
public void AddTab(string query = null, RDAPQueryType? queryType = null)
118121
{
122+
if (queryType == null && !string.IsNullOrWhiteSpace(query))
123+
queryType = RDAPQueryParser.DetectType(query);
124+
119125
var tabId = Guid.NewGuid();
120126

121127
TabItems.Add(new DragablzTabItem(query ?? Strings.NewTab, new RDAPView(tabId, query, queryType),

‎Source/NETworkManager/Views/RDAPView.xaml‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,20 @@
4343
<Setter Property="CanUserAddRows" Value="False" />
4444
<Setter Property="SelectionMode" Value="Extended" />
4545
</Style>
46+
<!-- Keyboard accessible link (Tab + Enter), styled like LinkTextBlock -->
47+
<Style x:Key="RDAPHyperlink" TargetType="{x:Type Hyperlink}">
48+
<Setter Property="Foreground" Value="{DynamicResource MahApps.Brushes.Accent}" />
49+
<Setter Property="TextDecorations" Value="{x:Null}" />
50+
<Setter Property="Cursor" Value="Hand" />
51+
<Style.Triggers>
52+
<Trigger Property="IsMouseOver" Value="True">
53+
<Setter Property="Foreground" Value="{DynamicResource MahApps.Brushes.Accent2}" />
54+
</Trigger>
55+
<Trigger Property="IsKeyboardFocused" Value="True">
56+
<Setter Property="TextDecorations" Value="Underline" />
57+
</Trigger>
58+
</Style.Triggers>
59+
</Style>
4660
<Style x:Key="RDAPWrapColumnStyle" TargetType="{x:Type TextBlock}">
4761
<Setter Property="TextWrapping" Value="Wrap" />
4862
</Style>
@@ -158,15 +172,15 @@
158172
<ItemsControl ItemsSource="{Binding Links}">
159173
<ItemsControl.ItemTemplate>
160174
<DataTemplate DataType="{x:Type viewModels:RDAPLinkItem}">
161-
<TextBlock Text="{Binding Text}" ToolTip="{Binding Url}"
162-
Style="{StaticResource LinkTextBlock}"
175+
<TextBlock Style="{StaticResource DefaultTextBlock}"
163176
HorizontalAlignment="Left" TextWrapping="Wrap">
164-
<TextBlock.InputBindings>
165-
<MouseBinding
166-
Command="{Binding Data.OpenUrlCommand, Source={StaticResource BindingProxy}}"
167-
CommandParameter="{Binding Url}"
168-
MouseAction="LeftClick" />
169-
</TextBlock.InputBindings>
177+
<Hyperlink
178+
Command="{Binding Data.OpenUrlCommand, Source={StaticResource BindingProxy}}"
179+
CommandParameter="{Binding Url}"
180+
ToolTip="{Binding Url}"
181+
Style="{StaticResource RDAPHyperlink}">
182+
<Run Text="{Binding Text, Mode=OneWay}" />
183+
</Hyperlink>
170184
</TextBlock>
171185
</DataTemplate>
172186
</ItemsControl.ItemTemplate>

0 commit comments

Comments
 (0)