Skip to content

Commit

Permalink
Merge pull request #1700 from bjoernricks/gsa-9.0
Browse files Browse the repository at this point in the history 
Fix handling error response during gmp authentication in gsad
  • Loading branch information
@bjoernricks
bjoernricks committed Oct 18, 2019
2 parents ab9e575 + cc6c468 commit 58abfa3
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 52 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
### Changed

### Fixed
- Handle authentication errors in gsad more carefully [#1700](https://github.com/greenbone/gsa/pull/1700)

### Removed

Expand Down
76 changes: 58 additions & 18 deletions gsad/src/gsad.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1423,23 +1423,45 @@ exec_gmp_post (http_connection_t *con, gsad_connection_info_t *con_info,
{
case 0:
break;
case -1:
case 1: /* manager closed connection */
cmd_response_data_set_status_code (response_data,
MHD_HTTP_INTERNAL_SERVER_ERROR);
res = gsad_message (credentials, "Internal error", __FUNCTION__, __LINE__,
"An internal error occurred. "
"Diagnostics: Could not connect to manager daemon. "
"Manager closed the connection.",
response_data);
break;
case 2: /* auth failed */
cmd_response_data_free (response_data);
return handler_send_reauthentication (con, MHD_HTTP_SERVICE_UNAVAILABLE,
GMP_SERVICE_DOWN);
case -2:
return handler_send_reauthentication (con, MHD_HTTP_UNAUTHORIZED,
LOGIN_FAILED);
case 3: /* timeout */
cmd_response_data_set_status_code (response_data,
MHD_HTTP_INTERNAL_SERVER_ERROR);
res = gsad_message (credentials, "Internal error", __FUNCTION__, __LINE__,
"An internal error occurred. "
"Diagnostics: Could not authenticate to manager "
"daemon.",
"Diagnostics: Could not connect to manager daemon. "
"Connection timeout.",
response_data);
break;
default:
case 4: /* can't connect to manager */
cmd_response_data_set_status_code (response_data,
MHD_HTTP_SERVICE_UNAVAILABLE);
res = gsad_message (credentials, "Internal error", __FUNCTION__, __LINE__,
"An internal error occurred. "
"Diagnostics: Failure to connect to manager daemon.",
"Diagnostics: Could not connect to manager daemon. "
"Could not open a connection.",
response_data);
break;
default: /* unknown error */
cmd_response_data_set_status_code (response_data,
MHD_HTTP_INTERNAL_SERVER_ERROR);
res = gsad_message (credentials, "Internal error", __FUNCTION__, __LINE__,
"An internal error occurred. "
"Diagnostics: Could not connect to manager daemon. "
"Unknown error.",
response_data);
}

if (res)
Expand Down Expand Up @@ -1871,26 +1893,44 @@ exec_gmp_get (http_connection_t *con, gsad_connection_info_t *con_info,
{
case 0:
break;
case -1:
case 1: /* manager closed connection */
cmd_response_data_set_status_code (response_data,
MHD_HTTP_INTERNAL_SERVER_ERROR);
res = gsad_message (credentials, "Internal error", __FUNCTION__, __LINE__,
"An internal error occurred. "
"Diagnostics: Could not connect to manager "
"daemon.",
"Diagnostics: Could not connect to manager daemon. "
"Manager closed the connection.",
response_data);
break;

case -2:
case 2: /* auth failed */
cmd_response_data_free (response_data);
return handler_send_reauthentication (con, MHD_HTTP_UNAUTHORIZED,
LOGIN_FAILED);
case 3: /* timeout */
cmd_response_data_set_status_code (response_data,
MHD_HTTP_INTERNAL_SERVER_ERROR);
res = gsad_message (credentials, "Internal error", __FUNCTION__, __LINE__,
"An internal error occurred. "
"Diagnostics: Could not connect to manager daemon. "
"Connection timeout.",
response_data);
break;
case 4: /* can't connect to manager */
cmd_response_data_set_status_code (response_data,
MHD_HTTP_SERVICE_UNAVAILABLE);
res = gsad_message (credentials, "Internal error", __FUNCTION__, __LINE__,
"An internal error occurred. "
"Diagnostics: Could not authenticate to manager "
"daemon.",
"Diagnostics: Could not connect to manager daemon. "
"Could not open a connection.",
response_data);
break;
default:
default: /* unknown error */
cmd_response_data_set_status_code (response_data,
MHD_HTTP_INTERNAL_SERVER_ERROR);
res = gsad_message (credentials, "Internal error", __FUNCTION__, __LINE__,
"An internal error occurred. "
"Diagnostics: Failure to connect to manager "
"daemon.",
"Diagnostics: Could not connect to manager daemon. "
"Unknown error.",
response_data);
}

Expand Down
72 changes: 46 additions & 26 deletions gsad/src/gsad_gmp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17168,7 +17168,7 @@ gvm_connection_open (gvm_connection_t *connection, const gchar *address,
* @param[out] language User Interface Language, or NULL.
* @param[out] pw_warning Password warning message, NULL if password is OK.
*
* @return 0 if valid, 1 failed, 2 manager down, -1 error.
* @return 0 if valid, 1 manager down, 2 failed, 3 timeout, -1 error.
*/
int
authenticate_gmp (const gchar *username, const gchar *password, gchar **role,
Expand All @@ -17182,7 +17182,7 @@ authenticate_gmp (const gchar *username, const gchar *password, gchar **role,
if (gvm_connection_open (&connection, manager_address, manager_port))
{
g_debug ("%s failed to acquire socket!\n", __FUNCTION__);
return 2;
return 1;
}

auth_opts = gmp_authenticate_info_opts_defaults;
Expand Down Expand Up @@ -17214,7 +17214,7 @@ authenticate_gmp (const gchar *username, const gchar *password, gchar **role,
case 1:
case 2:
gvm_connection_close (&connection);
return 2;
return 1;
default:
gvm_connection_close (&connection);
return -1;
Expand All @@ -17227,7 +17227,7 @@ authenticate_gmp (const gchar *username, const gchar *password, gchar **role,
if (ret)
{
gvm_connection_close (&connection);
return 2;
return 1;
}

/* Read the response. */
Expand All @@ -17236,7 +17236,7 @@ authenticate_gmp (const gchar *username, const gchar *password, gchar **role,
if (read_entity_and_text_c (&connection, &entity, &response))
{
gvm_connection_close (&connection);
return 2;
return 1;
}

/* Check the response. */
Expand All @@ -17248,8 +17248,10 @@ authenticate_gmp (const gchar *username, const gchar *password, gchar **role,
free_entity (entity);
return -1;
}

first = status[0];
free_entity (entity);

if (first == '2')
{
*capabilities = response;
Expand All @@ -17267,7 +17269,16 @@ authenticate_gmp (const gchar *username, const gchar *password, gchar **role,
else
{
gvm_connection_close (&connection);
return 1;

switch (auth)
{
case 1: /* manager closed connection */
case 2: /* auth failed */
case 3: /* timeout */
return auth;
default:
return -1;
}
}
}

Expand All @@ -17285,7 +17296,7 @@ int
login (http_connection_t *con, params_t *params,
cmd_response_data_t *response_data, const char *client_address)
{
int ret;
int ret, status;
authentication_reason_t auth_reason;
credentials_t *credentials;
gchar *timezone;
Expand All @@ -17308,19 +17319,26 @@ login (http_connection_t *con, params_t *params,
&capabilities, &language, &pw_warning);
if (ret)
{
int status;
if (ret == -1)
status = MHD_HTTP_INTERNAL_SERVER_ERROR;
if (ret == 2)
status = MHD_HTTP_SERVICE_UNAVAILABLE;
else
status = MHD_HTTP_UNAUTHORIZED;

auth_reason = ret == 2 ? GMP_SERVICE_DOWN
: (ret == -1 ? LOGIN_ERROR : LOGIN_FAILED);
switch (ret)
{
case 1: /* could not connect to manager */
case 3: /* timeout */
status = MHD_HTTP_SERVICE_UNAVAILABLE;
auth_reason = GMP_SERVICE_DOWN;
break;
case 2: /* authentication failure */
status = MHD_HTTP_UNAUTHORIZED;
auth_reason = LOGIN_FAILED;
break;
default: /* unspecified error */
status = MHD_HTTP_INTERNAL_SERVER_ERROR;
auth_reason = LOGIN_ERROR;
break;
}

g_warning ("Authentication failure for '%s' from %s", login ?: "",
client_address);
g_warning ("Authentication failure for '%s' from %s. "
"Status was %d.",
login ?: "", client_address, ret);
return handler_send_reauthentication (con, status, auth_reason);
}
else
Expand Down Expand Up @@ -17370,7 +17388,8 @@ login (http_connection_t *con, params_t *params,
* @param[out] connection Connection to Manager on success.
* @param[out] response_data Extra data return for the HTTP response.
*
* @return 0 success, -1 failed to connect, -2 authentication failed.
* @return 0 success, 1 if manager closed connection, 2 if auth failed,
* 3 on timeout, 4 failed to connect, -1 on error
*/
int
manager_connect (credentials_t *credentials, gvm_connection_t *connection,
Expand All @@ -17380,20 +17399,21 @@ manager_connect (credentials_t *credentials, gvm_connection_t *connection,

if (gvm_connection_open (connection, manager_address, manager_port))
{
cmd_response_data_set_status_code (response_data,
MHD_HTTP_SERVICE_UNAVAILABLE);
return -1;
return 4;
}

user_t *user = credentials_get_user (credentials);

auth_opts = gmp_authenticate_info_opts_defaults;
auth_opts.username = user_get_username (user);
auth_opts.password = user_get_password (user);
if (gmp_authenticate_info_ext_c (connection, auth_opts))

int ret = gmp_authenticate_info_ext_c (connection, auth_opts);

if (ret)
{
g_debug ("authenticate failed!\n");
gvm_connection_close (connection);
return -2;
return ret;
}

#ifdef DEBUG
Expand Down
3 changes: 3 additions & 0 deletions gsad/src/gsad_http.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,9 @@ handler_send_reauthentication (http_connection_t *connection,
case LOGOUT:
msg = "Successfully logged out.";
break;
case UNKOWN_ERROR:
msg = "Unknown error.";
break;
default:
msg = "";
}
Expand Down
1 change: 1 addition & 0 deletions gsad/src/gsad_http.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ enum authentication_reason
SESSION_EXPIRED,
BAD_MISSING_COOKIE,
BAD_MISSING_TOKEN,
UNKOWN_ERROR,
};

typedef enum authentication_reason authentication_reason_t;
Expand Down
41 changes: 33 additions & 8 deletions gsad/src/gsad_http_handler.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -598,28 +598,53 @@ handle_system_report (http_connection_t *connection, const char *method,
/* Connect to manager */
switch (manager_connect (credentials, &con, response_data))
{
case 0:
case 0: /* success */
res = get_system_report_gmp (&con, credentials,
&url[0] + strlen ("/system_report/"), params,
response_data);
gvm_connection_close (&con);
break;
case -1:
return handler_send_reauthentication (
connection, MHD_HTTP_SERVICE_UNAVAILABLE, GMP_SERVICE_DOWN);
case 1: /* manager closed connection */
cmd_response_data_set_status_code (response_data,
MHD_HTTP_INTERNAL_SERVER_ERROR);
res = gsad_message (credentials, "Internal error", __FUNCTION__, __LINE__,
"An internal error occurred. "
"Diagnostics: Failure to connect to manager daemon. "
"Manager daemon doesn't respond.",
response_data);
break;
case 2: /* authentication failed */
cmd_response_data_free (response_data);
credentials_free (credentials);
return handler_send_reauthentication (connection, MHD_HTTP_UNAUTHORIZED,
LOGIN_FAILED);

break;
case -2:
case 3: /* timeout */
cmd_response_data_set_status_code (response_data,
MHD_HTTP_INTERNAL_SERVER_ERROR);
res = gsad_message (credentials, "Internal error", __FUNCTION__, __LINE__,
"An internal error occurred. "
"Diagnostics: Failure to connect to manager daemon. "
"Timeout while waiting for manager response.",
response_data);
break;
case 4: /* failed to connect to manager */
cmd_response_data_set_status_code (response_data,
MHD_HTTP_INTERNAL_SERVER_ERROR);
res = gsad_message (credentials, "Internal error", __FUNCTION__, __LINE__,
"An internal error occurred. "
"Diagnostics: Could not authenticate to manager "
"daemon.",
"Diagnostics: Failure to connect to manager daemon. "
"Could not open a connection.",
response_data);
break;
default:
cmd_response_data_set_status_code (response_data,
MHD_HTTP_INTERNAL_SERVER_ERROR);
res = gsad_message (credentials, "Internal error", __FUNCTION__, __LINE__,
"An internal error occurred. "
"Diagnostics: Failure to connect to manager daemon.",
"Diagnostics: Failure to connect to manager daemon. "
"Unknown error.",
response_data);
break;
}
Expand Down

0 comments on commit 58abfa3

Please sign in to comment.