jpsider / RestPS

Simple ReST Framework for Powershell
MIT License
112 stars 30 forks source link

Improvements to Certificate functions #86

Open matsmcp opened 1 month ago

matsmcp commented 1 month ago

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 commented 4 weeks ago

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.

jpsider commented 4 weeks ago

Wow! That's a fantastic improvement!

matsmcp commented 4 weeks ago

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 commented 4 weeks ago

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
}
jpsider commented 3 weeks ago

I look forward to the pull request!