modify Add-SslCert logic to allow for certificate updates #993

natesuver · 2021-10-27T15:22:16Z

Add-SslCert was modified to accommodate a few different cases related to certificate binding:

If the specified host/ip and port has nothing bound to it, add a new binding.
If the specified host/ip and port is bound, and that certificate thumbprint does not match the new thumbprint, update the certificate thumbprint to match the new one.
If the specified host/ip and port is bound, and that certificate thumbprint matches the new thumbprint, do nothing.

This addresses an issue with this task where updating a certificate thumbprint from the build pipeline caused netsh to fail, if that binding already existed.

ghost · 2021-10-27T15:22:28Z

All CLA requirements met.

PawelHaracz

I will only change these two things but looks good

PawelHaracz · 2021-10-27T18:21:23Z

TaskModules/powershell/TaskModuleIISManageUtility/AppCmdOnTargetMachines.ps1

-        $isItSameBinding = $result.Get(4).Contains([string]::Format("{0}:{1}", $ipAddress, $port))
-
-        $addCertCmd = [string]::Format("http add sslcert ipport={0}:{1} certhash={2} appid={{{3}}} certstorename=MY", $ipAddress, $port, $certhash, [System.Guid]::NewGuid().toString())
+        $certCmd = Get-Netsh-Command -port $port -newCertHash $certhash -keyName "ipport" -hostOrIp $hostname        


it can be a one-line, I prefer to have something like that:

$port = "ipport" if($sni -eq "true" -and $iisVersion -ge 8 -and -not [string]::IsNullOrWhiteSpace($hostname)) { $port = "hostnameport" } $certCmd = Get-Netsh-Command -port $port -newCertHash $certhash -keyName $port -hostOrIp $hostname

yeah, good catch, I like that, less nesting

PawelHaracz · 2021-10-27T18:23:33Z

TaskModules/powershell/TaskModuleIISManageUtility/AppCmdOnTargetMachines.ps1

+            return [string]::Format("http update sslcert {4}={0}:{1} certhash={2} appid='{3}' certstorename=MY", $hostOrIp, $port, $certhash, $applicationId, $keyName) #TODO: this won't work with older versions of netsh, add something here to check the netsh version.
+        } else { #Case 3: the certificate bound to this host/ip and port has the same thumbprint as the new certificate.  Do nothing.
+            return [string]::Empty
+        }


you can remove this else statement and leave only return [string]::Empty

altwohill · 2021-12-03T02:03:10Z

When might we expect this fix to go live?

natesuver · 2021-12-04T14:34:46Z

We're still testing it, and have been busy on other projects, but will pick this up sometime after the new year. Notably, netsh update won't work with older versions of that utility, but I'm not sure that matters here, because it never did that functionality anyway. It will still break for older netsh, but for a different reason. So not sure how much that matters to people.

r-t-m · 2022-01-20T16:58:09Z

@mericstam @tauhid621 @pavanvamsi3 @MOlausson any chance this PR could be reviewed and merged?

jepperip · 2022-03-25T14:39:54Z

This is a major pain for us. Since the task is failing any appcmd commands are not run afterwards.

jiggybyte · 2022-05-19T21:19:24Z

Can we get the fixes for server 2022 published please? This is a seriously annoying issue.

milliamp · 2022-09-06T06:35:16Z

This also fixes the problem in newer Server versions (ie 2022) where you can't deploy the same SSL binding twice even with no changes as the number of new lines in the netsh output has changed breaking the logic. (microsoft/azure-pipelines-tasks#13398)

It would also fix:
microsoft/azure-pipelines-tasks#15419
https://developercommunity.visualstudio.com/t/iis-web-app-manage-task-should-replace-the-certifi/547395

Is there any help we can provide to get this one tested and merged soon?

Alternatively, should an interim pull request be written to handle the new lines issue independent of the remainder of the new code?

maximebakercssc · 2022-09-07T13:46:42Z

We are waiting for that fix to use our new servers... We are stuck because of that problem. We can't deploy our apps with SSL automatically and it is being really annoying.

leonvandebroek · 2022-11-01T10:05:12Z

Is there any indication of when the update that fixes this issue for WINS2022 will be available?

magahl · 2022-12-01T17:30:11Z

Yeah, will this be available soon? Alot of servers gonna be updated to Windows Server 2022, and this will fail every deployment that configures ssl bindings

PaulGauthier · 2022-12-28T21:41:34Z

Please consider testing/mergjng this soon to resolve #1008 . This is a bug with no workaround for Windows Server 2022 and only grows in impact as people upgrade their server OS.

morgannyman · 2023-01-11T13:02:11Z

Maybe I'm missing something but to me it seems that without fix of item 3 in the original post it's impossible to do CI/CD with Azure Pipelines and a IIS website deployment release pipeline/stage. First release deploys ok, but the next one stops on the "IIS Web App Manage" task which means that the "IIS Web App Deploy" task never runs.
This is the failing part of the log:

2023-01-11T11:27:08.3117321Z ##[command]"netsh" http show sslcert hostnameport=localhost:44901
2023-01-11T11:27:08.4282750Z ##[command]"netsh" http add sslcert hostnameport=localhost:44901 certhash=*** appid={f7f39b4b-8a63-44f5-befe-cdeb903344cb} certstorename=MY
2023-01-11T11:27:08.5167621Z 
2023-01-11T11:27:08.5168040Z SSL Certificate add failed, Error: 183
2023-01-11T11:27:08.5169413Z Cannot create a file when that file already exists.

Note that I'm deploying to my localhost and that for debug purposes I'm refering to the the IIS Express Development Certificate thumbprint in the binding information (https, ip=All unassigned, port=44901, hostname=localhost, SNI checked).

SDohle · 2023-01-16T12:04:53Z

+1

This prevents us to migrate to server 2022.
Please fix that @ Microsoft!

maximebakercssc · 2023-02-02T19:42:54Z

Nous allons bientôt migrer nos serveurs dans une solution cloud. Les serveurs devront être sous Windows Server 2022. Est-ce possible de nous donner une date pour la publication du correctif?

joeywas · 2023-03-03T03:20:14Z

We are experiencing this issue with pipelines that try to update bindings for an IIS website on Server 2022.

This pull request would also address two issues in the azure-pipelines-tasks repo:
microsoft/azure-pipelines-tasks#15686
microsoft/azure-pipelines-tasks#17462

@z00sts @mkonjikovac @DmitriiBobreshev @mmrazik May this pull request be approved/merged soon?

Thank you!

mmrazik · 2023-03-03T08:11:05Z

/azp run

azure-pipelines · 2023-03-03T08:11:14Z

Azure Pipelines successfully started running 1 pipeline(s).

jabteles · 2023-05-10T09:47:55Z

+1 preventing to migrate to Server 2022. No more workarounds.

foomaniac · 2023-05-19T13:25:18Z

Can't believe this is still an issue, fed up of the work around we have. Pleeease get this across the line!

anujmca · 2023-06-15T00:08:31Z

Do we have a solution to this, other than not migrating to windows server 2022?

patrickfegan · 2023-06-30T12:47:45Z

Do we have a solution to this, other than not migrating to windows server 2022?

As a temporary solution, I added a CI task to remove the binding before attempting the "IIS web app manage" task:

netsh http delete sslcert hostnameport=$(IISHostname):$(IISPort)

Replace IISHostname and IISPort with whatever value / pipeline variable required.

Hope this helps.

altwohill · 2023-06-30T22:03:14Z

How do you handle first runs on the pipeline? That step will fail if there is no cert. Ngā mihi, Al

…

________________________________ From: patrickfegan ***@***.***> Sent: Saturday, July 1, 2023 12:47:56 AM To: microsoft/azure-pipelines-extensions ***@***.***> Cc: Al Twohill ***@***.***>; Comment ***@***.***> Subject: Re: [microsoft/azure-pipelines-extensions] modify Add-SslCert logic to allow for certificate updates (PR #993) Do we have a solution to this, other than not migrating to windows server 2022? As a temporary solution, I added a CI task to remove the binding before attempting the "IIS web app manage" task: netsh http delete sslcert hostnameport=$(IISHostname):$(IISPort) Replace IISHostname and IISPort with whatever value / pipeline variable required. Hope this helps. — Reply to this email directly, view it on GitHub<#993 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAEZUBT3UIV4IBDADDZT2U3XN3DHZANCNFSM5G2UHJCQ>. You are receiving this because you commented.Message ID: ***@***.***>

jabteles · 2023-06-30T22:55:30Z

Custom PowerShell to remove cert if it exists Enviado de Outlook para Android<https://aka.ms/AAb9ysg>

…

________________________________ From: Al ***@***.***> Sent: Friday, June 30, 2023 11:03:27 PM To: microsoft/azure-pipelines-extensions ***@***.***> Cc: jabteles ***@***.***>; Comment ***@***.***> Subject: Re: [microsoft/azure-pipelines-extensions] modify Add-SslCert logic to allow for certificate updates (PR #993) How do you handle first runs on the pipeline? That step will fail if there is no cert. Ngā mihi, Al

________________________________ From: patrickfegan ***@***.***> Sent: Saturday, July 1, 2023 12:47:56 AM To: microsoft/azure-pipelines-extensions ***@***.***> Cc: Al Twohill ***@***.***>; Comment ***@***.***> Subject: Re: [microsoft/azure-pipelines-extensions] modify Add-SslCert logic to allow for certificate updates (PR #993) Do we have a solution to this, other than not migrating to windows server 2022? As a temporary solution, I added a CI task to remove the binding before attempting the "IIS web app manage" task: netsh http delete sslcert hostnameport=$(IISHostname):$(IISPort) Replace IISHostname and IISPort with whatever value / pipeline variable required. Hope this helps. — Reply to this email directly, view it on GitHub<#993 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAEZUBT3UIV4IBDADDZT2U3XN3DHZANCNFSM5G2UHJCQ>. You are receiving this because you commented.Message ID: ***@***.***> — Reply to this email directly, view it on GitHub<#993 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AIT3MFWZTWW5NTAF6OC7LR3XN5EK7ANCNFSM5G2UHJCQ>. You are receiving this because you commented.Message ID: ***@***.***>

patrickfegan · 2023-07-03T07:11:44Z

How do you handle first runs on the pipeline? That step will fail if there is no cert. Ngā mihi, Al
…
________________________________ From: patrickfegan @.> Sent: Saturday, July 1, 2023 12:47:56 AM To: microsoft/azure-pipelines-extensions @.> Cc: Al Twohill @.>; Comment @.> Subject: Re: [microsoft/azure-pipelines-extensions] modify Add-SslCert logic to allow for certificate updates (PR #993) Do we have a solution to this, other than not migrating to windows server 2022? As a temporary solution, I added a CI task to remove the binding before attempting the "IIS web app manage" task: netsh http delete sslcert hostnameport=$(IISHostname):$(IISPort) Replace IISHostname and IISPort with whatever value / pipeline variable required. Hope this helps. — Reply to this email directly, view it on GitHub<#993 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAEZUBT3UIV4IBDADDZT2U3XN3DHZANCNFSM5G2UHJCQ. You are receiving this because you commented.Message ID: @.***>

Have you tried anything? Tick the "continue on error" checkbox maybe?

joeywas · 2023-07-03T14:36:42Z

How do you handle first runs on the pipeline? That step will fail if there is no cert. Ngā mihi, Al
Have you tried anything? Tick the "continue on error" checkbox maybe?

The work around I used is inline powershell task that removes SSL cert bindings. It wraps the removals in try catch statements, and in the catch it does not throw an error. It will remove SSL cert bindings that were bound to an IP address or hostname.
At the end of the task, it forces a "clean" exit with [Environment]::Exit(0)

Here is the inline PS:

$appCmd = 'C:\Windows\system32\inetsrv\appcmd.exe'
Write-Host "[Start] removing sslcert binding from $(bindIPAddress):443 with netsh"
try {
    netsh http delete sslcert ipport=$(bindIPAddress):443
} catch {
    Write-Host $_
}
Write-Host "[Finish] removing sslcert binding from $(bindIPAddress):443 with netsh"
Write-Host "[Start] removing binding from $(dnsEntry):443 with appcmd"
try {
    . $appcmd set site /site.name:"$(dnsEntry)" /-bindings.[protocol='https',bindingInformation='*:443:$(dnsEntry)']
} catch {
    Write-Host $_
}
Write-Host "[Finish] removing binding from $(dnsEntry):443 with appcmd"
# Force clean exit no matter what happened previously
[Environment]::Exit(0)

edit: missing letters added

anoordover · 2023-09-01T05:42:56Z

+1 prevents migration to windows 2022

sticl · 2023-09-22T13:58:54Z

+1 please fix this

ImperatorRuscal

@PawelHaracz has a couple of recommended changes for better coding, but that do not change the functionality.
That said, this Pull will significantly improve the product (making it compatible with Server 2022) and further delay of release is not in the best interest of the community. Thus I recommend completing this Pull in order to benefit the community. (Or immediately implementing the noted changes, and then Pulling that.)

TyranVdMerwe · 2024-05-23T08:33:06Z

Here is the temp workaround that I used on Server 2022, only resolves the existing binding check issue.

$filePath = $(Get-ChildItem -Path "C:\agent\_work\_tasks" -Recurse -Filter "AppCmdOnTargetMachines.ps1").FullName

# Define key-value pairs for old and new strings
$replacementPairs = @{
    '$result.Get(4)' = '$result.Get(6)'
    '$result.Get(5)' = '$result.Get(7)'
}

$fileContent = Get-Content -Path $filePath
$modifiedContent = $fileContent

# Iterate through each pair and perform the replacement
foreach ($oldString in $replacementPairs.Keys) {
    $newString = $replacementPairs[$oldString]
    # Check if the replacement is necessary
    if ($fileContent -match [regex]::Escape($oldString)) {
        $fileContent = $fileContent -replace [regex]::Escape($oldString), $newString
        $changeDetected = $true
    }
}

# Write to file only if changes have been made
if ($changeDetected) {
    $fileContent | Set-Content -Path $filePath
}

Again not ideal by any means but at least allows the task to complete successfully.

The only remaining issue (for my use case) even after adding the workaround above is that AppCmdOnTargetMachines.ps1 does not cater for certificate update scenarios so hopefully the script is fixed in time for my next round of certificate updates :).

modify Add-SslCert logic to all cert updates

baca9b7

Nate Suver added 2 commits October 27, 2021 11:28

remove bad comment character

cddc080

correct function name typo

de375ca

natesuver changed the title ~~modify Add-SslCert logic to all cert updates~~ modify Add-SslCert logic to allow for certificate updates Oct 27, 2021

remove stupid comment

a623a57

PawelHaracz approved these changes Oct 27, 2021

View reviewed changes

remove else statement, reduce "if" nesting

25dd023

altwohill mentioned this pull request Nov 26, 2021

IISWebAppManagementOnMachineGroupV0 can't detect existing SSL bindings on Server 2022 #1008

Open

coolhome mentioned this pull request May 22, 2023

Support multiple certificate and different number of empty lines for netsh #1153

Open

ImperatorRuscal approved these changes Apr 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

modify Add-SslCert logic to allow for certificate updates #993

modify Add-SslCert logic to allow for certificate updates #993

natesuver commented Oct 27, 2021

ghost commented Oct 27, 2021 •

edited by ghost

Loading

PawelHaracz left a comment

PawelHaracz Oct 27, 2021

natesuver Oct 27, 2021

PawelHaracz Oct 27, 2021

altwohill commented Dec 3, 2021

natesuver commented Dec 4, 2021

r-t-m commented Jan 20, 2022

jepperip commented Mar 25, 2022

jiggybyte commented May 19, 2022

milliamp commented Sep 6, 2022 •

edited

Loading

maximebakercssc commented Sep 7, 2022 •

edited

Loading

leonvandebroek commented Nov 1, 2022

magahl commented Dec 1, 2022 •

edited

Loading

PaulGauthier commented Dec 28, 2022

morgannyman commented Jan 11, 2023 •

edited

Loading

SDohle commented Jan 16, 2023

maximebakercssc commented Feb 2, 2023

joeywas commented Mar 3, 2023

mmrazik commented Mar 3, 2023

azure-pipelines bot commented Mar 3, 2023

jabteles commented May 10, 2023

foomaniac commented May 19, 2023

anujmca commented Jun 15, 2023

patrickfegan commented Jun 30, 2023

altwohill commented Jun 30, 2023 via email

jabteles commented Jun 30, 2023 via email

patrickfegan commented Jul 3, 2023

joeywas commented Jul 3, 2023 •

edited

Loading

anoordover commented Sep 1, 2023

sticl commented Sep 22, 2023

ImperatorRuscal left a comment

TyranVdMerwe commented May 23, 2024 •

edited

Loading

modify Add-SslCert logic to allow for certificate updates #993

Are you sure you want to change the base?

modify Add-SslCert logic to allow for certificate updates #993

Conversation

natesuver commented Oct 27, 2021

ghost commented Oct 27, 2021 • edited by ghost Loading

PawelHaracz left a comment

Choose a reason for hiding this comment

PawelHaracz Oct 27, 2021

Choose a reason for hiding this comment

natesuver Oct 27, 2021

Choose a reason for hiding this comment

PawelHaracz Oct 27, 2021

Choose a reason for hiding this comment

altwohill commented Dec 3, 2021

natesuver commented Dec 4, 2021

r-t-m commented Jan 20, 2022

jepperip commented Mar 25, 2022

jiggybyte commented May 19, 2022

milliamp commented Sep 6, 2022 • edited Loading

maximebakercssc commented Sep 7, 2022 • edited Loading

leonvandebroek commented Nov 1, 2022

magahl commented Dec 1, 2022 • edited Loading

PaulGauthier commented Dec 28, 2022

morgannyman commented Jan 11, 2023 • edited Loading

SDohle commented Jan 16, 2023

maximebakercssc commented Feb 2, 2023

joeywas commented Mar 3, 2023

mmrazik commented Mar 3, 2023

azure-pipelines bot commented Mar 3, 2023

jabteles commented May 10, 2023

foomaniac commented May 19, 2023

anujmca commented Jun 15, 2023

patrickfegan commented Jun 30, 2023

altwohill commented Jun 30, 2023 via email

jabteles commented Jun 30, 2023 via email

patrickfegan commented Jul 3, 2023

joeywas commented Jul 3, 2023 • edited Loading

anoordover commented Sep 1, 2023

sticl commented Sep 22, 2023

ImperatorRuscal left a comment

Choose a reason for hiding this comment

TyranVdMerwe commented May 23, 2024 • edited Loading

ghost commented Oct 27, 2021 •

edited by ghost

Loading

milliamp commented Sep 6, 2022 •

edited

Loading

maximebakercssc commented Sep 7, 2022 •

edited

Loading

magahl commented Dec 1, 2022 •

edited

Loading

morgannyman commented Jan 11, 2023 •

edited

Loading

joeywas commented Jul 3, 2023 •

edited

Loading

TyranVdMerwe commented May 23, 2024 •

edited

Loading