Skip to content

Commit

Permalink
Improve SAML authentication
Browse files Browse the repository at this point in the history 
In the initial implementation there were still some issues that needed
to be fixed.
One main improvement is that the http server thread does not run the
tunnel thread. Instead the http server is first shut down and then
the tunnel is started as usual.
  • Loading branch information
@Rainer-Keller
Rainer-Keller committed Sep 15, 2024
1 parent 994fabe commit d2b21c6
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 62 deletions.
6 changes: 3 additions & 3 deletions src/config.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ const struct vpn_config invalid_cfg = {
.password = {'\0'},
.password_set = 0,
.cookie = NULL,
.saml_port = 0,
.saml_session_id = {'\0'},
.otp = {'\0'},
.otp_prompt = NULL,
.otp_delay = -1,
Expand Down Expand Up @@ -418,9 +420,7 @@ int load_config(struct vpn_config *cfg, const char *filename)
if (strncmp(cfg->user_cert, "pkcs11:", 7) == 0)
cfg->use_engine = 1;
} else if (strcmp(key, "saml-login") == 0) {
free(cfg->saml_port);
cfg->saml_port = atol(val);
continue;
cfg->saml_port = atoi(val);
} else if (strcmp(key, "user-key") == 0) {
free(cfg->user_key);
cfg->user_key = strdup(val);
Expand Down
5 changes: 3 additions & 2 deletions src/config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ struct x509_digest {
* We believe we are on the safe side using this value.
*/
#define MAX_DOMAIN_LENGTH 256
#define MAX_SAML_SESSION_ID_LENGTH 1024

struct vpn_config {
char gateway_host[GATEWAY_HOST_SIZE + 1];
Expand All @@ -91,8 +92,8 @@ struct vpn_config {
int password_set;
char otp[OTP_SIZE + 1];
char *cookie;
int saml_port;
char saml_session_id[1024];
int saml_port;
char saml_session_id[MAX_SAML_SESSION_ID_LENGTH];
char *otp_prompt;
unsigned int otp_delay;
int no_ftm_push;
Expand Down
15 changes: 8 additions & 7 deletions src/http.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -634,12 +634,15 @@ int saml_login(struct tunnel *tunnel)
int ret;
ssl_connect(tunnel);

char uri[1024];
snprintf(uri, sizeof(uri), "/remote/saml/auth_id?id=%s", tunnel->config->saml_session_id);
char uri_pattern[] = "/remote/saml/auth_id?id=%s";
int required_size = snprintf(NULL, 0, uri_pattern, tunnel->config->saml_session_id) + 1;
char *uri = alloca(required_size);
snprintf(uri, required_size, uri_pattern, tunnel->config->saml_session_id);

char *response;
uint32_t response_size = 0;
ret = http_request(tunnel, "GET", uri, "", &response, &response_size);
if(ret != 1 || response_size <= 15) return ret;
if(ret != 1 || response_size <= 15) return -1;
if (memcmp(response, "HTTP/1.1 200 OK", 15) != 0){
log_error("SAML login failed: %s\n", response);
return ret;
Expand All @@ -648,11 +651,9 @@ int saml_login(struct tunnel *tunnel)
if (ret == ERR_HTTP_NO_COOKIE){
log_error("SAML login failed: no cookie\n");
return ret;
}

// free(response);

}

free(response);

return ret;
}
Expand Down
1 change: 0 additions & 1 deletion src/http.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#define OPENFORTIVPN_HTTP_H

#include "tunnel.h"
#include "config.h"

#include <stdint.h>

Expand Down
58 changes: 17 additions & 41 deletions src/http_server.c
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
#include <sys/socket.h>
#include <netinet/in.h>
#include <string.h>
#include <unistd.h>
#include "config.h"
#include "log.h"
#include <pthread.h>
#include "tunnel.h"
#include <ctype.h>
#include <signal.h>
#include <netinet/in.h>
#include <netinet/tcp.h>


#include "config.h"
#include "log.h"
#include "tunnel.h"

int process_request(int new_socket, char *id) {
log_info("Processing request\n");
Expand All @@ -32,23 +28,20 @@ int process_request(int new_socket, char *id) {
ssize_t write_result = write(new_socket, response, buffer_size);
(void)write_result;

// dup2(new_socket, STDOUT_FILENO);
// dup2(new_socket, STDERR_FILENO);



char request[1024];
ssize_t read_result = read(new_socket, request, sizeof(request));

// Check for '=id' in the response
if (read_result < 0 || strlen(request) < 10 || strncmp(request, "GET /?id=", 9) != 0) {
// If the recevied request from the server is larger than the buffer, the result will not be null-terminated.
// Causing strlen to behave wrong.
if (read_result < 0 || read_result == sizeof(request) || strlen(request) < 10 || strncmp(request, "GET /?id=", 9) != 0) {
log_error("Bad request\n");
return -1;
}

// Extract the id
const int id_start = 9;
char *id_end = memchr(&request[id_start], ' ', 1000);
char *id_end = memchr(&request[id_start], ' ', sizeof(request) - id_start);

if (id_end == NULL) {
log_error("Bad request format\n");
Expand All @@ -57,7 +50,7 @@ int process_request(int new_socket, char *id) {

int id_length = id_end - &request[id_start];

if(id_length == 0 || id_length > 1000) {
if(id_length == 0 || id_length > MAX_SAML_SESSION_ID_LENGTH) {
log_error("Bad request id\n");
return -1;
}
Expand All @@ -69,7 +62,7 @@ int process_request(int new_socket, char *id) {
log_error("Invalid id format\n");
return -1;
}
// close(new_socket);
close(new_socket);
log_info("Extracted id: %s\n", id);
return 0;
}
Expand All @@ -94,6 +87,7 @@ void* start_http_server(void *void_config) {

// Forcefully attaching socket to the port
if (setsockopt(server_fd, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt))) {
close(server_fd);
log_error("Failed to set socket options\n");
return NULL;
}
Expand All @@ -104,52 +98,34 @@ void* start_http_server(void *void_config) {

// Forcefully attaching socket to the port
if (bind(server_fd, (struct sockaddr *)&address, sizeof(address)) < 0) {
close(server_fd);
log_error("Failed to bind socket to port %d \n",saml_port);
return NULL;
}

if (listen(server_fd, 3) < 0) {
close(server_fd);
log_error("Failed to listen on socket\n");
return NULL;
}
log_info("Listening for saml login on port: %d\n", saml_port);
int running = 1;

pthread_t vpn_thread = 0;


while(running) {
while(1) {
if ((new_socket = accept(server_fd, (struct sockaddr *)&address, (socklen_t*)&addrlen)) < 0) {
log_error("Failed to accept connection\n");
continue;
}

int result = process_request(new_socket, config->saml_session_id);
close(new_socket);
if(result != 0) {
log_error("Failed to process request\n");
continue;
}


// Kill previous thread if it exists
if (vpn_thread != 0) {
log_error("Stop existing tunnel\n");
pthread_kill(vpn_thread,SIGTERM);
pthread_join(vpn_thread, NULL);
}

int thread_create_result = pthread_create(&vpn_thread, NULL, run_tunnel_wrapper, (void *)config);
if (thread_create_result != 0) {
log_error("Failed to create VPN thread\n");
continue;
}


// pthread_detach(vpn_thread);

result = 0;
break;
}

close(server_fd);
return NULL;
}

20 changes: 12 additions & 8 deletions src/main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ int main(int argc, char *argv[])
.password_set = 0,
.cookie = NULL,
.saml_port = 0,
.saml_session_id = NULL,
.saml_session_id = {'\0'},
.otp = {'\0'},
.otp_prompt = NULL,
.otp_delay = 0,
Expand Down Expand Up @@ -290,7 +290,7 @@ int main(int argc, char *argv[])
{"password", required_argument, NULL, 'p'},
{"cookie", required_argument, NULL, 0},
{"cookie-on-stdin", no_argument, NULL, 0},
{"saml-login", optional_argument, NULL, 0},
{"saml-login", optional_argument, NULL, 0},
{"otp", required_argument, NULL, 'o'},
{"otp-prompt", required_argument, NULL, 0},
{"otp-delay", required_argument, NULL, 0},
Expand Down Expand Up @@ -615,7 +615,7 @@ int main(int argc, char *argv[])
if(optarg != NULL){
port = strtol(optarg, NULL, 0);
}
if (port < 0 || port > UINT_MAX) {
if (port < 0 || port > 65535) {
log_warn("Invalid saml listen port: %s! Default port is 8020 \n", optarg);
break;
}
Expand Down Expand Up @@ -732,7 +732,7 @@ int main(int argc, char *argv[])
goto user_error;
}
// If username but no password given, interactively ask user
if (!cfg.password_set && cfg.username[0] != '\0' && !cfg.cookie) {
if (!cfg.password_set && cfg.username[0] != '\0' && !cfg.cookie && !cfg.saml_port) {
char hint[USERNAME_SIZE + 1 + REALM_SIZE + 1 + GATEWAY_HOST_SIZE + 10];

sprintf(hint, "%s_%s_%s_password",
Expand Down Expand Up @@ -761,12 +761,16 @@ int main(int argc, char *argv[])

if(pthread_create(&server_thread, NULL, start_http_server, &cfg) != 0){
log_error("Failed to create saml login server thread\n");
// ret = EXIT_FAILURE;
ret = EXIT_FAILURE;
goto exit;
}
log_debug("Running http server on port %d\n", cfg.saml_port);
pthread_join(server_thread, NULL);

if (strlen(cfg.saml_session_id) == 0) {
log_error("Failed to receive SAML session id\n");
goto exit;
}
// log_debug("Running http server on port %d\n", cfg.saml_port);
while(get_sig_received() == 0);
goto exit;
}

do {
Expand Down

0 comments on commit d2b21c6

Please sign in to comment.