Azure / SimuLand

Understand adversary tradecraft and improve detection strategies
MIT License
700 stars 80 forks source link

Adding one parameter to deployment parameters file #19

Closed jorlamd closed 3 years ago

jorlamd commented 3 years ago

In attempting to build the environment using the supplied azuredeploy.paramenters.json I noticed the InstallADFS sub-deployment was failing, this was due to the $cert variable in InstallADFS.ps1 being blank, as the domainFQDN eventually passed as a parameter was not present. Adding it here along with a cautionary note to avoid placing the wildcard, as installADFS.ps1 does this alrady.

Cyb3rWard0g commented 3 years ago

Hello @jorlamd ! The parameter exists in the main template:

https://github.com/Azure/SimuLand/blob/main/2_deploy/aadHybridIdentityADFS/azuredeploy.json#L184-L190

It was not in the parameters file which would be good to add it yes. However, I do not think the issue is that it is blank. The issue might be that your CERT domain does not align with the domain that is provided by default 'simulandlabs.com' (You have to change that).

In the deployment steps, I explain what parameters need to be modified depending on the environment:

https://github.com/Azure/SimuLand/tree/main/2_deploy/aadHybridIdentityADFS#deploy-arm-template

I still believe this PR is valid because we did not have the domainFQDN parameter in the parameters file. Can you please add this description to this PR? : https://github.com/Azure/SimuLand/blob/main/2_deploy/aadHybridIdentityADFS/azuredeploy.json#L187

jorlamd commented 3 years ago

Ah, you mean that the default value would've been populated by default and resulted in 'simulandlabs.com' not aligning with the provided cert domain? If so, understood. Makes complete sense. Thanks for clarifying that detail.

re the description. Do you mean putting that text in the "value" field of the parameters file to make it match the description in the main template?

Cyb3rWard0g commented 3 years ago

Correct, the default value would have been populated by default by the domainFQDN parameter in the main template.

Yes, I was wondering if you can modify the file that you have in the PR and basically have the same description as the parameter in the main template. Yes in the value:

https://github.com/Azure/SimuLand/blob/main/2_deploy/aadHybridIdentityADFS/azuredeploy.json#L184-L190

"domainFQDN": {
            "type": "string",
            "metadata": {
                "description": "The FQDN of the Active Directory Domain to be created"
            },
            "defaultValue": "simulandlabs.com"
        }

Please and thank you! then it should be good to merge.

Cyb3rWard0g commented 3 years ago

Thank you @jorlamd !