View Issue Details

IDProjectCategoryView StatusLast Update
0000419Pgpool-IIBugpublic2018-11-01 09:29
ReporterjpedersenAssigned Tohoshiai 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionopen 
Product Version 
Target Version4.0.1Fixed in Version4.0.1 
Summary0000419: pg_md5: -m removes users
DescriptionWhen calling pg_md5 -m -u user2 password2 with an existing pool_passwd file, as

user1:password1
user2:password2
user3:password3

the resulting file will contain

user1:password1
user2:md5XXX

So, user3's entry was removed.
TagsNo tags attached.

Activities

jpedersen

2018-08-09 21:45

reporter   ~0002139

Using master branch as of August 9, 2018

pengbo

2018-08-13 09:04

developer   ~0002145

I will look into this one.

hoshiai

2018-08-16 16:43

developer   ~0002160

This case occurs because new password length is different old password length in pool_passwd file.

Usually pool_passwd file only contains encrypted password(length is 32 characters),
so Pgpool-II simple rewrite 32 characters .
In this case , pool_passwd file probably edited directly and contained unencrypted password(not 32 charcters).

jpedersen

2018-08-16 20:11

reporter   ~0002166

Assuming 32 characters won't work in all cases, like

pg_md5 -m -f conf/pgpool.conf -u user1 password1
pg_enc -m -f conf/pgpool.conf -K mypass -u user2 password2
pg_md5 -m -f conf/pgpool.conf -u user3 password3

gives

user1:md5bb1ea422b041dd851629d58572c7c744
user2:AESeIacnDW5ZOaHMzyCZKu7Nw==
user3:md56ca81b90e107955fffea74bfa0e5d546

Then

pg_md5 -m -f conf/pgpool.conf -u user2 password2

gives

user1:md5bb1ea422b041dd851629d58572c7c744
user2:md582e1b7ca43a87dbcf129edf1d026f97d
56ca81b90e107955fffea74bfa0e5d546

The user3 entry is broken.

hoshiai

2018-08-20 17:32

developer   ~0002168

you are right. When using pg_enc will added with Pgpool-II 4.0, It is not always 32 Charcters.
This problem needs discussion.

Pgpool-II stable version(before 3.7), password length is always 32 characters.
Not incorrectly overwrite other user information, So I created a patch for stable version .

bug419.patch (884 bytes)
diff --git a/src/auth/pool_passwd.c b/src/auth/pool_passwd.c
index 660a6c5..ec88d60 100644
--- a/src/auth/pool_passwd.c
+++ b/src/auth/pool_passwd.c
@@ -129,12 +129,26 @@ int pool_create_passwdent(char *username, char *passwd)
 		if (!strcmp(username, name))
 		{
 			/* User name found. Update password. */
-			while ((c = *passwd++))
+			readlen = 0;
+			while((c = fgetc(passwd_fd)) != EOF && c != '\n')
+			{
+				readlen++;
+			}
+			if(readlen != POOL_PASSWD_LEN)
+			{
+				ereport(ERROR,
+					(errmsg("error updating password, invalid password length in pool_passwd file username:%s",name)));
+			}
+			else
 			{
-				fputc(c, passwd_fd);
+				fseek(passwd_fd, -(POOL_PASSWD_LEN+1), SEEK_CUR);
+				while ((c = *passwd++))
+				{
+					fputc(c, passwd_fd);
+				}
+				fputc('\n', passwd_fd);
+				return 0;
 			}
-			fputc('\n', passwd_fd);
-			return 0;
 		}
 		else
 		{
bug419.patch (884 bytes)

jpedersen

2018-08-20 20:33

reporter   ~0002169

POOL_PASSWD_LEN is the maximum length of a password; either plain text (PASSWORD_TYPE_PLAINTEXT) or encoded (PASSWORD_TYPE_MD5 / PASSWORD_TYPE_AES / PASSWORD_TYPE_SCRAM_SHA_256). The current patch won't work with PASSWORD_TYPE_PLAINTEXT, where the password isn't POOL_PASSWD_LEN.

pool_create_passwdent() should also check that 'len' isn't higher than POOL_PASSWD_LEN. pool_get_passwd() won't read past that, which is correct - see pool_passwd.c:224.

The patch could keep track of the previous position of the '\n' character (or start of file), and only replace the string within the previous and the current position of the '\n' character - however, this will require some logic to deal with passwords that are either shorter or longer than the previous password. Another idea could be to create a new file and swap pool_passwd.tmp with pool_passwd.

t-ishii

2018-08-22 13:52

developer   ~0002170

I think we should emit error and abort if pg_md5 finds invalid length entry in pool_passwd because it's a corrupted pool_passwd. With the patch (bug419.patch) happily updates pool_passwd which is something like this.

foo:aaa
t-ishii:md56963d0223207af3de48ff43391db1347

hoshiai

2018-08-23 10:17

developer   ~0002172

I understood your idea, I think its a good idea.
I fix that pool_create_passwdent function check all user' password in pool_passwd file.

bug419v2.patch (1,108 bytes)
diff --git a/src/auth/pool_passwd.c b/src/auth/pool_passwd.c
index 660a6c5..e5260c4 100644
--- a/src/auth/pool_passwd.c
+++ b/src/auth/pool_passwd.c
@@ -91,6 +91,7 @@ int pool_create_passwdent(char *username, char *passwd)
 	char name[MAX_USER_NAME_LEN];
 	char *p;
 	int readlen;
+	int passwdlen;
 
 	if (!passwd_fd)
 		ereport(ERROR,
@@ -106,6 +107,34 @@ int pool_create_passwdent(char *username, char *passwd)
 				(errmsg("error updating password, invalid password length:%d",len)));
 
 	rewind(passwd_fd);
+	while (!feof(passwd_fd))
+	{
+		p = name;
+		readlen = 0;
+		while (readlen < sizeof(name))
+		{
+			c = fgetc(passwd_fd);
+			if (c == EOF)
+				break;
+			else if (c == ':')
+				break;
+			readlen++;
+			*p++ = c;
+		}
+		*p = '\0';
+
+		passwdlen=0;
+		if (readlen){
+			while((c = fgetc(passwd_fd)) != EOF && c != '\n')
+			{
+				passwdlen++;
+			}
+			if(passwdlen != POOL_PASSWD_LEN)
+				ereport(ERROR,
+					(errmsg("error updating password, invalid password length in pool_passwd file username:%s",name)));
+		}
+	}
+	rewind(passwd_fd);
 	name[0] = '\0';
 
 	while (!feof(passwd_fd))
bug419v2.patch (1,108 bytes)

t-ishii

2018-08-27 11:18

developer   ~0002174

hoshiai, the patch doesn't apply to current master branch. For which branch did you create the patch?

hoshiai

2018-08-27 14:17

developer   ~0002175

Sorry , patch created V3_7_5 tag, I shoud have done it on 3_7_STABLE.

Its patch was wrong coding rule, so I fixed and recreated patch on 3_7_STABLE.

bug419v3.patch (1,114 bytes)
diff --git a/src/auth/pool_passwd.c b/src/auth/pool_passwd.c
index 660a6c5..9230006 100644
--- a/src/auth/pool_passwd.c
+++ b/src/auth/pool_passwd.c
@@ -91,6 +91,7 @@ int pool_create_passwdent(char *username, char *passwd)
 	char name[MAX_USER_NAME_LEN];
 	char *p;
 	int readlen;
+	int passwdlen;
 
 	if (!passwd_fd)
 		ereport(ERROR,
@@ -106,6 +107,35 @@ int pool_create_passwdent(char *username, char *passwd)
 				(errmsg("error updating password, invalid password length:%d",len)));
 
 	rewind(passwd_fd);
+	while (!feof(passwd_fd))
+	{
+		p = name;
+		readlen = 0;
+		while (readlen < sizeof(name))
+		{
+			c = fgetc(passwd_fd);
+			if (c == EOF)
+				break;
+			else if (c == ':')
+				break;
+			readlen++;
+			*p++ = c;
+		}
+		*p = '\0';
+
+		passwdlen=0;
+		if (readlen)
+		{
+			while ((c = fgetc(passwd_fd)) != EOF && c != '\n')
+			{
+				passwdlen++;
+			}
+			if (passwdlen != POOL_PASSWD_LEN)
+				ereport(ERROR,
+					(errmsg("error updating password, invalid password length in pool_passwd file username:%s",name)));
+		}
+	}
+	rewind(passwd_fd);
 	name[0] = '\0';
 
 	while (!feof(passwd_fd))
bug419v3.patch (1,114 bytes)

t-ishii

2018-08-27 18:05

developer   ~0002177

I have pushed the patch along with minor error message tweak to 3.4 to 3.7 branch.

For master branch, now it is possible to mix up different password format, so completely different patch is required.
We need to wait for Usama to deal with clear text password format.

hoshiai

2018-10-31 11:21

developer   ~0002231

Mr. Usama pushed the patch to 4.0 branch.

https://git.postgresql.org/gitweb/?p=pgpool2.git;a=commitdiff;h=c78cb93e5337c825e463c3c6cfd418bcf0eccb31

Issue History

Date Modified Username Field Change
2018-08-09 21:38 jpedersen New Issue
2018-08-09 21:45 jpedersen Note Added: 0002139
2018-08-13 09:04 pengbo Note Added: 0002145
2018-08-16 13:35 hoshiai Assigned To => hoshiai
2018-08-16 13:35 hoshiai Status new => assigned
2018-08-16 16:43 hoshiai Note Added: 0002160
2018-08-16 20:11 jpedersen Note Added: 0002166
2018-08-20 17:32 hoshiai File Added: bug419.patch
2018-08-20 17:32 hoshiai Note Added: 0002168
2018-08-20 20:33 jpedersen Note Added: 0002169
2018-08-21 11:44 hoshiai Status assigned => confirmed
2018-08-22 13:52 t-ishii Note Added: 0002170
2018-08-23 10:17 hoshiai File Added: bug419v2.patch
2018-08-23 10:17 hoshiai Note Added: 0002172
2018-08-27 11:18 t-ishii Note Added: 0002174
2018-08-27 14:17 hoshiai File Added: bug419v3.patch
2018-08-27 14:17 hoshiai Note Added: 0002175
2018-08-27 18:05 t-ishii Note Added: 0002177
2018-10-31 11:21 hoshiai Reproducibility have not tried => always
2018-10-31 11:21 hoshiai Target Version => 4.0.1
2018-10-31 11:21 hoshiai Note Added: 0002231
2018-11-01 09:29 hoshiai Status confirmed => resolved
2018-11-01 09:29 hoshiai Fixed in Version => 4.0.1