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

Generate sha256 ssh pubkey fingerprints for hosts #385

Closed
wants to merge 1 commit into from

Conversation

stlaz
Copy link
Contributor

@stlaz stlaz commented Jan 10, 2017

Replace md5 with sha256 for host ssh pubkey fingerprints. MD5 is disabled in FIPS mode, newer versions of OpenSSH print SHA256 public key fingeprint anyway.

https://fedorahosted.org/freeipa/ticket/5695

@tkrizek tkrizek self-assigned this Jan 10, 2017
@tkrizek tkrizek added the ack Pull Request approved, can be merged label Jan 11, 2017
@tiran
Copy link
Member

tiran commented Jan 11, 2017

What's the migration plan for existing SSHFP records? Are there any supported versions of OpenSSH or other SSH client that do not support SSHFP with SHA256? Would it make sense to run a hybrid mode for a while (SHA256 and MD5 records unless FIPS is enabled)?

@stlaz
Copy link
Contributor Author

stlaz commented Jan 11, 2017

@tiran Which SSHFP records do you mean?

@tiran
Copy link
Member

tiran commented Jan 11, 2017

Your change influenced the value of entry_attrs['sshpubkeyfp'] in convert_sshpubkey_post. Is the value only used in UI or does it affect data in LDAP like DNS SSHFP records? I tracked down some code paths and it looks like the pubkey fingerprint isn't stored in LDAP. The DNS plugin uses different method to calculate SSHFP records. Am I right to assume that this change only affects UI?

@stlaz
Copy link
Contributor Author

stlaz commented Jan 11, 2017

@tiran Yes, exactly, this is only a UI thing.

@tiran
Copy link
Member

tiran commented Jan 11, 2017

@stlaz I'm sorry, go ahead and ignore what I said! :)

@MartinBasti MartinBasti added the pushed Pull Request has already been pushed label Jan 12, 2017
@MartinBasti
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
4 participants