RestPS icon indicating copy to clipboard operation
RestPS copied to clipboard

Improvements to Certificate functions

Open matsmcp opened this issue 1 year ago • 5 comments

I did take a look at the certificate functions in Invoke-VerifyRootCA.ps1 and I would like to suggest a number of changes.

This is based on a little more complex CA environment with 2 levels of CA server. First we have a root CA. Secondly we have 2 intermediate CA:s (lets say one for servers and one for clients)

The server running RestPS has a certificate issued by the server intermediate CA and the client connection to RestPS has a certificate issued by client intermediate CA.

The current code will (as I read the code) take the Authority Key Identifier (Oid "2.5.29.35") from the server certificate and compare it with the same Oid from the client connecting.

The Issue with this is that Oid "2.5.29.35" of the server cert will be the Subject Key Identifier (Oid "2.5.29.14") of the server intermediate CA certificate and for the client it will be the same of the client intermediate CA, hence the two will not match and the code will reject the cert.

I have a couple of ideas how to improve this.

The first is to add a Get-restCA.ps1 function that would allow the listing of accepted CA values. It would also mean that the server cert could be decoupled from the auth function. We could also make the current code recursive to find the true root and check that but I don't like that approach since it doesn't decouple the auth code from the cert that is used to run the server. A recursive cert check will also take a little bit of time.

The second is to use the test-certificate commandlet. It will verify that the calling cert is valid. For example the current code will accept an expired certificate. Test-certificate will not, it also seems faster than a recursive search. I'm seeing two options, either drop invalid connections or adding a $script:CertISValid boolean variable and let the specific script decide if it should accept it or not

I will be playing around a little bit with this

matsmcp avatar Jun 17 '24 11:06 matsmcp

By changing from dot sourcing . $RestPSLocalRoot\bin\Get-RestCAList.ps1 $CAList= Invoke-VerifyRootCA to do
$CAList=Get-Content $RestPSLocalRoot\bin\calist.txt
I get huge performance improvements. From between 380-520ms down to below 15 ms.

matsmcp avatar Jun 18 '24 10:06 matsmcp

Wow! That's a fantastic improvement!

jpsider avatar Jun 18 '24 12:06 jpsider

Yes. I was surprised myself. needs more testing to be 100% sure but there are articles about dot sourcing being a little slow.

I just found/made an alternative I'm testing at the moment. Instead of . C:\subscript.ps1

I'm doing Invoke-Expression (get-content -Path C:\subscript.ps1 -raw) Seems to behave the same way but loads much faster

Edit: Removed a unneeded step

matsmcp avatar Jun 19 '24 07:06 matsmcp

A draft of a new function. Intended as a replacement for Invoke-VerifyRootCA. It is decoupled from the cert running the server. It loads faster It can also use test-certificate to validate that the client cert is valid and trusted by the machine (according to docs)

function Invoke-VerifyCAList
{

  if ($null -ne $script:ClientCert){ 
    
    # Get the list over allowed CA:s
    $CaList=""
    Invoke-Expression (get-content -Path $RestPSLocalRoot\bin\Get-RestCAList.ps1 -raw)
    $CaList=Get-restCAList
	Write-Log -LogFile $Logfile -LogLevel $logLevel -MsgType INFO -Message "Invoke-VerifyCA: CAList is: $CaList"
    
    if ($null -ne $CaList){
      
      #Get the value from the oid
      $LocalCAIdentifier = ($script:ClientCert.Extensions | Where-Object {$_.oid.value -eq "2.5.29.35"}).format(0)
      $LocalCAIdentifier=$LocalCAIdentifier.Substring($LocalCAIdentifier.IndexOf('=')+1)
      Write-Log -LogFile $Logfile -LogLevel $logLevel -MsgType INFO -Message "Invoke-VerifyCA: $LocalCAIdentifier"
      
      if ($null -ne $LocalCAIdentifier){
      
        if ($CAList.contains($LocalCAIdentifier)){
          Write-Log -LogFile $Logfile -LogLevel $logLevel -MsgType INFO -Message "Invoke-VerifyCA: CA is Verified."
          
          # Run the cert through test-certificate if check-certificate is enabled
          if(check-certificate -eq $true){
      
            if(Test-Certificate $script:ClientCert -ErrorAction SilentlyContinue -WarningAction SilentlyContinue){
              Write-Log -LogFile $Logfile -LogLevel $logLevel -MsgType INFO -Message "Invoke-VerifyCA: certificate is Verified."
              $script:VerifyStatus = $true
              }
            else
              {
              Write-Log -LogFile $Logfile -LogLevel $logLevel -MsgType INFO -Message "Invoke-VerifyCA: The Certificate is not valid."
              $script:VerifyStatus = $false
              }
            
            }
          else
            {
            Write-Log -LogFile $Logfile -LogLevel $logLevel -MsgType INFO -Message "Invoke-VerifyCA: Skipping Certificate validation."
            $script:VerifyStatus = $true
            }  
               
          }
        Else{
          Write-Log -LogFile $Logfile -LogLevel $logLevel -MsgType INFO -Message "Invoke-VerifyCA: CA is not allowed."
          $script:VerifyStatus = $false
          }

        }
      Else
        {
        Write-Log -LogFile $Logfile -LogLevel $logLevel -MsgType INFO -Message "Invoke-VerifyCA: Client cert does not have a CA."
        }
        
      }
    Else
      {
      Write-Log -LogFile $Logfile -LogLevel $logLevel -MsgType INFO -Message "Invoke-VerifyCA: No ACL list available."
      }
    
    }
  else
    {
    Write-Log -LogFile $Logfile -LogLevel $logLevel -MsgType INFO -Message "Invoke-VerifyCA: The Client did not present a certificate."
    }
 
  $script:VerifyStatus
}		

Should be used together with a Get-RestCAList.ps1

function Get-RestCAList
{
    # This Function represents a way to Gather a List of accepted CA:s.
    # it uses the value from OID 2.5.29.14 of the CA Certificate
    
    $CAList = @('Testvalue','Testvalue2')
    $CAList
}

function check-certificate
{
    #Set to true to enable certificate validation
    #$checkcertificate=$false
    $checkcertificate=$true
    $checkcertificate
}

matsmcp avatar Jun 19 '24 11:06 matsmcp

I look forward to the pull request!

jpsider avatar Jun 20 '24 19:06 jpsider

implemented in https://github.com/jpsider/RestPS/pull/89

matsmcp avatar Sep 13 '24 13:09 matsmcp