Go: x/crypto/ssh: PublicKey Fingerprint

[dragon3]

It would be useful if PublicKey interface has Fingerprint method which returns the value described bellow:
http://tools.ietf.org/html/rfc4716#section-4

Now I'm trying to make my ssh server with go.
When I made it with python twisted.conch, I used twisted.conch.ssh.keys.Key#fingerprint method to find user's public key, It was useful.

[ianlancetaylor]

CC @hanwen @agl

[hanwen]

As the RFC describes, it's just the md5 of PublicKey.Marshal().

h := md5.New()
h.Write(key.Marshal())
fp := fmt.Sprintf("%x", h.Sum(nil))

there is no need to add this to the SSH package.

[dragon3]

@hanwen Thank you for your comment.

I know that I can get fingerprint with as you said and "separate by colons".
But I think it is convenient if PublicKey has that, because I found many developers write own "fingerprint" function/method ( I searched on github ).

[adg]

It seems like a reasonable feature to implement a standards-compliant fingerprint format method.

[dragon3]

@adg @agl Thank you for your response!

[hanwen]

are there other features that you miss? I am not per se against utility methods like these in a centralized place, but the package is quite big as it is. Next thing we know, we'd be adding a parser for .ssh/known_hosts and .ssh/config.

If there are more utility methods missing, we might have enough for a ssh/sshutil package. (in retrospect, the handling of Authorized keys should have probably been moved out of the main package too.)

[dragon3]

@hanwen

are there other features that you miss?

No, that's it so far.

[dragon3]

It's been almost 1 year since I created this issue, so I'm closing this issue.
Please let me know and re-open this issue if you are still interested in this issue.

Thank you!

[stouset]

As the RFC describes, it's just the md5 of PublicKey.Marshal().

You're literally suggesting here that you'd rather users of this library consult an RFC than for you to provide and implement a simple three-line function.

I really, really, really don't understand this mentality. Yes, it's a simple implementation. But a) having to look this procedure up costs your users time, b) having to reimplement, maintain, and test this function costs your users more time, and c) if 1,000 developers implement this on their own, at least one of these implementations will have a bug.

Why not just implement this once, and save your users a pointless duplication of time and effort?

[bradfitz]

@dragon3, no harm keeping this open. Still seems like a valid request.

[turnage]

@hanwen Why do you fear the growth of this package? I'm confused because of your suggestion to make an sshutil package; surely that would only postpone the problem of package growth.

[hanwen]

@turnage - mostly because of feature creep. Perhaps this one won't add much complexity, but it's also not a big win; you're basically saving out 3 lines of code. I take the point about people having to read the RFC, though.

That said, if we do this, we'll almost certainly will want the newer sha256 fingerprint, since we shouldn't be using md5 any more.

[bradfitz]

I'm not worried about feature creep. It's not like this is the stdlib's net package. This is in a package specifically dedicated to SSH.

[hanwen]

Brad, any opinion on md5 fp's? From a crypto perspective we want to discourage them, but if we add just the sha256 one, I bet we'll soon have a bugreport will be about adding the md5 flavor too.

[bradfitz]

Give them hideously specific names.

FingerprintLegacyMD5
FingerprintSHA256

And use the depreciation syntax on the md5 one?

[hanwen]

didn't know we have depreciation syntax? Isn't that contrary to the compat promise?

[bradfitz]

Just search the stdlib for "Deprecated:" for examples.

x/net/ssh isn't under the compat promise. But also, deprecated doesn't mean we're going to remove it necessarily. It just means it's not recommended and there's a better way and we might hide it a bit in the docs.

[hanwen]

Fair enough. Let's start with SHA256 first.

Anybody want to send a CL ? It should be a function taking a PublicKey as argument.

[stouset]

func FingerprintMD5(key ssh.PublicKey) []byte {
    var (
        hash        = md5.New()
        fingerprint = make([]byte, 0, hash.BlockSize()/4*3-1)
    )

    hash.Write(key.Marshal())

    for i, byte := range hash.Sum(nil) {
        fingerprint = append(fingerprint, fmt.Sprintf("%x", byte)...)
        if i != len(fingerprint)-1 {
            fingerprint = append(fingerprint, ':')
        }
    }

    return fingerprint
}

Maybe this is wrong, or buggy, or slower than necessary, or non-idiomatic — I don't claim to be particularly proficient in go.

[hanwen]

please use the documented approach for sending CLs for review, thanks.

[stouset]

I'm not a gopher. I have no idea what a CL is, and searching doesn't really turn up anything useful other than the contribution guidelines. If I can simply open a pull request, I'm happy to.

On the other hand, if I have to read a five-page document, sign up for a third-party code review tool, install strange and mysterious new git commands, and send a diff to a mailing list, only to invariably need to argue endlessly to get a ~15-line function added to a x/ package, hopefully you'll understand why I don't.

The code is there. I hereby release it into the public domain, so you (and anyone reading this ticket) may reuse it and relicense it as you wish.

[majic0]

Just going to copypaste what I'm using, feel free to do whatever you want with it

import (
    "crypto/md5"
    "crypto/sha256"
    "encoding/base64"
    "fmt"
    "strings"

    "golang.org/x/crypto/ssh"
)

// hexadecimal md5 hash grouped by 2 characters separated by colons
func FingerprintMD5(key ssh.PublicKey) string {
    hash := md5.Sum(key.Marshal())
    out := ""
    for i := 0; i < 16; i++ {
        if i > 0 {
            out += ":"
        }
        out += fmt.Sprintf("%02x", hash[i]) // don't forget the leading zeroes
    }
    return out
}

// base64 sha256 hash with the trailing equal sign removed
func FingerprintSHA256(key ssh.PublicKey) string {
    hash := sha256.Sum256(key.Marshal())
    b64hash := base64.StdEncoding.EncodeToString(hash[:])
    return strings.TrimRight(b64hash, "=")
}

[dragon3]

I've just sent a CL now.
https://go-review.googlesource.com/#/c/32814/

Since it is my first time to send a CL, please let me know if I have anything.

Thanks!

[gopherbot]

CL https://golang.org/cl/32814 mentions this issue.

[dragon3]

@hanwen Thank you for your reviewing and merging!
And thank you all for your comments and advices.