-
Notifications
You must be signed in to change notification settings - Fork 61
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
fix broken zone cloning due to OpenSSH DSA deprecation #503
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding ecdsa instead seems like a sensible move
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for finding this and submitting the change. It's been a while since I've looked at this file and so I have a couple of suggestions to future proof things in here.
If you prefer, we can take your change as it is, as it certainly improves the situation, and then follow up with the rest.
src/brand/ipkg/system-unconfigure
Outdated
/usr/bin/ssh-keygen -q -t rsa -b 2048 -N '' -C root@unknown \ | ||
-f $ALTROOT/etc/ssh/ssh_host_rsa_key \ | ||
|| bomb "Failed to create new $ALTROOT/etc/ssh/ssh_host_rsa_key" | ||
/usr/bin/ssh-keygen -q -t dsa -N '' -C root@unknown \ | ||
-f $ALTROOT/etc/ssh/ssh_host_dsa_key \ | ||
/usr/bin/ssh-keygen -q -t ecdsa -b 521 -N '' -C root@unknown \ | ||
-f $ALTROOT/etc/ssh/ssh_host_ecdsa_key \ | ||
|| bomb "Failed to create new $ALTROOT/etc/ssh/ssh_host_dsa_key" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point I think we should replace all of these with
/usr/bin/ssh-keygen -A -f $ALTROOT
That way we'll end up with the same set of keys as a freshly installed system, and generated with the same options.
We should also do something more generic with the old key renaming as it shouldn't need to know what the valid set of key types is, and that's even harder across upgrades.
This is already missing ed25519
for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's a good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated as above. Here's a zone cloned with the new script:
root@serv-cron:~# ls -l /etc/*ssh*
total 161
-rw-r--r-- 1 root bin 541716 Dec 1 22:36 moduli
-rw-r--r-- 1 root bin 1526 Dec 1 22:36 ssh_config
-rw------- 1 root root 505 Dec 1 23:11 ssh_host_ecdsa_key
-rw-r--r-- 1 root root 176 Dec 1 23:11 ssh_host_ecdsa_key.pub
-rw------- 1 root root 411 Dec 1 23:11 ssh_host_ed25519_key
-rw-r--r-- 1 root root 96 Dec 1 23:11 ssh_host_ed25519_key.pub
-rw------- 1 root root 2602 Dec 1 23:11 ssh_host_rsa_key
-rw-r--r-- 1 root root 568 Dec 1 23:11 ssh_host_rsa_key.pub
-rw-r--r-- 1 root bin 3259 Dec 1 23:11 sshd_config
Thanks! |
Trying to clone a lipkg zone (and, I assume, any other brand) currently fails with
This is because the
system-unconfigure
script attempts to usessh-keygen
to generate a DSA key. DSA keys have been removed from OpenSSH, so the script errors.This change creates an ECDSA key instead. Perhaps DSA was only ever in there for backward compatibility, so I leave it to the more knowledgeable reviewer to decide if a more correct fix would be to simply remove the DSA lines altogether.
I ran the test suite and had a few permissions-based failures (I don't have a proper OmniOS build env at the moment) but nothing which looked related to this change.