On Mon, Nov 3, 2014 at 9:50 AM, Woodhouse, David <david.woodhouse at intel.com> wrote: > I've just pushed out some changes to the way we handle server > certificates. > > Firstly, the OPENCONNECT_X509 opaque type is dead, along with the > openconnect_get_peer_cert() function. > > The openconnect_get_cert_DER() and openconnect_get_cert_details() > functions were *only* ever called on the peer cert (there was no way of > getting any *other* object of the OPENCONNECT_X509 type), and they have > been changes to openconnect_get_peer_cert_DER() and > openconnect_get_peer_cert_details() respectively. > > The openconnect_get_cert_sha1() function has been changed to > openconnect_get_peer_cert_hash() and it no longer returns a SHA1 of the > whole certificate DER. Instead, it returns a hash (*currently* SHA1) of > the server's public key. the horrid "pass me a pointer to a 41-character > buffer" part of the libopenconnect API is also now gone, as this new > function returns a const char *. > > Hashing just the public key means that the certificate can be reissued > and as long as the key remains the same, the user doesn't have to > manually accept the key again. It also defends against social > engineering attacks where a MITM repeatedly tampers with non-critical > parts of the certificate, effectively training the user to just click > 'accept' each time... until one time, the attacker *has* hijacked the > connection. > > There is a new openconnect_check_peer_cert_hash() function which, given > a hash, will check it against the server's certificate. It will accept > either the old-style 40-digit SHA1 of the whole cert, *or* the new-style > SHA1 of the pubkey, which is prefixed by 'sha1:'. > > In future, the new style may use something better than SHA1 and the > openconnect_check_peer_cert_hash() function will obviously be adjusted > to cope. > > A client is expected to use openconnect_check_peer_cert_hash() to check > if a 'remembered' certificate is indeed a match for the one currently > offered by the server ? don't use strcmp() against the new hash because > you'll get false negatives. Even on a match, the client is expected to > update its storage to contain *new* hash returned by > openconnect_get_peer_cert_hash(). > > The --servercert and --authenticate command line options now behave this > way too. The former will accept either type of hash, and the latter > generates the new style only for its FINGERPRINT= output. So, I finally got around to integrating the new library on the Android side, and I was wondering how you envisioned the UI working. With the old API, I was able to request a SHA1 of the whole certificate DER and present that to the user. They could compare the SHA1 to the output from e.g. "openssl x509 -fingerprint -sha1 -in foo.pem" or "View Certificate" in Firefox. The known-good SHA1 could also be provided by a sysadmin, posted on a sticky note, or read off over the phone. This fingerprinting convention is reasonably standard across different clients and OSes. On Android I intentionally did not add a "View Certificate" button as it is trivial to make a spoofed cert that has authentic data in all of the human-readable fields. Allowing the user to assess risk based on that content creates a false sense of security. So they are forced to use the hash to make a decision. But now we're getting a different hash from openconnect_get_peer_cert_hash(), and I'm not sure what the user should be comparing it against, other than asking "is it the same as last time?" Unless the new fingerprinting scheme is widely used, it doesn't help authenticate the server when they're connecting for the first time. So maybe we need a new library function to return the old (standardized) hash, and once that is accepted through the UI, cache the pubkey hash and use that for future comparisons?