Subject: Remove AUTH_REQ_KRB4 and AUTH_REQ_KRB5 in libpq code

From: Michael Paquier <[email-masked]>
Date: Mon, 20 Mar 2023 07:20:47 +0900

Hi all,

$subject has been discussed here, still seems worth its own thread for
clarity:
https://www.postgresql.org/message-id/[email-masked]

Support for Kerberos v4 has been removed in a159ad3 (2005) and the
same happened for v5 in 98de86e (2014, meaning that this is still
possible with 9.2 and 9.3 backends).  Anyway, the attached seems worth
the simplifications now?  This includes a cleanup of protocol.sgml.

Thoughts?
--
Michael
diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h
index bff7dd18a2..8251639cd3 100644
--- a/src/include/libpq/pqcomm.h
+++ b/src/include/libpq/pqcomm.h
@@ -111,8 +111,8 @@ extern PGDLLIMPORT bool Db_user_namespace;
 /* These are the authentication request codes sent by the backend. */
 
 #define AUTH_REQ_OK			0	/* User is authenticated  */
-#define AUTH_REQ_KRB4		1	/* Kerberos V4. Not supported any more. */
-#define AUTH_REQ_KRB5		2	/* Kerberos V5. Not supported any more. */
+/* 1 is available. It was used for Kerberos V4, not supported any more  */
+/* 2 is available. It was used for Kerberos V5, not supported any more  */
 #define AUTH_REQ_PASSWORD	3	/* Password */
 #define AUTH_REQ_CRYPT		4	/* crypt password. Not supported any more. */
 #define AUTH_REQ_MD5		5	/* md5 password */
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index fa95f8e6e9..1ce0794f89 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -942,14 +942,6 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
 		case AUTH_REQ_OK:
 			break;
 
-		case AUTH_REQ_KRB4:
-			libpq_append_conn_error(conn, "Kerberos 4 authentication not supported");
-			return STATUS_ERROR;
-
-		case AUTH_REQ_KRB5:
-			libpq_append_conn_error(conn, "Kerberos 5 authentication not supported");
-			return STATUS_ERROR;
-
 #if defined(ENABLE_GSS) || defined(ENABLE_SSPI)
 		case AUTH_REQ_GSS:
 #if !defined(ENABLE_SSPI)
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 8b5e7b1ad7..ed7a3ceda1 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -271,20 +271,6 @@
       </listitem>
      </varlistentry>
 
-     <varlistentry>
-      <term>AuthenticationKerberosV5</term>
-      <listitem>
-       <para>
-        The frontend must now take part in a Kerberos V5
-        authentication dialog (not described here, part of the
-        Kerberos specification) with the server.  If this is
-        successful, the server responds with an AuthenticationOk,
-        otherwise it responds with an ErrorResponse. This is no
-        longer supported.
-       </para>
-      </listitem>
-     </varlistentry>
-
      <varlistentry>
       <term>AuthenticationCleartextPassword</term>
       <listitem>
@@ -3318,41 +3304,6 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;"
     </listitem>
    </varlistentry>
 
-   <varlistentry id="protocol-message-formats-AuthenticationKerberosV5">
-    <term>AuthenticationKerberosV5 (B)</term>
-    <listitem>
-
-     <variablelist>
-      <varlistentry>
-       <term>Byte1('R')</term>
-       <listitem>
-        <para>
-         Identifies the message as an authentication request.
-        </para>
-       </listitem>
-      </varlistentry>
-
-      <varlistentry>
-       <term>Int32(8)</term>
-       <listitem>
-        <para>
-         Length of message contents in bytes, including self.
-        </para>
-       </listitem>
-      </varlistentry>
-
-      <varlistentry>
-       <term>Int32(2)</term>
-       <listitem>
-        <para>
-         Specifies that Kerberos V5 authentication is required.
-        </para>
-       </listitem>
-      </varlistentry>
-     </variablelist>
-    </listitem>
-   </varlistentry>
-
    <varlistentry id="protocol-message-formats-AuthenticationCleartextPassword">
     <term>AuthenticationCleartextPassword (B)</term>
     <listitem>

---

From: Tom Lane <[email-masked]>
Date: Sun, 19 Mar 2023 18:53:28 -0400

> the simplifications now?  This includes a cleanup of protocol.sgml.

9.2 is still within our "supported old versions" window, so it's
at least plausible that somebody would hit this for KRB5.  Still,
the net effect would be that they'd get "authentication method 2
not supported" instead of "Kerberos 5 authentication not supported".
I lean (weakly) to the idea that it's no longer worth the translation
maintenance effort to keep the special message.

A compromise could be to drop KRB4 but keep the KRB5 case for
awhile yet.

One other thought is that I don't really like these comments
implying that recycling these AUTH_REQ codes might be a good
thing to do:

I think we'd be better off treating them as permanently retired.
It's not like there's any shortage of code space to worry about.
More, there might be other implementations of our wire protocol
that still have support for these codes, so that re-using them
could cause compatibility issues.  So maybe write "reserved"
instead of "available"?

			regards, tom lane

---

From: Michael Paquier <[email-masked]>
Date: Mon, 20 Mar 2023 08:48:50 +0900

> A compromise could be to drop KRB4 but keep the KRB5 case for

Hmm.  I think that I would still drop both of them at the end, even in
v16 but I won't fight hard on that, either.  The only difference is
the verbosity of the error string generated, and there is still a
trace of what the code numbers were in pqcomm.h.

> could cause compatibility issues.  So maybe write "reserved"

Okay, fine by me.
--
Michael