Support OpenSSH public key format. #29
Conversation
Motivation: It'll be useful to be able to deserialize the OpenSSH public key format, so let's add some hooks for doing that! Modifications: - Add parser for OpenSSH public key format. Result: Can parse OpenSSH public keys.
| //===----------------------------------------------------------------------===// | ||
|
|
||
| import Crypto | ||
| import Foundation |
There was a problem hiding this comment.
what do we need it for?
There was a problem hiding this comment.
It's not a meaningful addition: Crypto depends on Foundation anyway, so we already have the dependency.
| NIOSSHError(type: .invalidHostKeyForKeyExchange, diagnostics: "Expected \(String(expected)), got \(String(actual))") | ||
| } | ||
|
|
||
| @inline(never) |
There was a problem hiding this comment.
What is the purpose of this @inline(never)?
There was a problem hiding this comment.
We believe that the case of throwing an error here will be very rare. For this reason, we can't expect to gain any performance from inlining this method: it'll cost binary size, and potentially prevent the compiler inlining something else that would be more useful to performance. For that reason we just force the compiler not to do that.
There was a problem hiding this comment.
Interesting micro optimisation!
| // | ||
| // We split on spaces, no more than twice. We then check if we know about the algorithm identifier and, if we | ||
| // do, we parse the key. | ||
| var components = ArraySlice(openSSHPublicKey.split(separator: " ", maxSplits: 2, omittingEmptySubsequences: true)) |
There was a problem hiding this comment.
Is there a specification for this format. Or is this a non-written standard? A quick search resulted in references to the protocol format, but not towards the format specified above.
The implementation uses this format as a String. Would it be helpful to split up the public initialiser to accept two parameters? One for the algorithm, and one for the base64 encoded bytes? Or would it be worth considering just decoding the base64, since the only purpose of the algorithm-id is to verify it against the neighbouring base64 contents?
What's the use case for this public implementation? Where does the string originate from?
There was a problem hiding this comment.
I'm not aware of a formal specification: this format is used by OpenSSH, not by the SSH protocol itself, so any specification would be documented there. However, I don't see one.
The implementation uses this format as a String. Would it be helpful to split up the public initialiser to accept two parameters?
We could, but I don't think it's terribly useful. Most users will have the content as a single whole string, as I'll show below.
What's the use case for this public implementation? Where does the string originate from?
This is the format of public keys in the .ssh directory, or /etc/ssh. This is useful, then, in cases where we'd like to load public keys from those locations. We can do that when we're trying to offer keys to remote peers, or when we're trying to evaluate the keys presented by those peers.
There is an associated private key format, but I wanted to keep it out of this diff for now. We can evaluate adding that as well.
Joannis
left a comment
Joannis
left a comment
There was a problem hiding this comment.
I just talked with @Lukasa about adopting this into a bigger authorized_keys parser. Appearantly that includes a lot of restrictions such as executable command limitations that are more complex and therefore out of scope for this PR. He also mentioned usecases such as parsing id_rsa.pub.
4 participants
Motivation:
It'll be useful to be able to deserialize the OpenSSH public key format,
so let's add some hooks for doing that!
Modifications:
Result:
Can parse OpenSSH public keys.