Make implementation of SASLprep compliant for ASCII characters

This commit makes our implementation of SASLprep() compliant with RFC
3454 (Stringprep) and RFC 4013 (SASLprep).  Originally, as introduced in
60f11b87a234, the operation considered a password made of only ASCII
characters as valid, performing an optimization for this case to skip
the internal NFKC transformation.

However, the RFCs listed above use a different definition, with the
following characters being prohibited:
- 0x00~0x1F (0~31), control characters.
- 0x7F (127, DEL).

In its SCRAM protocol, Postgres has the idea to apply a password as-is
if SASLprep() is not a success, so this change is safe on
backward-compatibility grounds:
- A libpq client with the compliant SASLprep can connect to a server
with a non-compliant SASLprep.
- A libpq client with the non-compliant SASLprep can connect to a server
with a compliant SASLprep.

This commit removes the all-ASCII optimization used in pg_saslprep() and
applies SASLprep even if a password is made only of ASCII characters,
making the operation compatible with the RFC.  All the in-core callers
of pg_saslprep() do that:
- pg_be_scram_build_secret() in auth-scram.c, when generating a
SCRAM verifier for rolpassword in the backend.
- scram_init() in fe-auth-scram.c, when starting the SASL exchange.
- pg_fe_scram_build_secret() in fe-auth-scram.c, when generating a SCRAM
verifier for the frontend with libpq, to generate it for a ALTER/CREATE
ROLE command for example.

The test module test_saslprep shows the difference this change is
leading to.

Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Discussion: https://postgr.es/m/aaEJ-El2seZHeFcG@paquier.xyz

diff --git a/src/common/saslprep.c b/src/common/saslprep.c
index 2ad2cefc14fbdf368cb9267dc7d970f80feccf9e..38d50dd823c4162020cd15c700cdbb7453b3e58b 100644 (file)
--- a/src/common/saslprep.c
+++ b/src/common/saslprep.c
@@ -1061,18 +1061,6 @@ pg_saslprep(const char *input, char **output)
        /* Ensure we return *output as NULL on failure */
        *output = NULL;
 
-       /*
-        * Quick check if the input is pure ASCII.  An ASCII string requires no
-        * further processing.
-        */
-       if (pg_is_ascii(input))
-       {
-               *output = STRDUP(input);
-               if (!(*output))
-                       goto oom;
-               return SASLPREP_SUCCESS;
-       }
-
        /*
         * Convert the input from UTF-8 to an array of Unicode codepoints.
         *

diff --git a/src/test/modules/test_saslprep/expected/test_saslprep.out b/src/test/modules/test_saslprep/expected/test_saslprep.out
index f72dbffa0a11b34cecdfd2b9ea452dde4e61d542..92f93365343e31d76d00a04b9c424e9461a4430f 100644 (file)
--- a/src/test/modules/test_saslprep/expected/test_saslprep.out
+++ b/src/test/modules/test_saslprep/expected/test_saslprep.out
@@ -19,38 +19,38 @@ SELECT
   FROM generate_series(0,127) AS a;
    dat    | byt  |     saslprep      
 ----------+------+-------------------
- <NUL>    | \x00 | ("\\x",SUCCESS)
- <CTL_1>  | \x01 | ("\\x01",SUCCESS)
- <CTL_2>  | \x02 | ("\\x02",SUCCESS)
- <CTL_3>  | \x03 | ("\\x03",SUCCESS)
- <CTL_4>  | \x04 | ("\\x04",SUCCESS)
- <CTL_5>  | \x05 | ("\\x05",SUCCESS)
- <CTL_6>  | \x06 | ("\\x06",SUCCESS)
- <CTL_7>  | \x07 | ("\\x07",SUCCESS)
- <CTL_8>  | \x08 | ("\\x08",SUCCESS)
- <CTL_9>  | \x09 | ("\\x09",SUCCESS)
- <CTL_10> | \x0a | ("\\x0a",SUCCESS)
- <CTL_11> | \x0b | ("\\x0b",SUCCESS)
- <CTL_12> | \x0c | ("\\x0c",SUCCESS)
- <CTL_13> | \x0d | ("\\x0d",SUCCESS)
- <CTL_14> | \x0e | ("\\x0e",SUCCESS)
- <CTL_15> | \x0f | ("\\x0f",SUCCESS)
- <CTL_16> | \x10 | ("\\x10",SUCCESS)
- <CTL_17> | \x11 | ("\\x11",SUCCESS)
- <CTL_18> | \x12 | ("\\x12",SUCCESS)
- <CTL_19> | \x13 | ("\\x13",SUCCESS)
- <CTL_20> | \x14 | ("\\x14",SUCCESS)
- <CTL_21> | \x15 | ("\\x15",SUCCESS)
- <CTL_22> | \x16 | ("\\x16",SUCCESS)
- <CTL_23> | \x17 | ("\\x17",SUCCESS)
- <CTL_24> | \x18 | ("\\x18",SUCCESS)
- <CTL_25> | \x19 | ("\\x19",SUCCESS)
- <CTL_26> | \x1a | ("\\x1a",SUCCESS)
- <CTL_27> | \x1b | ("\\x1b",SUCCESS)
- <CTL_28> | \x1c | ("\\x1c",SUCCESS)
- <CTL_29> | \x1d | ("\\x1d",SUCCESS)
- <CTL_30> | \x1e | ("\\x1e",SUCCESS)
- <CTL_31> | \x1f | ("\\x1f",SUCCESS)
+ <NUL>    | \x00 | (,PROHIBITED)
+ <CTL_1>  | \x01 | (,PROHIBITED)
+ <CTL_2>  | \x02 | (,PROHIBITED)
+ <CTL_3>  | \x03 | (,PROHIBITED)
+ <CTL_4>  | \x04 | (,PROHIBITED)
+ <CTL_5>  | \x05 | (,PROHIBITED)
+ <CTL_6>  | \x06 | (,PROHIBITED)
+ <CTL_7>  | \x07 | (,PROHIBITED)
+ <CTL_8>  | \x08 | (,PROHIBITED)
+ <CTL_9>  | \x09 | (,PROHIBITED)
+ <CTL_10> | \x0a | (,PROHIBITED)
+ <CTL_11> | \x0b | (,PROHIBITED)
+ <CTL_12> | \x0c | (,PROHIBITED)
+ <CTL_13> | \x0d | (,PROHIBITED)
+ <CTL_14> | \x0e | (,PROHIBITED)
+ <CTL_15> | \x0f | (,PROHIBITED)
+ <CTL_16> | \x10 | (,PROHIBITED)
+ <CTL_17> | \x11 | (,PROHIBITED)
+ <CTL_18> | \x12 | (,PROHIBITED)
+ <CTL_19> | \x13 | (,PROHIBITED)
+ <CTL_20> | \x14 | (,PROHIBITED)
+ <CTL_21> | \x15 | (,PROHIBITED)
+ <CTL_22> | \x16 | (,PROHIBITED)
+ <CTL_23> | \x17 | (,PROHIBITED)
+ <CTL_24> | \x18 | (,PROHIBITED)
+ <CTL_25> | \x19 | (,PROHIBITED)
+ <CTL_26> | \x1a | (,PROHIBITED)
+ <CTL_27> | \x1b | (,PROHIBITED)
+ <CTL_28> | \x1c | (,PROHIBITED)
+ <CTL_29> | \x1d | (,PROHIBITED)
+ <CTL_30> | \x1e | (,PROHIBITED)
+ <CTL_31> | \x1f | (,PROHIBITED)
           | \x20 | ("\\x20",SUCCESS)
  !        | \x21 | ("\\x21",SUCCESS)
  "        | \x22 | ("\\x22",SUCCESS)
@@ -146,7 +146,7 @@ SELECT
  |        | \x7c | ("\\x7c",SUCCESS)
  }        | \x7d | ("\\x7d",SUCCESS)
  ~        | \x7e | ("\\x7e",SUCCESS)
- <DEL>    | \x7f | ("\\x7f",SUCCESS)
+ <DEL>    | \x7f | (,PROHIBITED)
 (128 rows)
 
 DROP EXTENSION test_saslprep;

diff --git a/src/test/modules/test_saslprep/t/001_saslprep_ranges.pl b/src/test/modules/test_saslprep/t/001_saslprep_ranges.pl
index 3ff0e3b10e674fc9b42b4b6af04d4f63b18c5110..4a7cb5aaa588609a704322f1982c4246df236235 100644 (file)
--- a/src/test/modules/test_saslprep/t/001_saslprep_ranges.pl
+++ b/src/test/modules/test_saslprep/t/001_saslprep_ranges.pl
@@ -25,15 +25,12 @@ $node->safe_psql('postgres', 'CREATE EXTENSION test_saslprep;');
 # Among all the valid UTF-8 codepoint ranges, our implementation of
 # SASLprep should never return an empty password if the operation is
 # considered a success.
-# The only exception is currently the nul character, prohibited in
-# input of CREATE/ALTER ROLE.
 my $result = $node->safe_psql(
        'postgres', qq[SELECT * FROM test_saslprep_ranges()
   WHERE status = 'SUCCESS' AND res IN (NULL, '')
 ]);
 
-is($result, 'U+0000|SUCCESS|\x00|\x',
-       "valid codepoints returning an empty password");
+is($result, '', "valid codepoints returning an empty password");
 
 $node->stop;
 done_testing();