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

Provides clarity on az user update for SSH keys #6044

Merged
merged 1 commit into from Mar 21, 2018
Merged

Provides clarity on az user update for SSH keys #6044

merged 1 commit into from Mar 21, 2018

Conversation

cicorias
Copy link
Member

The az user update .. --ssh-key-value {} does not update, replace, or remove any existing public keys in the admin user's ~/.ssh/authorized_keys file. This actually appends the supplied key text.

This is important as compromised key means you flatten the box or hand edit authorized keys on the VM.

This has been tested on numerous machines and can be reviewed in the source code for VM Extension here: https://github.com/Azure/azure-linux-extensions/blob/master/VMAccess/vmaccess.py#L232

 # Reset ssh key with the new public key passed in or reuse old public key.
    if cert_txt or len(ovf_env.SshPublicKeys) > 0:
        if cert_txt and cert_txt.strip().lower().startswith("ssh-rsa"):
            no_convert = True
        try:
            pub_path = os.path.join('/home/', user_name, '.ssh',
                'authorized_keys')
            ovf_env.UserName = user_name
            if no_convert:
                if cert_txt:
                    pub_path = ovf_env.PrepareDir(pub_path)
                    final_cert_txt = cert_txt
                    if(not cert_txt.endswith("\n")):
                        final_cert_txt = final_cert_txt+"\n"
                    waagent.AppendFileContents(pub_path, final_cert_txt)

@PRMerger9
Copy link
Contributor

@cicorias : Thanks for your contribution to the Azure documentation! The author, @dlepow, has been notified to review your proposed change.

@PRMerger16
Copy link
Contributor

@cicorias : Thanks for your contribution to the Azure documentation! The author, @dlepow, has been notified to review your proposed change.

@dlepow
Copy link
Contributor

dlepow commented Mar 20, 2018

@cicorias - Shawn, this is a helpful clarification. Do you think it would also help to change the preceding heading from "Reset SSH key" to "Update SSH key"? If you agree, please update the PR and I'll sign off. Thanks for taking the time to contribute.

@mimckitt
Copy link
Contributor

#6045

@cicorias
Copy link
Member Author

Update SSH would be a better section name. But I’ve had too many questions about whether it truncated or replaced the keys, although it just appends. I really feel the note clarifying and acknowledging that this isn’t something to use for key rotation is important.

Are you saying I should just change the section name and remove the note or does the note stay?

@dlepow
Copy link
Contributor

dlepow commented Mar 21, 2018

@cicorias - I think the note should stay; just suggesting the different section heading to complement that change. Apologies if my comment wasn't clear. Thx.

@PRMerger6
Copy link
Contributor

@cicorias : Thanks for your contribution! The author, @dlepow, has been notified to review your proposed change.

The `az user update .. --ssh-key-value {}` does not update, replace, or remove any existing public keys in the admin user's `~/.ssh/authorized_keys` file. This actually appends the supplied key text.

This is important as compromised key means you flatten the box or hand edit authorized keys on the VM.

This has been tested on numerous machines and can be reviewed in the source code for VM Extension here: https://github.com/Azure/azure-linux-extensions/blob/master/VMAccess/vmaccess.py#L232

```
 # Reset ssh key with the new public key passed in or reuse old public key.
    if cert_txt or len(ovf_env.SshPublicKeys) > 0:
        if cert_txt and cert_txt.strip().lower().startswith("ssh-rsa"):
            no_convert = True
        try:
            pub_path = os.path.join('/home/', user_name, '.ssh',
                'authorized_keys')
            ovf_env.UserName = user_name
            if no_convert:
                if cert_txt:
                    pub_path = ovf_env.PrepareDir(pub_path)
                    final_cert_txt = cert_txt
                    if(not cert_txt.endswith("\n")):
                        final_cert_txt = final_cert_txt+"\n"
                    waagent.AppendFileContents(pub_path, final_cert_txt)
```
@PRMerger20
Copy link
Contributor

@cicorias : Thanks for your contribution! The author, @dlepow, has been notified to review your proposed change.

@cicorias
Copy link
Member Author

Hi @dlepow could you take a look now. I squashed commits too. But also updated some of the language to use update instead of 'reset' in several places.

@dlepow
Copy link
Contributor

dlepow commented Mar 21, 2018

@cicorias - Shawn, LGTM! Thanks for making these helpful updates.
#sign-off

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet