Fixed CORE-4964: Real errors during connect to security database are …

…hidden by Srp user manager. Errors should be logged no matter what AuthServer is used. (taking into an account Sean's request re. special error for system-related problems)
FirebirdSQL · Dec 22, 2015 · df2fb33 · df2fb33
1 parent a4b437e
commit df2fb33
Show file tree

Hide file tree

Showing 13 changed files with 44 additions and 27 deletions.
diff --git a/lang_helpers/gds_codes.ftn b/lang_helpers/gds_codes.ftn
@@ -1622,6 +1622,8 @@ C --
       PARAMETER (GDS__invalid_attachment_charset       = 335545104)
       INTEGER*4 GDS__map_down                        
       PARAMETER (GDS__map_down                         = 335545105)
+      INTEGER*4 GDS__login_error                     
+      PARAMETER (GDS__login_error                      = 335545106)
       INTEGER*4 GDS__gfix_db_name                    
       PARAMETER (GDS__gfix_db_name                     = 335740929)
       INTEGER*4 GDS__gfix_invalid_sw                 

diff --git a/lang_helpers/gds_codes.pas b/lang_helpers/gds_codes.pas
@@ -1617,6 +1617,8 @@
 	gds_invalid_attachment_charset       = 335545104;
 	isc_map_down                         = 335545105;
 	gds_map_down                         = 335545105;
+	isc_login_error                      = 335545106;
+	gds_login_error                      = 335545106;
 	isc_gfix_db_name                     = 335740929;
 	gds_gfix_db_name                     = 335740929;
 	isc_gfix_invalid_sw                  = 335740930;

diff --git a/src/auth/SecureRemotePassword/server/SrpServer.cpp b/src/auth/SecureRemotePassword/server/SrpServer.cpp
@@ -276,6 +276,7 @@ int SrpServer::authenticate(CheckStatusWrapper* status, IServerBlock* sb, IWrite
 		switch(status->getErrors()[1])
 		{
 		case isc_stream_eof:	// User name not found in security database
+			status->init();
 			return AUTH_CONTINUE;
 		default:
 			break;

diff --git a/src/auth/SecurityDatabase/LegacyServer.cpp b/src/auth/SecurityDatabase/LegacyServer.cpp
@@ -385,28 +385,21 @@ int SecurityDatabase::verify(IWriter* authBlock, IServerBlock* sBlock)
 
 void SecurityDatabase::checkStatus(const char* callName, ISC_STATUS userError)
 {
-	// showing real problems with security database to users is not good idea
-	// from security POV - therefore some generic message is used
-	// also suppress throwing errors from destructor which passes userError == 0
-
 	if (status[1] == 0)
-	{
 		return;
-	}
+
+	// suppress throwing errors from destructor which passes userError == 0
+	if (!userError)
+		return;
+
+	Arg::Gds secDbError(userError);
 
 	string message;
 	message.printf("Error in %s() API call when working with legacy security database", callName);
-	iscLogStatus(message.c_str(), status);
+	secDbError << Arg::Gds(isc_random) << message;
 
-	if (userError)
-	{
-#ifdef DEV_BUILD
-	// throw original status error
-		status_exception::raise(status);
-#else
-		Arg::Gds(userError).raise();
-#endif
-	}
+	secDbError << Arg::StatusVector(status);
+	secDbError.raise();
 }
 
 typedef HalfStaticArray<SecurityDatabase*, 4> InstancesArray;

diff --git a/src/include/gen/codetext.h b/src/include/gen/codetext.h
@@ -807,6 +807,7 @@ static const struct {
 	{"domain_primary_key_notnull", 335545103},
 	{"invalid_attachment_charset", 335545104},
 	{"map_down", 335545105},
+	{"login_error", 335545106},
 	{"gfix_db_name", 335740929},
 	{"gfix_invalid_sw", 335740930},
 	{"gfix_incmp_sw", 335740932},

diff --git a/src/include/gen/iberror.h b/src/include/gen/iberror.h
@@ -841,6 +841,7 @@ const ISC_STATUS isc_savepoint_backout_err            = 335545102L;
 const ISC_STATUS isc_domain_primary_key_notnull       = 335545103L;
 const ISC_STATUS isc_invalid_attachment_charset       = 335545104L;
 const ISC_STATUS isc_map_down                         = 335545105L;
+const ISC_STATUS isc_login_error                      = 335545106L;
 const ISC_STATUS isc_gfix_db_name                     = 335740929L;
 const ISC_STATUS isc_gfix_invalid_sw                  = 335740930L;
 const ISC_STATUS isc_gfix_incmp_sw                    = 335740932L;
@@ -1304,7 +1305,7 @@ const ISC_STATUS isc_trace_switch_user_only           = 337182757L;
 const ISC_STATUS isc_trace_switch_param_miss          = 337182758L;
 const ISC_STATUS isc_trace_param_act_notcompat        = 337182759L;
 const ISC_STATUS isc_trace_mandatory_switch_miss      = 337182760L;
-const ISC_STATUS isc_err_max                          = 1248;
+const ISC_STATUS isc_err_max                          = 1249;
 
 #else /* c definitions */
 
@@ -2115,6 +2116,7 @@ const ISC_STATUS isc_err_max                          = 1248;
 #define isc_domain_primary_key_notnull       335545103L
 #define isc_invalid_attachment_charset       335545104L
 #define isc_map_down                         335545105L
+#define isc_login_error                      335545106L
 #define isc_gfix_db_name                     335740929L
 #define isc_gfix_invalid_sw                  335740930L
 #define isc_gfix_incmp_sw                    335740932L
@@ -2578,7 +2580,7 @@ const ISC_STATUS isc_err_max                          = 1248;
 #define isc_trace_switch_param_miss          337182758L
 #define isc_trace_param_act_notcompat        337182759L
 #define isc_trace_mandatory_switch_miss      337182760L
-#define isc_err_max                          1248
+#define isc_err_max                          1249
 
 #endif
 

diff --git a/src/include/gen/msgs.h b/src/include/gen/msgs.h
@@ -810,6 +810,7 @@ Data source : @4"},		/* eds_statement */
 	{335545103, "Domain used in the PRIMARY KEY constraint of table @1 must be NOT NULL"},		/* domain_primary_key_notnull */
 	{335545104, "CHARACTER SET @1 cannot be used as a attachment character set"},		/* invalid_attachment_charset */
 	{335545105, "Some database(s) were shutdown when trying to read mapping data"},		/* map_down */
+	{335545106, "Error occurred during login, please check server firebird.log for details"},		/* login_error */
 	{335740929, "data base file name (@1) already given"},		/* gfix_db_name */
 	{335740930, "invalid switch @1"},		/* gfix_invalid_sw */
 	{335740932, "incompatible switch combination"},		/* gfix_incmp_sw */

diff --git a/src/include/gen/sql_code.h b/src/include/gen/sql_code.h
@@ -806,6 +806,7 @@ static const struct {
 	{335545103, -291}, /* 783 domain_primary_key_notnull */
 	{335545104, -204}, /* 784 invalid_attachment_charset */
 	{335545105, -901}, /* 785 map_down */
+	{335545106, -902}, /* 786 login_error */
 	{335740929, -901}, /*   1 gfix_db_name */
 	{335740930, -901}, /*   2 gfix_invalid_sw */
 	{335740932, -901}, /*   4 gfix_incmp_sw */

diff --git a/src/include/gen/sql_state.h b/src/include/gen/sql_state.h
@@ -806,6 +806,7 @@ static const struct {
 	{335545103, "42000"}, // 783 domain_primary_key_notnull
 	{335545104, "2C000"}, // 784 invalid_attachment_charset
 	{335545105, "08004"}, // 785 map_down
+	{335545106, "08006"}, // 786 login_error
 	{335740929, "00000"}, //   1 gfix_db_name
 	{335740930, "00000"}, //   2 gfix_invalid_sw
 	{335740932, "00000"}, //   4 gfix_incmp_sw

diff --git a/src/msgs/facilities2.sql b/src/msgs/facilities2.sql
@@ -1,7 +1,7 @@
 /* MAX_NUMBER is the next number to be used, always one more than the highest message number. */
 set bulk_insert INSERT INTO FACILITIES (LAST_CHANGE, FACILITY, FAC_CODE, MAX_NUMBER) VALUES (?, ?, ?, ?);
 --
-('2015-08-17 20:53:01', 'JRD', 0, 786)
+('2015-12-22 20:33:22', 'JRD', 0, 787)
 ('2015-03-17 18:33:00', 'QLI', 1, 533)
 ('2015-01-07 18:01:51', 'GFIX', 3, 134)
 ('1996-11-07 13:39:40', 'GPRE', 4, 1)

diff --git a/src/msgs/messages2.sql b/src/msgs/messages2.sql
@@ -893,6 +893,7 @@ Data source : @4', NULL, NULL)
 ('domain_primary_key_notnull', NULL, 'DdlNodes.epp', NULL, 0, 783, NULL, 'Domain used in the PRIMARY KEY constraint of table @1 must be NOT NULL', NULL, NULL);
 ('invalid_attachment_charset', NULL, NULL, NULL, 0, 784, NULL, 'CHARACTER SET @1 cannot be used as a attachment character set', NULL, NULL);
 ('map_down', NULL, 'Mapping.cpp', NULL, 0, 785, NULL, 'Some database(s) were shutdown when trying to read mapping data', NULL, NULL);
+('login_error', NULL, 'server.cpp', NULL, 0, 786, NULL, 'Error occurred during login, please check server firebird.log for details', NULL, NULL);
 -- QLI
 (NULL, NULL, NULL, NULL, 1, 0, NULL, 'expected type', NULL, NULL);
 (NULL, NULL, NULL, NULL, 1, 1, NULL, 'bad block type', NULL, NULL);

diff --git a/src/msgs/system_errors2.sql b/src/msgs/system_errors2.sql
@@ -792,6 +792,7 @@ set bulk_insert INSERT INTO SYSTEM_ERRORS (SQL_CODE, SQL_CLASS, SQL_SUBCLASS, FA
 (-291, '42', '000', 0, 783, 'domain_primary_key_notnull', NULL, NULL)
 (-204, '2C', '000', 0, 784, 'invalid_attachment_charset', NULL, NULL)
 (-901, '08', '004', 0, 785, 'map_down', NULL, NULL)
+(-902, '08', '006', 0, 786, 'login_error', NULL, NULL)
 -- GFIX
 (-901, '00', '000', 3, 1, 'gfix_db_name', NULL, NULL)
 (-901, '00', '000', 3, 2, 'gfix_invalid_sw', NULL, NULL)

diff --git a/src/remote/server/server.cpp b/src/remote/server/server.cpp
@@ -460,6 +460,7 @@ class ServerAuth : public GlobalStorage, public ServerAuthBase
 				{
 					authServer = NULL;
 					working = false;
+					(Arg::Gds(isc_random) << "Plugin not supported by network protocol").copyTo(&st);		// add port_protocol parameter
 					break;
 				}
 
@@ -495,6 +496,7 @@ class ServerAuth : public GlobalStorage, public ServerAuthBase
 					{
 						authServer = NULL;
 						working = false;
+						(Arg::Gds(isc_random) << "Plugin not supported by network protocol").copyTo(&st);		// add port_protocol parameter
 						break;
 					}
 				}
@@ -517,14 +519,21 @@ class ServerAuth : public GlobalStorage, public ServerAuthBase
 		// no success - perform failure processing
 		loginFail(userName, authPort->getRemoteId());
 
-		Arg::Gds loginError(isc_login);
-#ifndef DEV_BUILD
-		if (st.getErrors()[1] == isc_missing_data_structures)
-#endif
+		if (st.hasData())
 		{
+			if (st.getErrors()[1] == isc_missing_data_structures)
+				status_exception::raise(&st);
+
+			iscLogStatus("Authentication error", &st);
+			Arg::Gds loginError(isc_login_error);
+#ifdef DEV_BUILD
 			loginError << Arg::StatusVector(&st);
+#endif
+			loginError.raise();
 		}
-		status_exception::raise(loginError.value());
+		else
+			Arg::Gds(isc_login).raise();
+
 		return false;	// compiler warning silencer
 	}
 
@@ -6416,11 +6425,13 @@ void SrvAuthBlock::createPluginsItr()
 	if (final.getCount() == 0)
 	{
 		HANDSHAKE_DEBUG(fprintf(stderr, "Srv: createPluginsItr: No matching plugins on server\n"));
-		(Arg::Gds(isc_login)
+
+		Arg::Gds loginError(isc_login_error);
 #ifdef DEV_BUILD
-							<< Arg::Gds(isc_random) << "No matching plugins on server"
+		loginError << Arg::Gds(isc_random) << "No matching plugins on server";
 #endif
-							).raise();
+		gds__log("Authentication error\n\tNo matching plugins on server");
+		loginError.raise();
 	}
 
 	// reorder to make it match first, already passed, plugin data