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

Why is Get-AzAccessToken going to start returning a SecureString? #26847

Open
jackmtpt opened this issue Dec 6, 2024 · 4 comments
Open

Why is Get-AzAccessToken going to start returning a SecureString? #26847

jackmtpt opened this issue Dec 6, 2024 · 4 comments
Assignees
Labels
customer-reported needs-team-attention This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@jackmtpt
Copy link

jackmtpt commented Dec 6, 2024

Description

Other Microsoft code is actively migrating away from SecureString , e.g. AzureAD/microsoft-authentication-library-for-dotnet#2437.

Microsoft's own documentation about SecureString recommends that it is not used: https://learn.microsoft.com/en-us/dotnet/fundamentals/runtime-libraries/system-security-securestring

Apparently Az version 14 is going to ignore this guidance and make a breaking change to use SecureString anyway:

WARNING: Upcoming breaking changes in the cmdlet 'Get-AzAccessToken' :
The Token property of the output type will be changed from String to SecureString. Add the [-AsSecureString] switch to avoid the impact of this upcoming breaking change.
- The change is expected to take effect in Az version : '14.0.0'
- The change is expected to take effect in Az.Accounts version : '4.0.0'

Why?

Script or Debug output

Environment data

Module versions

Error output

@jackmtpt jackmtpt added needs-triage This is a new issue that needs to be triaged to the appropriate team. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Dec 6, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added customer-reported and removed needs-triage This is a new issue that needs to be triaged to the appropriate team. labels Dec 6, 2024
@isra-fel isra-fel self-assigned this Dec 12, 2024
@isra-fel
Copy link
Member

Hi @jackmtpt thanks for reaching out. Long story short, we promote secure string to prevent the risk of unintentional exposure of plain text string, not for cryptographic reasons.

PowerShell scripts are often executed in automation systems. Some of them lack comprehensive access control over logs, which when combined with unsafe programming practices, leads to the exposure of secrets. That's the problem we want to address with SecureString. Even if a SecureString is logged, it's never in plain text, hence free of risk.

The reason why SecureString is unrecommended in .NET is that it's not safe cryptographically. Since Azure PowerShell is not relying on it to encrypt those data, it is unnecessary to move away from SecureString as well.

@isra-fel isra-fel added the needs-author-feedback More information is needed from author to address the issue. label Dec 13, 2024
@jackmtpt
Copy link
Author

jackmtpt commented Dec 13, 2024

There's better ways to do that though aren't there? e.g. via the Format.ps1xml files that specify which fields are shown by default when an object is written to the pipeline. That would prevent the token getting printed to the terminal/logs accidentally unless someone explicitly accesses .Token.

@microsoft-github-policy-service microsoft-github-policy-service bot added needs-team-attention This issue needs attention from Azure service team or SDK team and removed needs-author-feedback More information is needed from author to address the issue. labels Dec 13, 2024
@isra-fel
Copy link
Member

isra-fel commented Dec 17, 2024

Yes that's one option. But there are limitations that (a) as you said if someone explicitly .Token or does customized format with | Format-* it won't work. (b) format.ps1xml doesn't apply to the result of jobs e.g. $j = start-job { get-azaccesstoken }; $j | receive-job.

@jackmtpt
Copy link
Author

For A I'd say that's up to the user - if they send .Token to the pipeline they shouldn't be surprised when it ends up in logs. For B does it work if you include Deserialized.Microsoft.Azure.Commands.Profile.Models.PSAccessToken in the format xml?

For context my reason for not wanting to deal with SecureStrings here is that for some API calls you need to include the Entra ID refresh token in the body of a POST request, such as when getting an ACR refresh token via the /oauth2/exchange endpoint. You'd have to go via PSCredential.GetNetworkCredential().Password to retrieve the token which is just pointless extra code vs being able to get the token directly.

If you still want to change the default to SecureString, would you be open to having a -AsPlainText argument to keep the current behaviour?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer-reported needs-team-attention This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
Development

No branches or pull requests

2 participants