Azure / azure-cli

Azure Command-Line Interface
MIT License
3.97k stars 2.95k forks source link

Leak of az secrets in file system system calls #28839

Open freedge opened 5 months ago

freedge commented 5 months ago

this is a spin off of https://github.com/Azure/login/issues/27

when running

az login --service-principal -u $U --tenant $T -p $P

the secret is leaked on the command line as known, it is also leaked through a newfstatat system call.

This is undocumented but it's possible to use instead a "@" so that the password is read from a file, eg in this fashion

az login --service-principal -u $U --tenant $T -p @<(echo "$P")

which prevents the password leak on the command line (in some cases somewhat addressing https://github.com/Azure/azure-cli/issues/10241 and https://github.com/Azure/azure-cli/pull/27938), however it is still leaked in the newfstatat system call:

15547 newfstatat(AT_FDCWD, "W3x..., 0x7fffe0296410, 0) = -1 ENOENT (No such file or directory)

performed from https://github.com/Azure/azure-cli/blob/246725e59015f4b267f5a5ea4fb8d832e153f58d/src/azure-cli-core/azure/cli/core/auth/identity.py#L285-L286

this means the secret is leaked to the file system (if a network file system like NFS it is sent over the network) and relevant auditing tools (auditd, selinux, fapolicyd, etc.)

In addition w.r.t to the previous opened ticket, any documentation that mentions az login --service-principal with a password provided on the command line is wrong and should be updated.

MoChilia commented 4 months ago

Hi @freedge, we recommend using OIDC or Managed Identity for login instead of using secrets to prevent leaks.

For measures you mentioned, unfortunately, azure-cli does not currently support using environment variables for authentication.

@jiasli, could you please confirm if the following method helps prevent password leaks on the command line?

az login --service-principal -u $U --tenant $T -p @<(echo "$P")
freedge commented 4 months ago

we recommend using OIDC or Managed Identity for login instead of using secrets

indeed this is what is said on https://learn.microsoft.com/en-us/cli/azure/azure-cli-sp-tutorial-2

it's however not mentioned https://learn.microsoft.com/en-us/cli/azure/authenticate-azure-cli-service-principal

to prevent leaks.

I don't think this reason is explained or mentioned in any documentation. The above link provides even an "important warning" supposedly improving security but still without any mention of it still leaking the password.

Could you put this recommendation (and its reasons) clearly in the documentation, to strongly discourage users to use service principal passwords? Could az login emit a warning everytime a user uses that feature since it's known to be insecure?

MoChilia commented 4 months ago

@freedge, as mentioned in https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#inputs

GitHub stores input parameters as environment variables.

Is it possible to retrieve the password in azure/login through a newfstatat system call?

The secret login method hasn't been proven to be insecure; it's just recommended to use the no-secret-login method, due to the risk of leaks when users manage their secrets.

freedge commented 4 months ago

for example if you cd into a directory mounted through nfs, and you run azure login -p, then the password will be sent in clear over the network, to the nfs server. So yes it is possible to retrieve the password.

The az login -p with a password is insecure since the password is visible to any user logged in on the system.

MoChilia commented 4 months ago

The GitHub-hosted runner is an ephemeral runner that exists only for the duration of a single job. Attacks of this appear to be invalid. This concern primarily pertains to self-hosted runners lacking proper management.

And it seems an issue for az login -p required to be solved in azure-cli. To address this issue, transferring it to azure-cli repo for now. If necessary, modifications to azure/login will be made accordingly.

yonzhan commented 4 months ago

Thank you for opening this issue, we will look into it.

jiasli commented 4 months ago

This is undocumented but it's possible to use instead a "@" so that the password is read from a file, eg in this fashion

We do have document for the @<file> convention: https://learn.microsoft.com/en-us/cli/azure/use-azure-cli-successfully#use-quotation-marks-in-parameters. Its implementation is at

https://github.com/Azure/azure-cli/blob/c4407842f98b5961fe25affefce13a0989238848/src/azure-cli-core/azure/cli/core/commands/__init__.py#L85-L101

Not https://github.com/Azure/azure-cli/blob/246725e59015f4b267f5a5ea4fb8d832e153f58d/src/azure-cli-core/azure/cli/core/auth/identity.py#L285-L286

The logic here is for specifying a certificate file path, without @ prefix, such as

az login --service-principal -u $U --tenant $T -p /home/user1/mycert.pem

however it is still leaked in the newfstatat system call:

I am wondering why <(echo "$P") is logged in newfstatat - is it writing to or reading from the df that results in the newfstatat system call? I think it is more about how Linux works internally, instead of Azure CLI's implementation. Can we repro this without Azrue CLI, such as

$ echo <(echo "aaa")
/dev/fd/63
$ cat <(echo "aaa")
aaa
$ python3 -c "import sys; print(sys.argv)" <(echo "aaa")
['-c', '/dev/fd/63']
$ python3 -c "import sys; print(open(sys.argv[1]).read(), end='')" <(echo "aaa")
aaa

@freedge, could you please share more details on how you captured this line?

15547 newfstatat(AT_FDCWD, "W3x..., 0x7fffe0296410, 0) = -1 ENOENT (No such file or directory)
freedge commented 4 months ago

We do have document for the @<file> convention:

ah indeed. it's not documented anywhere as a way to prevent leaking secrets though

https://github.com/Azure/azure-cli/blob/c4407842f98b5961fe25affefce13a0989238848/src/azure-cli-core/azure/cli/core/commands/__init__.py#L85-L101

Not

https://github.com/Azure/azure-cli/blob/246725e59015f4b267f5a5ea4fb8d832e153f58d/src/azure-cli-core/azure/cli/core/auth/identity.py#L285-L286

right. but the identity.py is still called with the actual password, and the stat call made with it.

I am wondering why <(echo "$P") is logged in newfstatat

it's not

how you captured this line?

strace az login etc.

daviewales commented 4 months ago

So far as I can tell, the <(echo "$P") construct doesn't leak information except to the current user and root. So it's a similar level of security to using environment variables. Is that correct?

daviewales commented 4 months ago

Note also that the security improvement can also be implemented without bash-specific process substitution using /dev/stdin:

printf '%s' "$P" | az login --service-principal -u $U --tenant $T -p @/dev/stdin
freedge commented 4 months ago

Right. Note that even specifying "@/dev/stdin" will still perform a system call to check whether there is a file named $P under the current directory, so the leak reported in this ticket persists.

daviewales commented 3 months ago

Does the system call check leak $P to all users? Or is it only visible to current user and root?