Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix handling error response during gmp authentication in gsad #1700

Merged
merged 7 commits into from
Oct 18, 2019
Merged

Fix handling error response during gmp authentication in gsad #1700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e00812b
Fix sending error responses
bjoernricks Oct 17, 2019
e9a70f9
Improve handling error code for user authentication
bjoernricks Oct 17, 2019
3f2c4e6
Add changelog entry
bjoernricks Oct 17, 2019
82fd519
Don't request re-authentication on unknown errors
bjoernricks Oct 17, 2019
4463348
Update handle_system_report to handle more error cases
bjoernricks Oct 17, 2019
e70df9f
Return timeout error code from authenticate_gmp
bjoernricks Oct 17, 2019
cc6c468
Fix typo
bjoernricks Oct 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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