Skip to content
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

Please fall back to SHA256 for channel binding when hash unsupported #63

Closed
ecbftw opened this issue Feb 13, 2020 · 4 comments · Fixed by #78
Closed

Please fall back to SHA256 for channel binding when hash unsupported #63

ecbftw opened this issue Feb 13, 2020 · 4 comments · Fixed by #78

Comments

@ecbftw
Copy link

ecbftw commented Feb 13, 2020

Hi there.

I've been trying to log in to hosts that have some odd certificates. These certificates use a hash algorithm of RSASSA-PSS (oid: 1.2.840.113549.1.1.10). This algorithm is known to pyca/cryptography when parsing a certificate, but there is no implementation of that algorithm in the library. For that reason, when setting up channel binding, the current _get_certificate_hash implementation in negotiate.py fails because of an exception when fetching the hash object. The exception is something like Signature algorithm OID:<...> not recognized

I was able to work around this issue by changing negotiate.py to fall back to SHA256 when the signature algorithm is not recognized (similar to how MD5 and SHA1 are handled now). Here is a suggested patch (sorry, too busy to do a pull request right now).

--- negotiate.py.old	2020-02-12 19:11:17.939188351 -0800
+++ negotiate.py	2020-02-12 19:13:09.343513262 -0800
@@ -217,19 +217,16 @@
         backend = default_backend()
 
         cert = x509.load_der_x509_certificate(certificate_der, backend)
-
+        hash_algorithm = None
         try:
             hash_algorithm = cert.signature_hash_algorithm
         except UnsupportedAlgorithm as ex:
-            warning = "Failed to get the signature algorithm from the " \
-                      "certificate, unable to pass channel bindings data: %s" \
-                      % str(ex)
+            warning = "Failed to get the signature algorithm from the certificate due to: %s" % str(ex)
             warnings.warn(warning, UnknownSignatureAlgorithmOID)
-            return None
 
-        # if the cert signature algorithm is either md5 or sha1 then use sha256
+        # if the cert signature algorithm is md5, sha1, or something we can't generate, then use sha256
         # otherwise use the signature algorithm of the cert itself
-        if hash_algorithm.name in ['md5', 'sha1']:
+        if not hash_algorithm or (hash_algorithm.name in ['md5', 'sha1']):
             digest = hashes.Hash(hashes.SHA256(), backend)
         else:
             digest = hashes.Hash(hash_algorithm, backend)
@jborean93
Copy link
Owner

Thanks for picking this up, I can't say I've tested with this type of certificate before but the logic is sound to me. Hopefully I'll get around to putting it in a PR soon unless you can find the time beforehand.

@jborean93
Copy link
Owner

I've used the below script to generate a self signed cert with the RSASSA-PSA algorithm for testing

$ErrorActionPreference = 'Stop'

$subjectName = New-Object -ComObject X509Enrollment.CX500DistinguishedName
$subjectName.Encode('CN=server2019.domain.local', 0)

$privateKey = New-Object -ComObject X509Enrollment.CX509PrivateKey
$privateKey.ProviderName = "Microsoft Enhanced RSA and AES Cryptographic Provider"
$privateKey.KeySpec = 1
$privateKey.Length = 4096
$privateKey.SecurityDescriptor = "D:PAI(A;;0xd01f01ff;;;SY)(A;;0xd01f01ff;;;BA)(A;;0x80120089;;;NS)"
$privateKey.MachineContext = 1
$privateKey.Create()

$serverAuthOID = New-Object -ComObject X509Enrollment.CObjectId
$serverAuthOID.InitializeFromValue("1.3.6.1.5.5.7.3.1")

$ekuoids = New-Object -ComObject X509Enrollment.CObjectIds
$ekuoids.Add($serverAuthOID)

$ekuExtension = New-Object -ComObject X509Enrollment.CX509ExtensionEnhancedKeyUsage
$ekuExtension.InitializeEncode($ekuoids)

$names = @($env:COMPUTERNAME, ([System.Net.Dns]::GetHostByName($env:COMPUTERNAME).Hostname))
$altNames = New-Object -ComObject X509Enrollment.CAlternativeNames
foreach ($name in $names) {
    $altName = New-Object -ComObject X509Enrollment.CAlternativeName
    $altName.InitializeFromString(0x3, $name)
    $altNames.Add($altName)
}
$altNamesExtension = New-Object -ComObject X509Enrollment.CX509ExtensionAlternativeNames
$altNamesExtension.InitializeEncode($altNames)

$digitalSignature = [Security.Cryptography.X509Certificates.X509KeyUsageFlags]::DigitalSignature
$keyEncipherment = [Security.Cryptography.X509Certificates.X509KeyUsageFlags]::KeyEncipherment
$keyUsage = [int]($digitalSignature -bor $keyEncipherment)
$keyUsageExtension = New-Object -ComObject X509Enrollment.CX509ExtensionKeyUsage
$keyUsageExtension.InitializeEncode($keyUsage)
$keyUsageExtension.Critical = $true

$signatureOID = New-Object -ComObject X509Enrollment.CObjectId
$sha256OID = New-Object -TypeName Security.Cryptography.Oid -ArgumentList "SHA256"
$signatureOID.InitializeFromValue($sha256OID.Value)

$certificate = New-Object -ComObject X509Enrollment.CX509CertificateRequestCertificate
$certificate.InitializeFromPrivateKey(2, $privateKey, "")
$certificate.Subject = $subjectName
$certificate.Issuer = $certificate.Subject
$certificate.NotBefore = (Get-Date).AddDays(-1)
$certificate.NotAfter = $certificate.NotBefore.AddDays(365)
$certificate.X509Extensions.Add($keyUsageExtension)
$certificate.X509Extensions.Add($altNamesExtension)
$certificate.X509Extensions.Add($ekuExtension)
$certificate.SignatureInformation.AlternateSignatureAlgorithm = $true
$certificate.SignatureInformation.HashAlgorithm = $signatureOID
$certificate.Encode()

$enrollment = New-Object -ComObject X509Enrollment.CX509Enrollment
$enrollment.InitializeFromRequest($certificate)
$certificateData = $enrollment.CreateRequest(0)
$enrollment.InstallResponse(2, $certificateData, 0, "")

$parsedCertificate = New-Object -TypeName System.Security.Cryptography.X509Certificates.X509Certificate2
$parsedCertificate.Import([System.Text.Encoding]::UTF8.GetBytes($certificateData))

$parsedCertificate.Thumbprint

With the changes in #78 authentication now works in this scenario. Even if the HashAlgorithm specified is something like SHA512, the actual hash algorithm is still meant to be SHA256 so the changes in that PR work well.

Thanks for reporting the issue, will close this issue once 78 is merged.

@jborean93
Copy link
Owner

Thanks for the report and apologies for the delay, this has been implemented with #78.

@ecbftw
Copy link
Author

ecbftw commented Jul 24, 2020

Great, thanks so much for taking care of this. Sorry I never got around to making it a proper PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants