From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#49055: 28.0.50; [PATCH] De-obfuscate gnutls_handshake loop Date: Sat, 19 Jun 2021 20:51:51 +0300 Message-ID: <83zgvlvj2g.fsf@gnu.org> References: <87zgvqd35s.fsf@dick> <87zgvmyost.fsf@gnus.org> <83im2avuuu.fsf@gnu.org> <87o8c2vupo.fsf@gnus.org> <83h7huvucx.fsf@gnu.org> <875yy9wyg2.fsf@dick> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="24444"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 49055@debbugs.gnu.org To: dick.r.chiang@gmail.com, Paul Eggert Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Jun 19 19:52:11 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1luf8d-0006AZ-Cz for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 19 Jun 2021 19:52:11 +0200 Original-Received: from localhost ([::1]:55806 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1luf8c-0004sr-CF for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 19 Jun 2021 13:52:10 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:60396) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1luf8U-0004rC-Js for bug-gnu-emacs@gnu.org; Sat, 19 Jun 2021 13:52:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:49216) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1luf8U-0003tC-C2 for bug-gnu-emacs@gnu.org; Sat, 19 Jun 2021 13:52:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1luf8U-0003f5-CI for bug-gnu-emacs@gnu.org; Sat, 19 Jun 2021 13:52:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 19 Jun 2021 17:52:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 49055 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 49055-submit@debbugs.gnu.org id=B49055.162412511114059 (code B ref 49055); Sat, 19 Jun 2021 17:52:02 +0000 Original-Received: (at 49055) by debbugs.gnu.org; 19 Jun 2021 17:51:51 +0000 Original-Received: from localhost ([127.0.0.1]:60762 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1luf8J-0003eg-AR for submit@debbugs.gnu.org; Sat, 19 Jun 2021 13:51:51 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:34196) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1luf8I-0003eL-4V for 49055@debbugs.gnu.org; Sat, 19 Jun 2021 13:51:50 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:44240) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1luf8C-0003Xv-74; Sat, 19 Jun 2021 13:51:44 -0400 Original-Received: from 84.94.185.95.cable.012.net.il ([84.94.185.95]:2000 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1luf8B-0001hL-Jr; Sat, 19 Jun 2021 13:51:44 -0400 In-Reply-To: <875yy9wyg2.fsf@dick> (dick.r.chiang@gmail.com) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:208763 Archived-At: > From: dick.r.chiang@gmail.com > Cc: 49055@debbugs.gnu.org > Date: Sat, 19 Jun 2021 13:34:21 -0400 > > > . the gnutls_error_is_fatal call is missing from the new code > > Yes, and just as well since it's redundant with `emacs_gnutls_handle_error`. Not really, no: emacs_gnutls_handle_error does more, so the code is not equivalent. > > . the negative values of 'ret' (if they are significant) aren't > > tested anymore > > This unchanged line 626 begs to differ. > > while ((ret = gnutls_handshake (state)) < 0) Only if we accept your removal of the inner loop. Which I don't, at least not yet; see below. > > . the condition of GNUTLS_E_INTERRUPTED is tested only once, and > > immediately causes the outer while-loop to be abandoned > > Yes, as the commit before e87e6a2 did. You do realize I hope that e87e6a2, in > its desire to keep the loop going under GNUTLS_E_INTERRUPTED, almost > certainly did not intend to call `gnutls_handshake` twice when > GNUTLS_E_INTERRUPTED was not applicable. Oh, yes, it did intend that: the changes are explicitly so that interrupted calls don't fail because they were interrupted, but are retried. > > I'd love to see some rationale for these differences. > > Your skepticism is a credit to your earnestness. However, your expert > scrutiny is better applied to misguided commits like e87e6a2 and d84d69d. The thing is that I understand the changes in e87e6a2, and don't consider them misguided. They make Emacs more reliable in the rare situations where the TLS handshake is interrupted. What happens with your changes if during the TLS handshake you repeatedly and quickly press C-g several times? Paul, I'd appreciate your views on this, please.