Skip to content

Bring back Certificate provider parameters #10622

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.

Sign up for GitHub

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

Merged
Travis Plunk (TravisEz13) merged 3 commits into from
Jun 2, 2020
Merged

Bring back Certificate provider parameters #10622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c4f735e
Add ExpiringInDays filter in Certificate provider
iSazonov Sep 24, 2019
ee6f752
Fix test
iSazonov Mar 29, 2020
430fa2a
Merge branch 'master' into add-expiringindays
iSazonov May 31, 2020
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
257 changes: 219 additions & 38 deletions src/Microsoft.PowerShell.Security/security/CertificateProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,18 @@ namespace Microsoft.PowerShell.Commands
{
Comment thread
iSazonov marked this conversation as resolved.
Outdated
Comment thread
iSazonov marked this conversation as resolved.
Outdated
Comment thread
iSazonov marked this conversation as resolved.
Outdated
Comment thread
iSazonov marked this conversation as resolved.
Outdated
Comment thread
iSazonov marked this conversation as resolved.
Outdated
Comment thread
iSazonov marked this conversation as resolved.
/// <summary>
/// Defines the Certificate Provider dynamic parameters.
///
/// We only support one dynamic parameter for Win 7 and earlier:
/// CodeSigningCert
/// If provided, we only return certificates valid for signing code or
/// scripts.
/// </summary>

internal sealed class CertificateProviderCodeSigningDynamicParameters
internal sealed class CertificateProviderDynamicParameters
{
/// <summary>
/// Switch that controls whether we only return
/// Gets or sets a switch that controls whether we only return
/// code signing certs.
/// </summary>
[Parameter()]
[Parameter]
public SwitchParameter CodeSigningCert
{
get { return _codeSigningCert; }
Expand All @@ -53,6 +51,70 @@ public SwitchParameter CodeSigningCert
}

private SwitchParameter _codeSigningCert = new SwitchParameter();

/// <summary>
/// Gets or sets a filter that controls whether we only return
/// data encipherment certs.
/// </summary>
[Parameter]
public SwitchParameter DocumentEncryptionCert
{
get;
set;
}

/// <summary>
/// Gets or sets a filter that controls whether we only return
/// server authentication certs.
/// </summary>
[Parameter]
public SwitchParameter SSLServerAuthentication
{
get;
set;
}

/// <summary>
/// Gets or sets a filter by DNSName.
/// Expected content is a single DNS Name that may start and/or end
/// with '*': "contoso.com" or "*toso.c*".
/// All WildcardPattern class features supported.
/// </summary>
[Parameter]
public string DnsName
{
get;
set;
}

/// <summary>
/// Gets or sets a filter by EKU.
/// Expected content is one or more OID strings:
/// "1.3.6.1.5.5.7.3.1", "*Server*", etc.
/// For a cert to match, it must be valid for all listed OIDs.
/// All WildcardPattern class features supported.
/// </summary>
[Parameter]
public string[] Eku
{
get;
set;
}

/// <summary>
/// Gets or sets a filter by the number of valid days.
/// Expected content is a non-negative integer.
/// "0" matches all certs that have already expired.
/// "1" matches all certs that are currently valid and will expire
/// by next day (local time).
/// </summary>
[Parameter]
[ValidateRange(ValidateRangeKind.NonNegative)]
public int ExpiringInDays
{
get;
set;
} = -1;
}

/// <summary>
Expand Down Expand Up @@ -169,7 +231,7 @@ internal sealed class ProviderRemoveItemDynamicParameters
/// Switch that controls whether we should delete private key
/// when remove a certificate.
/// </summary>
[Parameter()]
[Parameter]
public SwitchParameter DeleteKey
{
get
Expand Down Expand Up @@ -1185,15 +1247,11 @@ protected override void GetItem(string path)
}
else
{
// The filter is non null. If the certificate
// satisfies the filter, output it. Otherwise, don't.

// The filter is non null. If the certificate
// satisfies the filter, output it. Otherwise, don't.
X509Certificate2 cert = item as X509Certificate2;
Dbg.Diagnostics.Assert(cert != null, "item should be a certificate");

// If it's Win8 or above, filter matching for certain properties is done by
// the certificate enumeration filter at the API level. In that case,
// filter.Purpose will be 'None' and MatchesFilter will return 'True'.
if (MatchesFilter(cert, filter))
{
WriteItemObject(item, path, isContainer);
Expand Down Expand Up @@ -2212,7 +2270,7 @@ protected override bool IsItemContainer(string path)
/// </returns>
protected override object GetItemDynamicParameters(string path)
{
return new CertificateProviderCodeSigningDynamicParameters();
return new CertificateProviderDynamicParameters();
}

/// <summary>
Expand All @@ -2234,7 +2292,7 @@ protected override object GetItemDynamicParameters(string path)
/// </returns>
protected override object GetChildItemsDynamicParameters(string path, bool recurse)
{
return new CertificateProviderCodeSigningDynamicParameters();
return new CertificateProviderDynamicParameters();
}

#endregion DriveCmdletProvider overrides
Expand Down Expand Up @@ -2607,15 +2665,49 @@ private CertificateFilterInfo GetFilter()

if (DynamicParameters != null)
{
CertificateProviderCodeSigningDynamicParameters dp =
DynamicParameters as CertificateProviderCodeSigningDynamicParameters;
CertificateProviderDynamicParameters dp =
DynamicParameters as CertificateProviderDynamicParameters;
if (dp != null)
{
if (dp.CodeSigningCert)
{
filter = new CertificateFilterInfo();
filter.Purpose = CertificatePurpose.CodeSigning;
}

if (dp.DocumentEncryptionCert)
{
filter = filter ?? new CertificateFilterInfo();
filter.Purpose = CertificatePurpose.DocumentEncryption;
}

if (dp.DnsName != null)
{
filter = filter ?? new CertificateFilterInfo();
filter.DnsName = new WildcardPattern(dp.DnsName, WildcardOptions.IgnoreCase);
}

if (dp.Eku != null)
{
filter = filter ?? new CertificateFilterInfo();
filter.Eku = new List<WildcardPattern>();
foreach (var pattern in dp.Eku)
{
filter.Eku.Add(new WildcardPattern(pattern, WildcardOptions.IgnoreCase));
}
}

if (dp.ExpiringInDays >= 0)
{
filter = filter ?? new CertificateFilterInfo();
filter.Expiring = DateTime.Now.AddDays(dp.ExpiringInDays);
}

if (dp.SSLServerAuthentication)
{
filter = filter ?? new CertificateFilterInfo();
filter.SSLServerAuthentication = true;
}
}
}

Expand All @@ -2634,42 +2726,131 @@ private bool IncludeArchivedCerts()
return includeArchivedCerts;
}

// If it's Win8 or above, filter matching for certain properties is done by
// the certificate enumeration filter at the API level. In that case,
// filter.Purpose will be 'None' and MatchesFilter will return 'True'.
private static bool MatchesFilter(X509Certificate2 cert,
CertificateFilterInfo filter)
private static bool MatchesFilter(X509Certificate2 cert, CertificateFilterInfo filter)
{
//
// no filter means, match everything
//
if ((filter == null) ||
(filter.Purpose == CertificatePurpose.NotSpecified) ||
(filter.Purpose == CertificatePurpose.All))
// No filter means, match everything
if (filter == null)
{
return true;
}

if (filter.Expiring > DateTime.MinValue && !SecuritySupport.CertExpiresByTime(cert, filter.Expiring))
{
return false;
}

if (filter.DnsName != null && !CertContainsName(cert, filter.DnsName))
{
return false;
}

if (filter.Eku != null && !CertContainsEku(cert, filter.Eku))
{
return false;
}

if (filter.SSLServerAuthentication && !CertIsSSLServerAuthentication(cert))
{
return false;
}

switch (filter.Purpose)
{
case CertificatePurpose.CodeSigning:
if (SecuritySupport.CertIsGoodForSigning(cert))
{
return true;
}
return SecuritySupport.CertIsGoodForSigning(cert);

case CertificatePurpose.DocumentEncryption:
return SecuritySupport.CertIsGoodForEncryption(cert);

case CertificatePurpose.NotSpecified:
case CertificatePurpose.All:
return true;

default:
break;
}

case CertificatePurpose.DocumentEncryption:
if (SecuritySupport.CertIsGoodForEncryption(cert))
return false;
}

/// <summary>
/// Check if the specified certificate has the name in DNS name list.
/// </summary>
/// <param name="cert">Certificate object.</param>
/// <param name="pattern">Wildcard pattern for DNS name to search.</param>
/// <returns>True on success, false otherwise.</returns>
internal static bool CertContainsName(X509Certificate2 cert, WildcardPattern pattern)

@TravisEz13 Travis Plunk (TravisEz13) Oct 4, 2019

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.

Have you verified this will work with punycode?

@iSazonov Ilya (iSazonov) Oct 4, 2019 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Do you mean that Windows PowerShell support also punycode in dnsname filter? I did not find this in docs https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.security/about/about_certificate_provider?view=powershell-6

What is a scenario where it could be used?

@TravisEz13 Travis Plunk (TravisEz13) Oct 4, 2019

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.

When the name in the cert is in punycode. This is a compliance requirement. you deleted the code to do this.

@iSazonov Ilya (iSazonov) Oct 4, 2019

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Clear about compliance. Not clear what code do you mean.
Also docs say:

DnsName <Microsoft.PowerShell.Commands.DnsNameRepresentation>
This parameter gets certificates that have the specified domain name or name pattern in the DNSNameList property of the certificate. The value of this parameter can either be "Unicode" or "ASCII". Punycode values are converted to Unicode. Wildcard characters (*) are permitted.

It is not clear how punycode converted to Unicode. It seems this did unpublic code (in P/Invoke) which we removed long ago.

@iSazonov Ilya (iSazonov) Oct 8, 2019

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I looked original code which was removed and see only one difference - DNSName parameter was DnsNameRepresentation type, now string.
In both cases DnsNameRepresentation is initialized by one constructor with string parameter which assigned to DNSname and punycode - no conversion. So new code works for punycode.
Question is should we change type from string to DnsNameRepresentation?

{
List<DnsNameRepresentation> list = (new DnsNameProperty(cert)).DnsNameList;
foreach (DnsNameRepresentation dnsName in list)
{
if (pattern.IsMatch(dnsName.Unicode))
Comment thread
TravisEz13 marked this conversation as resolved.
Outdated
{
return true;
}
}

return false;
}

/// <summary>
/// Check if the specified certificate is a server authentication certificate.
/// </summary>
/// <param name="cert">Certificate object.</param>
/// <returns>True on success, false otherwise.</returns>
internal static bool CertIsSSLServerAuthentication(X509Certificate2 cert)
{
X509ExtensionCollection extentionList = cert.Extensions;
foreach (var extension in extentionList)
{
if (extension is X509EnhancedKeyUsageExtension eku)
{
foreach (Oid usage in eku.EnhancedKeyUsages)
{
return true;
if (usage.Value.Equals(CertificateFilterInfo.OID_PKIX_KP_SERVER_AUTH, StringComparison.Ordinal))
{
return true;
}
}
}
}

break;
return false;
}

default:
break;
/// <summary>
/// Check if the specified certificate contains EKU matching all of these patterns.
/// </summary>
/// <param name="cert">Certificate object.</param>
/// <param name="ekuPatterns">EKU patterns.</param>
/// <returns>True on success, false otherwise.</returns>
internal static bool CertContainsEku(X509Certificate2 cert, List<WildcardPattern> ekuPatterns)
{
X509ExtensionCollection extensionList = cert.Extensions;
foreach (var extension in extensionList)
{
if (extension is X509EnhancedKeyUsageExtension eku)
{
OidCollection enhancedKeyUsages = eku.EnhancedKeyUsages;
foreach (WildcardPattern ekuPattern in ekuPatterns)
{
bool patternPassed = false;
foreach (var usage in enhancedKeyUsages)
{
if (ekuPattern.IsMatch(usage.Value) || ekuPattern.IsMatch(usage.FriendlyName))
{
return true;
}
}

if (!patternPassed)

@rjmholt Rob Holt (rjmholt) Nov 8, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can remove this

{
return false;
}
}

return true;

@rjmholt Rob Holt (rjmholt) Nov 8, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return true;
return false;

}
}

return false;
Expand Down Expand Up @@ -3195,7 +3376,7 @@ public List<DnsNameRepresentation> DnsNameList
}

/// <summary>
/// Constructor for EkuList.
/// Constructor for DnsNameProperty.
/// </summary>
public DnsNameProperty(X509Certificate2 cert)
{
Expand Down
Loading