1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-06-30 19:12:48 -05:00

Invent a struct type for polymorphic SSH key data.

During last week's work, I made a mistake in which I got the arguments
backwards in one of the key-blob-generating functions - mistakenly
swapped the 'void *' key instance with the 'BinarySink *' output
destination - and I didn't spot the mistake until run time, because in
C you can implicitly convert both to and from void * and so there was
no compile-time failure of type checking.

Now that I've introduced the FROMFIELD macro that downcasts a pointer
to one field of a structure to retrieve a pointer to the whole
structure, I think I might start using that more widely to indicate
this kind of polymorphic subtyping. So now all the public-key
functions in the struct ssh_signkey vtable handle their data instance
in the form of a pointer to a subfield of a new zero-sized structure
type 'ssh_key', which outside the key implementations indicates 'this
is some kind of key instance but it could be of any type'; they
downcast that pointer internally using FROMFIELD in place of the
previous ordinary C cast, and return one by returning &foo->sshk for
whatever foo they've just made up.

The sshk member is not at the beginning of the structure, which means
all those FROMFIELDs and &key->sshk are actually adding and
subtracting an offset. Of course I could have put the member at the
start anyway, but I had the idea that it's actually a feature _not_ to
have the two types start at the same address, because it means you
should notice earlier rather than later if you absentmindedly cast
from one to the other directly rather than by the approved method (in
particular, if you accidentally assign one through a void * and back
without even _noticing_ you perpetrated a cast). In particular, this
enforces that you can't sfree() the thing even once without realising
you should instead of called the right freekey function. (I found
several bugs by this method during initial testing, so I think it's
already proved its worth!)

While I'm here, I've also renamed the vtable structure ssh_signkey to
ssh_keyalg, because it was a confusing name anyway - it describes the
_algorithm_ for handling all keys of that type, not a specific key. So
ssh_keyalg is the collection of code, and ssh_key is one instance of
the data it handles.
This commit is contained in:
Simon Tatham
2018-05-27 08:32:21 +01:00
parent 9375f594c2
commit 0fc2d3b455
9 changed files with 225 additions and 220 deletions

View File

@ -649,7 +649,7 @@ struct ssh2_userkey *openssh_pem_read(const Filename *filename,
/* And now for something completely different */
unsigned char *priv;
int privlen;
const struct ssh_signkey *alg;
const ssh_keyalg *alg;
const struct ec_curve *curve;
/* Read INTEGER 1 */
ret = ber_read_id_len(p, key->keyblob+key->keyblob_len-p,
@ -980,6 +980,7 @@ int openssh_pem_write(const Filename *filename, struct ssh2_userkey *key,
key->alg == &ssh_ecdsa_nistp384 ||
key->alg == &ssh_ecdsa_nistp521) {
const unsigned char *oid;
struct ec_key *ec = FROMFIELD(key->data, struct ec_key, sshk);
int oidlen;
int pointlen;
strbuf *seq, *sub;
@ -995,8 +996,7 @@ int openssh_pem_write(const Filename *filename, struct ssh2_userkey *key,
* BIT STRING (0x00 public key point)
*/
oid = ec_alg_oid(key->alg, &oidlen);
pointlen = (((struct ec_key *)key->data)->publicKey.curve->fieldBits
+ 7) / 8 * 2;
pointlen = (ec->publicKey.curve->fieldBits + 7) / 8 * 2;
seq = strbuf_new();
@ -1430,7 +1430,7 @@ struct ssh2_userkey *openssh_new_read(const Filename *filename,
unsigned checkint0, checkint1;
const void *priv, *string;
int privlen, stringlen, key_index;
const struct ssh_signkey *alg = NULL;
const ssh_keyalg *alg = NULL;
if (!key)
return NULL;
@ -2114,7 +2114,7 @@ struct ssh2_userkey *sshcom_read(const Filename *filename, char *passphrase,
char *ciphertext;
int cipherlen;
struct ssh2_userkey *ret = NULL, *retkey;
const struct ssh_signkey *alg;
const ssh_keyalg *alg;
strbuf *blob = NULL;
if (!key)