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

Update Agent To Differentiate Key Types #377

Closed
wants to merge 1 commit into from

Conversation

NoMoreFood
Copy link

  • Adjusted registry entires used by the SSH agent to include a SSH key type in the registry key name.

@NoMoreFood NoMoreFood force-pushed the agent_issue branch 2 times, most recently from 0a1f377 to 37845e7 Compare February 26, 2019 12:29
- Adjusted registry entires used by the SSH agent to include a SSH key type in the registry key name.
@manojampalam
Copy link

Adding a reference to related conversation - NoMoreFood@d7b6489

@@ -154,7 +174,7 @@ process_add_identity(struct sshbuf* request, struct sshbuf* response, struct age
if ((!ConvertStringSecurityDescriptorToSecurityDescriptorW(REG_KEY_SDDL, SDDL_REVISION_1, &sa.lpSecurityDescriptor, &sa.nLength)) ||
sshkey_to_blob(key, &pubkey_blob, &pubkey_blob_len) != 0 ||
convert_blob(con, blob, blob_len, &eblob, &eblob_len, 1) != 0 ||
((thumbprint = sshkey_fingerprint(key, SSH_FP_HASH_DEFAULT, SSH_FP_DEFAULT)) == NULL) ||
((thumbprint = get_key_subkey_name(key)) == NULL) ||

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the advantage of appending ":key_type_string"?
Don't we need the same logic when we read from the registry to remove the ":key_typ_string"?

@@ -179,6 +199,12 @@ process_add_identity(struct sshbuf* request, struct sshbuf* response, struct age
if ((success == 0) && reg && thumbprint)
RegDeleteKeyExA(reg, thumbprint, KEY_WOW64_64KEY, 0);

/* delete previous version of thumbprint keys without type */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the logic should be before we create the thumbprint?

line 180 on the right hand side adds the thumbprint to registry.
Line 203 deletes the thumbprint added in line 180?

@bagajjal
Copy link

bagajjal commented Dec 12, 2019

@NoMoreFood - Could you please add the use case why this change is made?
I tried to look at the reference ( NoMoreFood/openssh-portable@d7b6489) but it didn't say why the change is initiated.
The proposal from @manojampalam discussed is different than what we see here?

@NoMoreFood
Copy link
Author

@bagajjal I'm going to close this for now. It was never updated in response to @manojampalam's suggestion.

@NoMoreFood NoMoreFood closed this Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants