Re: [Curdle] Mirja Kühlewind's Discuss on draft-ietf-curdle-ssh-curves-10: (with DISCUSS and COMMENT)

[Ron Frederick <[e-mail]>]

On Aug 28, 2019, at 10:08 AM, Aris Adamantiadis <[e-mail]> wrote:
> Le 28-08-19 à 17:53, Mark D. Baushke a écrit :
>> In section 3, the appropriate SSH "protocol error" might be one of a
>> two:
>> 
>>   SSH_ERR_KEY_INVALID_EC_VALUE for zeros found for the all-zero shared
>>   secret
>> 
>> and
>>      SSH_ERR_SIGNATURE_INVALID if the blob is not the correct size for a
>>   public key
>> 
>> However, an implementation miight return SSH_ERR_SIGNATURE_INVALID
>> rather than SSH_ERR_KEY_INVALID_EC_VALUE depending on how the
>> implementation was written.
>> 
>> As long as the connection aborts and does not continue in the case of a
>> "protocol error", it is sufficient in my opinion.
>> 
>> Is it really needful to be prescriptive in this given that many
>> implementations of this code have already been fielded and I do not
>> have any ability to know which design choices they made?
> 
> Hi Mark,
> 
> I would think it's best that as little information as possible about the error is sent over the SSH connection. This vector was historically used to exploit other cryptographic protocols, enabling attacks such as padding oracles and others. Additionally it doesn't really help the end user who would see this message, because this is clearly an implementation issue within the SSH software stack, not something that the user can tweak in a configuration file.
> 
> I'd then recommend a SHOULD send SSH_ERR_SIGNATURE_INVALID for all cases where protocol errors happen when handling the shared secret or the peer's key, if we really wish to be prescriptive, something I don't think is important.

The SSH_ERR_* names are not actually wire protocol constructs, so I don’t believe they should be used in this spec. From what I can see, they are values defined internally by OpenSSH and LibSSH (and possibly others), but in the end they are converted to a text string before being used anywhere. I didn’t track all the way through the code to see if the text strings ever appear on the wire or not or only in local log messages, but if they did appear on the wire I expect they would be as the “description” text in an SSH_MSG_DISCONNECT message. The “reason code” in that message comes from a much smaller list (defined in RFC 4253 section 11.1) and would probably be set to SSH_DISCONNECT_PROTOCOL_ERROR or perhaps SSH_DISCONNECT_KEY_EXCHANGE_FAILED.

I agree that this spec should not attempt to go into detail about what error is returned, especially not for a text string like this. I could see potentially recommending that a disconnect be performed, but even attempting to recommend the specific disconnect reason runs the risk of not matching what existing implementations are currently doing in this case and I see very little value in trying to standardize the exact disconnect reason.
-- 
Ron Frederick
[e-mail]