The 'savedpos' field in 'struct Packet', which was already unused on
the output side after I threw away ssh_pkt_addstring_start, is now
unused on the input side too because a BinarySource implementation has
taken over. So it's now completely gone.
I had a one-off 'Strange packet received' error yesterday for an
SSH_MSG_GLOBAL_REQUEST at connection startup. I wasn't able to
reproduce, but examining the packet logs from the successful retry
suggested that the global request in question was an OpenSSH 'here are
my hostkeys' message.
My belief is that in the failing connection that request packet must
have arrived exceptionally promptly, and been added to pq_full before
the preceding SSH_MSG_USERAUTH_SUCCESS had been processed. So the code
that loops along pq_full feeding everything to the dispatch table
would have moved the USERAUTH_SUCCESS on to pq_ssh2_userauth and
scheduled a callback to handle it, and then moved on to the
GLOBAL_REQUEST which would have gone straight to the dispatch table
handler for 'help, we weren't expecting this message'. The userauth
callback would later on have installed a more sensible dispatch table
handler for global requests, but by then, it was too late.
Solution: make a special case during pq_full processing, so that when
we see USERAUTH_SUCCESS we _immediately_ - before continuing to
traverse pq_full - install the initial dispatch table entries for the
ssh-connection protocol. That way, even if connection messages are
already in the queue, we won't get confused by them.
Now I've got FROMFIELD, I can rework it so that structures providing
an implementation of the Socket or Plug trait no longer have to have
the vtable pointer as the very first thing in the structure. In
particular, this means that the ProxySocket structure can now directly
implement _both_ the Socket and Plug traits, which is always
_logically_ how it's worked, but previously it had to be implemented
via two separate structs linked to each other.
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 is a cleanup I started to notice a need for during the BinarySink
work. It removes a lot of faffing about casting things to char * or
unsigned char * so that some API will accept them, even though lots of
such APIs really take a plain 'block of raw binary data' argument and
don't care what C thinks the signedness of that data might be - they
may well reinterpret it back and forth internally.
So I've tried to arrange for all the function call APIs that ought to
have a void * (or const void *) to have one, and those that need to do
pointer arithmetic on the parameter internally can cast it back at the
top of the function. That saves endless ad-hoc casts at the call
sites.
This removes a lot of pointless duplications of those constants.
Of course, _ideally_, I should upgrade to C99 bool throughout the code
base, replacing TRUE and FALSE with true and false and tagging
variables explicitly as bool when that's what they semantically are.
But that's a much bigger piece of work, and shouldn't block this
trivial cleanup!
There was a time, back when the USA was more vigorously against
cryptography, when we toyed with the idea of having a version of PuTTY
that outsourced its cryptographic primitives to the Microsoft optional
encryption API, which would effectively create a tool that acted like
PuTTY proper on a system with that API installed, but automatically
degraded to being PuTTYtel on a system without, and meanwhile (so went
the theory) it could be moved freely across national borders with
crypto restrictions, because it didn't _contain_ any of the actual
crypto.
I don't recall that we ever got it working at all. And certainly the
vestiges of it here and there in the current code are completely
unworkable - they refer to an 'mscrypto.c' that doesn't even exist,
and the ifdefs in the definitions of structures like RSAKey and
MD5Context are not matched by any corresponding ifdefs in the code. So
I ought to have got round to removing it long ago, in order to avoid
misleading anyone.
At the point where the do_ssh2_connection coroutine rewrites lots of
dispatch table entries to point to things other than itself, there's a
possibility that it's too late, because some packets of those types
have already arrived and been put into pq_ssh2_connection. So you'd
get weird errors like 'Strange packet received: type 94', in which the
only thing that's strange is why packet 94 might conceivably be
unexpected, since it's SSH_MSG_CHANNEL_DATA!
(I observed this when 'plink host -nc otherhost:port' became a sharing
downstream, but I have no reason to think that's the only circumstance
where this can possibly occur, or even a reliable one. It just
happened to have the right timing.)
I'm not completely sure this is the _best_ solution, but it seems to
work: at the point when I rewrite the dispatch table, I also return
the whole of the connection packet queue to the main queue, so that
it'll be run through the dispatch table a second time which will
effectively filter out any packets that we've just installed new
handlers for.
This simplifies the client code both in ssh.c and in the client side
of Pageant.
I've cheated a tiny bit by preparing agent requests in a strbuf that
has space reserved at the front for the packet frame, which makes life
easier for the code that sends them off.
Just as I did a few commits ago with the low-level SHA_Bytes type
functions, the ssh_hash and ssh_mac abstract types now no longer have
a direct foo->bytes() update method at all. Instead, each one has a
foo->sink() function that returns a BinarySink with the same lifetime
as the hash context, and then the caller can feed data into that in
the usual way.
This lets me get rid of a couple more duplicate marshalling routines
in ssh.c: hash_string(), hash_uint32(), hash_mpint().
This affects all the functions that generate public and private key
and signature blobs of all kinds, plus ssh_ecdhkex_getpublic. Instead
of returning a bare block of memory and taking an extra 'int *length'
parameter, all these functions now write to a BinarySink, and it's the
caller's job to have prepared an appropriate one where they want the
output to go (usually a strbuf).
The main value of this change is that those blob-generation functions
were chock full of ad-hoc length-counting and data marshalling. You
have only to look at rsa2_{public,private}_blob, for example, to see
the kind of thing I was keen to get rid of!
There's no reason to pass a 'void *' through and then cast it back to
a packet. All those functions are really doing is serialising a pile
of output on to an arbitrary receiver, so it's nicer to use the new
abstract BinarySink type, and then the do_mode function doesn't have
to cast it at all.
This also removes one lingering ugliness from the previous commit, in
the form of the variable 'stringstart' I introduced in ssh2_setup_pty
to work around the removal of the addstring_start system. Now that's
done the sensible way, by constructing the subsidiary terminal-modes
string in a strbuf (taking advantage of the new type-genericity
meaning I can pass that to the underlying functions just as easily as
a struct Packet), and then putting that into the final packet
afterwards.
All previous uses of them are replaced by the new put_* family.
This is a fairly mechanical transformation, which doesn't take full
advantage of the new ways things can be done more usefully. I'll come
back and clean parts of it up in separate patches later; muddling that
together with this main search-and-replace part would make (even more
of) a giant unreadable monolith.
In fact, those functions don't even exist any more. The only way to
get data into a primitive hash state is via the new put_* system. Of
course, that means put_data() is a viable replacement for every
previous call to one of the per-hash update functions - but just
mechanically doing that would have missed the opportunity to simplify
a lot of the call sites.
Lots of functions had really generic names (like 'makekey'), or names
that missed out an important concept (like 'rsakey_pubblob', which
loads a public blob from a _file_ and doesn't generate it from an
in-memory representation at all). Also, the opaque 'int order' that
distinguishes the two formats of public key blob is now a mnemonic
enumeration, and while I'm at it, rsa_ssh1_public_blob takes one of
those as an extra argument.
Added a missing pq_push_front (see commit a41eefff4), without which
the SSH_MSG_USERAUTH_FAILURE was being dropped from the userauth
packet queue before returning to the top of the loop which waits for
it to arrive.
Another piece of fallout from this morning's patch series, which I
didn't notice until I left a session running for more than an hour:
once do_ssh2_transport is told to begin a rekey, it has no way of
knowing _not_ to immediately do another one, and another, and so on.
Added a value RK_NONE to the rekey class enumeration, and set
rekey_class to that immediately after a key exchange completes. Then a
new one won't start until some code actually sets rekey_class to a
nonzero value again.
Returning from the coroutine every time we finish putting together a
packet is wrong, because it means that any further data in the
incoming queue won't get processed until something triggers another
call to the coroutine. In ssh2_rdpkt I got this right - the only
crReturn that _isn't_ due to running out of data is the special one
immediately after a NEWKEYS - but I forgot to fix it the same way in
ssh1_rdpkt.
ssh->current_user_input_fn was not set to NULL when the ssh structure
was initially set up, which meant that with sufficiently eager
typeahead it could accidentally be called while still full of garbage.
And ssh->connshare is freed in more than one place (by ssh_free and
also by do_ssh_close), but only one of those places nulls it out to
stop the other one trying to free it a second time.
This fixes further embarrassing hangs in the wake of the big
restructuring, in which we'd assign a new function pointer into
ssh->current_user_input_fn but not immediately call it to let it
process whatever might already be waiting for it in the user input
queue.
A large part of the purpose of the 20-patch series just past is to
arrange that user input is never dropped on the floor: if you type it
at a moment where the active protocol coroutine has nothing it can
usefully do with it, the default action will now be to leave it on the
user_input queue where it will eventually be picked up by some later
coroutine, or later phase of this one, that does.
And did I _test_ this feature, end to end, just once, before pushing
the giant patch series?
I did not.
It doesn't work, and the reason why it doesn't work is because various
loops that spin round alternating a crReturn with a check of the input
queue do the first crReturn _before_ the first queue check. So if
there's already data in the queue, well, it won't be _dropped_, but it
also won't be passed on immediately to where it needs to be - instead,
it will sit in the queue until you press _another_ key, at which point
a queue check will happen and all your backed-up typeahead data will
come out.
Fixed by restructuring those loops to do a queue check first. This
applies to the final loop in do_ssh2_connection, and all the little
loops during userauth that prompt for usernames, passwords,
passphrases etc via get_userpass_input.
do_ssh2_authconn is no more! In its place are two separate functions
do_ssh2_userauth and do_ssh2_connection, each handling just one of the
three main protocol components of SSH-2, and each with its own
separate packet queue feeding from pq_full via the dispatch table.
This should be entirely NFC: no bugs are fixed as a side effect of
this change. It's mostly done in the hope that further refactoring in
the future might finally be able to split up ssh.c into smaller source
files, for which it's useful to have these two routines separate,
since each one will be talking to a completely different set of
supporting machinery.
This is not as trivial as it sounds, because although
do_ssh2_transport never uses user input, its in/inlen parameter pair
were nonetheless actually doing something, because by setting inlen to
special values -1 or -2 they doubled as a way to initiate a rekey with
a stated textual reason. So _that_ use of in and inlen has to be
replaced. (Good thing too - that was a horribly ugly API!)
I've replaced them with a new pair of fields in the main ssh
structure, a textual "rekey_reason" for the Event Log and an
enumeration "rekey_class" for discrimination within the code. In
particular, that allows me to get rid of the ugly strcmp that was
checking the textual rekey reason to find out whether the rekey was
due to a GSSAPI credentials update - now there's a separate enum value
for that kind of rekey and we can test for that more sensibly.
Even with my love of verbose commit messages, I don't think there's
anything interesting to say about this commit that the previous few
similar ones haven't mentioned already. This is just more of the same
transformations.
The connection protocol didn't have to do anything too complicated
with user input - just remember to check for it and turn it into
SSH1_CMSG_STDIN_DATA - so this is a much less involved change than the
previous user input conversions.
The userauth loop has always had a rather awkward feature whereby some
of its branches have _already_ received an SSH_MSG_USERAUTH_SUCCESS or
SSH_MSG_USERAUTH_FAILURE packet (e.g. because they have to wait for a
packet that might be one of those _or_ a continuation packet of some
kind), whereas other branches go back round to the top of the loop at
the moment that they know one of those will be the next packet to
arrive. So we had a flag 's->gotit' which tells the start of the loop
whether it's already sitting on the success or failure message, or
whether the first thing it should do is to crWait for one.
Now that the packets are coming from a linked list, there's a simpler
way to handle this: the top of the userauth loop _always_ expects to
find a success/failure message as the first thing in the queue, and if
any branch of the auth code finds it's already removed such a message
from the queue, it can simply put it back on the front again before
going back round.
Just like do_ssh1_login, do_ssh2_authconn is now no longer receiving
its packets via function arguments; instead, it's pulling them off a
dedicated PacketQueue populated by particular dispatch table entries,
with the same reference-counting system to stop them being freed when
the pq_full processing function regains control.
This eliminates further ugly bombout()s from the code while waiting
for authentication responses from the user or the SSH agent.
As I mentioned in the previous commit, I've had to keep
ssh2_authconn_input separate from do_ssh2_authconn, and this time the
reason why is thoroughly annoying: it's because do_ssh2_authconn has
changed its prototype to take the Ssh structure as a void * so that it
can be called from the idempotent callback system, whereas
ssh2_authconn_input has to have the type of ssh->current_user_input_fn
which takes the more sensible pointer type 'Ssh'.
Just as I did to do_ssh1_login, I'm removing the 'in' and 'inlen'
parameters from the combined SSH-2 userauth + connection coroutine, in
favour of it reading directly from ssh->user_input, and in particular
also allowing get_userpass_input to do so on its behalf.
Similarly to the previous case, I've completely emptied the wrapper
function ssh2_authconn_input(), and again, there is a reason I can't
quite get away with removing it...
This introduces the first of several filtered PacketQueues that
receive subsets of pq_full destined for a particular coroutine. The
wrapper function ssh1_coro_wrapper_initial, whose purpose I just
removed in the previous commit, now gains the replacement purpose of
accepting a packet as a function argument and putting it on the new
queue for do_ssh1_login to handle when it's ready. That wrapper in
turn is called from the packet-type dispatch table, meaning that the
new pq_ssh1_login will be filtered down to only the packets destined
for this coroutine.
This is the point where I finally start using the reference counting
system that I added to 'struct Packet' about a dozen commits ago. The
general packet handling calls ssh_unref_packet for everything that
it's just pulled off pq_full and handed to a dispatch-table function -
so _this_ dispatch-table function, which needs the packet not to be
freed because it's going to go on to another queue and wait to be
handled there, can arrange that by incrementing its ref count.
This completes the transformation of do_ssh1_login into a function
with a trivial argument list, whose job is to read from a pair of
input queues (one for user keyboard input and one for SSH packets) and
respond by taking action directly rather than returning a value to its
caller to request action.
It also lets me get rid of a big pile of those really annoying
bombout() calls that I used to work around the old coroutine system's
inability to deal with receiving an SSH packet when the control flow
was in the middle of waiting for some other kind of input. That was
always the weakness of the coroutine structure of this code, which I
accepted as the price for the convenience of coroutines the rest of
the time - but now I think I've got the benefits without that cost :-)
The one remaining argument to do_ssh1_login is the main Ssh structure,
although I've had to turn it into a void * to make the function's type
compatible with the idempotent callback mechanism, since that will be
calling do_ssh1_login directly when its input queue needs looking at.
Now it does its post-completion work itself instead of telling the
callee to do the same. So its caller, ssh1_coro_wrapper_initial, is
now a _completely_ trivial wrapper - but I'm not taking the
opportunity to fold the two functions together completely, because the
wrapper is going to acquire a new purpose in the next commit :-)
This is the first refactoring of a major coroutine enabled by adding
the ssh->user_input queue. Now, instead of receiving a fixed block of
parameter data, do_ssh1_login reads directly from the user input
bufchain.
In particular, I can get rid of all the temporary bufchains I
constructed to pass to get_userpass_input (or rather, the ones in this
particular function), because now we can let get_userpass_input read
directly from the main user_input bufchain, and it will read only as
much data as it has an interest in, and leave the rest as type-ahead
for future prompts or the main session.
After the last few commits, neither incoming SSH packets nor incoming
user input goes through those functions any more - each of those
directions of data goes into a queue and from there to a callback
specifically processing that queue. So the centralised top-level
protocol switching functions have nothing left to switch, and can go.
This change introduces a new bufchain ssh->user_input, into which we
put every byte received via back->send() (i.e. keyboard input from the
GUI PuTTY window, data read by Plink from standard input, or outgoing
SCP/SFTP protocol data made up by the file transfer utilities).
Just like ssh->incoming_data, there's also a function pointer
ssh->current_user_input_fn which says who currently has responsibility
for pulling data back off that bufchain and processing it. So that can
be changed over when the connection moves into a different major phase.
At the moment, nothing very interesting is being done with this
bufchain: each phase of the connection has its own little function
that pulls chunks back out of it with bufchain_prefix and passes them
to the unchanged main protocol coroutines. But this is groundwork for
being able to switch over each of those coroutines in turn to read
directly from ssh->user_input, with the aim of eliminating a
collection of annoying bugs in which typed-ahead data is accidentally
discarded at an SSH phase transition.
This flag was used to indicate that ssh1_protocol (or, as of the
previous commit, ssh1_coro_wrapper) should stop passing packets to
do_ssh1_login and start passing them to do_ssh1_connection.
Now, instead of using a flag, we simply have two separate versions of
ssh1_coro_wrapper for the two phases, and indicate the change by
rewriting all the entries in the dispatch table. So now we _just_ have
a function-pointer dereference per packet, rather than one of those
and then a flag check.
After the previous two refactorings, there's no longer any need to
pass packets to ssh1_protocol or ssh2_protocol so that each one can do
its own thing with them, because now the handling is the same in both
cases: first call the general type-independent packet processing code
(if any), and then call the dispatch table entry for the packet type
(which now always exists).
In SSH-2, every possible packet type code has a non-NULL entry in the
dispatch table, even if most of them are just ssh2_msg_unimplemented.
In SSH-1, some dispatch table entries are NULL, which means that the
code processing the dispatch table has to have some SSH-1 specific
fallback logic.
Now I've put the fallback logic in a separate function, and replaced
the NULL table entries with pointers to that function, so that another
pointless difference between the SSH-1 and SSH-2 code is removed.
NFC: I'm just moving a small piece of code out into a separate
function, which does processing on incoming SSH-2 packets that is
completely independent of the packet type. (Specifically, we count up
the total amount of data so far transferred, and use it to trigger a
rekey when we get over the per-session-key data limit.)
The aim is that I'll be able to call this function from a central
location that's not SSH-2 specific, by using a function pointer that
points to this function in SSH-2 mode or is null in SSH-1 mode.
I've completely removed the top-level coroutine ssh_gotdata(), and
replaced it with a system in which ssh_receive (which is a plug
function, i.e. called directly from the network code) simply adds the
incoming data to a new bufchain called ssh->incoming_data, and then
queues an idempotent callback to ensure that whatever function is
currently responsible for the top-level handling of wire data will be
invoked in the near future.
So the decisions that ssh_gotdata was previously making are now made
by changing which function is invoked by that idempotent callback:
when we finish doing SSH greeting exchange and move on to the packet-
structured main phase of the protocol, we just change
ssh->current_incoming_data_fn and ensure that the new function gets
called to take over anything still outstanding in the queue.
This simplifies the _other_ end of the API of the rdpkt functions. In
the previous commit, they stopped returning their 'struct Packet'
directly, and instead put it on a queue; in this commit, they're no
longer receiving a (data, length) pair in their parameter list, and
instead, they're just reading from ssh->incoming_data. So now, API-
wise, they take no arguments at all except the main 'ssh' state
structure.
It's not just the rdpkt functions that needed to change, of course.
The SSH greeting handlers have also had to switch to reading from
ssh->incoming_data, and are quite substantially rewritten as a result.
(I think they look simpler in the new style, personally.)
This new bufchain takes over from the previous queued_incoming_data,
which was only used at all in cases where we throttled the entire SSH
connection. Now, data is unconditionally left on the new bufchain
whether we're throttled or not, and the only question is whether we're
currently bothering to read it; so all the special-purpose code to
read data from a bufchain and pass it to rdpkt can go away, because
rdpkt itself already knows how to do that job.
One slightly fiddly point is that we now have to defer processing of
EOF from the SSH server: if we have data already in the incoming
bufchain and then the server slams the connection shut, we want to
process the data we've got _before_ reacting to the remote EOF, just
in case that data gives us some reason to change our mind about how we
react to the EOF, or a last-minute important piece of data we might
need to log.
Each of the coroutines that parses the incoming wire data into a
stream of 'struct Packet' now delivers those packets to a PacketQueue
called ssh->pq_full (containing the full, unfiltered stream of all
packets received on the SSH connection), replacing the old API in
which each coroutine would directly return a 'struct Packet *' to its
caller, or NULL if it didn't have one ready yet.
This simplifies the function-call API of the rdpkt coroutines (they
now return void). It increases the complexity at the other end,
because we've now got a function ssh_process_pq_full (scheduled as an
idempotent callback whenever rdpkt appends anything to the queue)
which pulls things out of the queue and passes them to ssh->protocol.
But that's only a temporary complexity increase; by the time I finish
the upcoming stream of refactorings, there won't be two chained
functions there any more.
One small workaround I had to add in this commit is a flag called
'pending_newkeys', which ssh2_rdpkt sets when it's just returned an
SSH_MSG_NEWKEYS packet, and then waits for the transport layer to
process the NEWKEYS and set up the new encryption context before
processing any more wire data. This wasn't necessary before, because
the old architecture was naturally synchronous - ssh2_rdpkt would
return a NEWKEYS, which would be immediately passed to
do_ssh2_transport, which would finish processing it immediately, and
by the time ssh2_rdpkt was next called, the keys would already be in
place.
This change adds a big while loop around the whole of each rdpkt
function, so it's easiest to read it as a whitespace-ignored diff.
NFC for the moment, because the bufchain is always specially
constructed to hold exactly the same data that would have been passed
in to the function as a (pointer,length) pair. But this API change
allows get_userpass_input to express the idea that it consumed some
but not all of the data in the bufchain, which means that later on
I'll be able to point the same function at a longer-lived bufchain
containing the full stream of keyboard input and avoid dropping
keystrokes that arrive too quickly after the end of an interactive
password prompt.
The crWaitUntil macros have do-while type semantics, i.e. they always
crReturn _at least_ once, and then perhaps more times if their
termination condition is still not met. But sometimes a coroutine will
want to wait for a condition that may _already_ be true - the key
examples being non-emptiness of a bufchain or a PacketQueue, which may
already be non-empty in spite of you having just removed something
from its head.
In that situation, it's obviously more convenient not to bother with a
crReturn in the first place than to do one anyway and have to fiddle
about with toplevel callbacks to make sure we resume later. So here's
a new pair of macros crMaybeWaitUntil{,V}, which have the semantics of
while rather than do-while, i.e. they test the condition _first_ and
don't return at all if it's already met.
This is another piece of not-yet-used infrastructure, which later on
will simplify my life when I start processing PacketQueues and adding
some of their packets to other PacketQueues, because this way the code
can unref every packet removed from the source queue in the same way,
whether or not the packet is actually finished with.
This is just a linked list of 'struct Packet' with a convenience API
on the front. As yet it's unused, so ssh.c will currently not compile
with gcc -Werror unless you also add -Wno-unused-function. But all the
functions I've added here will be used in later commits in the current
patch series, so that's only a temporary condition.
Previously it was local, which _mostly_ worked, except that if the SSH
host key needed verifying via a non-modal dialog box, there could be a
crReturn in between writing it and reading it.
It's pretty tempting to suggest that because nobody has noticed this
before, SSH-1 can't be needed any more! But actually I suspect the
intervening crReturn has only appeared since the last release,
probably around November when I was messing about with GTK dialog box
modality. (I observed the problem just now on the GTK build, while
trying to check that a completely different set of changes hadn't
broken SSH-1.)
It hasn't been used since 2012, when commit 8e0ab8be5 introduced a new
method of getting the do_ssh2_authconn coroutine started, and didn't
notice that the variable we were previously using was now completely
unused.
The 2-minutely check to see whether new GSS credentials need to be
forwarded to the server is pointless if we're not even in the mode
where we _have_ forwarded a previous set.
This was made obvious by the overly verbose diagnostic fixed in the
previous commit, so it's a good thing that bug was temporarily there!
Now we don't generate that message as a side effect of the periodic
check for new GSS credentials; we only generate it as part of the much
larger slew of messages that happen during a rekey.
