refactor(adminui): cert manager review polish — char.IsAsciiHexDigit, filtered catch, TOCTOU note
This commit is contained in:
@@ -1,3 +1,4 @@
|
||||
using System.Security.Cryptography;
|
||||
using System.Security.Cryptography.X509Certificates;
|
||||
using Microsoft.Extensions.Configuration;
|
||||
|
||||
@@ -87,6 +88,9 @@ public sealed class CertificateStoreManager
|
||||
if (File.Exists(dest))
|
||||
dest = Path.Combine(destDir,
|
||||
$"{Path.GetFileNameWithoutExtension(src)}_{thumbprint}{Path.GetExtension(src)}");
|
||||
// Narrow TOCTOU window: if a concurrent admin action created `dest` between the
|
||||
// check above and here, File.Move throws — we surface that as a Fail (no data loss,
|
||||
// no overwrite of a different cert). Cert-store edits are rare manual operations.
|
||||
File.Move(src, dest);
|
||||
return CertActionResult.Ok();
|
||||
}
|
||||
@@ -110,7 +114,10 @@ public sealed class CertificateStoreManager
|
||||
if (string.Equals(cert.Thumbprint, thumbprint, StringComparison.OrdinalIgnoreCase))
|
||||
return file;
|
||||
}
|
||||
catch { /* ignore unreadable entries */ }
|
||||
catch (Exception ex) when (ex is CryptographicException or IOException or UnauthorizedAccessException)
|
||||
{
|
||||
/* ignore unreadable/corrupt entries — a bad DER must not abort enumeration */
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
@@ -118,5 +125,5 @@ public sealed class CertificateStoreManager
|
||||
private static bool IsValidThumbprint(string thumbprint) =>
|
||||
!string.IsNullOrEmpty(thumbprint)
|
||||
&& (thumbprint.Length == 40 || thumbprint.Length == 64)
|
||||
&& thumbprint.All(Uri.IsHexDigit);
|
||||
&& thumbprint.All(char.IsAsciiHexDigit);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user