On Wed, Jul 21, 2004 at 07:30:51PM -0400, Garrett Kajmowicz wrote:

OK - this patch backs out a few trivial changes which were made about 2 patches ago due to code redundancy and are no longer needed.

This patch detects self-signed certificates and gives the user the option to permanently trust them, autosaving to the .dillo/certs directory. Please give this a whirl, and then commit it. Let me know when done so I can while my current devel tree clean and get a new copy - this way I don't have to manually correct all of the spelling errors with each patch :-)

Oh!??? This patch may be not what you intended, as I found many weird things I prefer to comment it, and wait for the version 2 of it... (my comments start with "##") diff -rpu dillo-cvs/dillo/dpi/dpiutil.c dillo-dev/dillo/dpi/dpiutil.c --- dillo-cvs/dillo/dpi/dpiutil.c 2004-07-16 22:19:00.000000000 +0000 +++ dillo-dev/dillo/dpi/dpiutil.c 2004-07-21 15:06:53.000000000 +0000 @@ -11,6 +11,7 @@ */ #include "dpiutil.h" +#include <unistd.h> #include <stdio.h> #include <string.h> #include <glib.h> @@ -127,7 +128,7 @@ gint send_stream_4(FILE *in_stream, gcha return 0; } -gint send_stream_mode(FILE *in_stream, gchar *url, int mode) +gint send_stream_mode(FILE *in_stream, gchar *url, gint mode) { switch (mode){ case SEND_STREAM_MODE_CHAR: @@ -143,3 +144,43 @@ gint send_stream_mode(FILE *in_stream, g return -1; } +/*gint dpi_message_response_number(){ + gint response_number; + gint rd_len; + gchar buf[4096]; + gchar * response = 0; +*/ + /*Read in user responce*/ +/* rd_len = read(STDIN_FILENO, buf, 4096); + response = Get_attr_value(buf, rd_len, "msg" ); + + if(response == NULL){ + return -1; + } + + sscanf(response, "%d", &response_number); + g_free(response); + response = NULL; + return response_number; + +} +*/ ## Why do we need these functions back again commented out? + +char * get_environment_variable_value(char ** envp, char * var){ + int i; + int var_length; + + var_length = strlen(var); + + for(i=0;i<10000;i++){ + if(envp[i] == NULL){ + return NULL; + } + if(strncmp(var, envp[i], var_length) == 0){ + /*We have found wanted variable*/ + return (envp[i] + var_length + 1); + } + } + + +} ## This can be avoided with g_get_home_dir(). diff -rpu dillo-cvs/dillo/dpi/dpiutil.h dillo-dev/dillo/dpi/dpiutil.h --- dillo-cvs/dillo/dpi/dpiutil.h 2004-07-13 21:42:22.000000000 +0000 +++ dillo-dev/dillo/dpi/dpiutil.h 2004-07-21 15:05:54.000000000 +0000 @@ -65,4 +65,17 @@ gint send_stream_4(FILE *in_stream, gcha #define SEND_STREAM_MODE_COARSE 3 #define SEND_STREAM_MODE_COARSE_FAST 4 -gint send_stream_mode(FILE *in_stream, gchar *url, int mode); +gint send_stream_mode(FILE *in_stream, gchar *url, gint mode); + + +/* + * Get dialog response number + */ +//gint dpi_message_response_number(); + +/* + * Process environmental variables, returning a specific one + * or NULL if it cannot be found + */ + +char * get_environment_variable_value(char ** envp, char * var); ## As with the commented functions, this is not necessary. diff -rpu dillo-cvs/dillo/dpi/https.c dillo-dev/dillo/dpi/https.c --- dillo-cvs/dillo/dpi/https.c 2004-07-21 14:49:16.000000000 +0000 +++ dillo-dev/dillo/dpi/https.c 2004-07-21 23:20:50.000000000 +0000 @@ -49,10 +49,10 @@ #include <sys/wait.h> #include <errno.h> #include <sys/time.h> +#include <sys/stat.h> #include <glib.h> #include "dpiutil.h" - #ifdef ENABLE_SSL #include <openssl/ssl.h> @@ -60,13 +60,17 @@ static int get_network_connection(gchar * url); static int handle_certificate_problem(SSL * ssl_connection); +static int save_certificate_home(X509 * cert); #endif +int main(int argc, char ** argv, char ** envp); ## The whole environment can be avoided by using g_get_home_dir(). ## (If the need arises for env variables, getenv() can be used) /*---------------------------------------------------------------------------*/ +char ** env; /*Points inside envp so no freeing*/ ## Not needed. +gchar * root_url; /*Holds the URL we are connecting to*/ #ifdef ENABLE_SSL @@ -99,7 +103,6 @@ static void yes_ssl_support(void) { SSL_CTX * ssl_context = NULL; SSL * ssl_connection = NULL; - X509 * remote_cert = NULL; FILE *F_stdin; gchar *dpip_tag = NULL, *cmd = NULL, *url = NULL, *http_query = NULL; @@ -125,11 +128,18 @@ static void yes_ssl_support(void) /*Set directory to load certificates from*/ /*FIXME - provide for sysconfdir variables and such*/ - SSL_CTX_load_verify_locations(ssl_context, NULL, "/etc/ssl/certs/" ); + if(SSL_CTX_load_verify_locations(ssl_context, NULL, "/etc/ssl/certs/" ) == 0){ + g_printerr("Error opening system x509 certificate location\n"); + } + snprintf(buf, 4095, "%s/.dillo/certs/", get_environment_variable_value(env, "HOME") ); + + if(SSL_CTX_load_verify_locations(ssl_context, NULL, buf )==0){ + g_printerr("Error opening user x509 certificate location\n"); + } ssl_connection = SSL_new(ssl_context); if (ssl_connection == NULL){ - printf("Error creating SSL connection\n"); + g_printerr("Error creating SSL connection\n"); } /*Need to do the following if we want to deal with all possible ciphers*/ @@ -168,6 +178,7 @@ static void yes_ssl_support(void) if (network_socket<0){ /*Cleanup*/ g_printerr("Network socket create error\n"); + g_free(root_url); g_free(http_query); g_free(url); g_free(cmd); @@ -181,6 +192,7 @@ static void yes_ssl_support(void) /* Configure SSL to use network file descriptor */ if (SSL_set_fd(ssl_connection, network_socket) == 0){ g_printerr("Error connecting network socket to SSL\n"); + g_free(root_url); g_free(http_query); g_free(url); g_free(cmd); @@ -195,6 +207,7 @@ static void yes_ssl_support(void) /*Actually do SSL connection handshake*/ if (SSL_connect(ssl_connection) != 1){ g_printerr("SSL_connect failed\n"); + g_free(root_url); g_free(http_query); g_free(url); g_free(cmd); @@ -206,53 +219,23 @@ static void yes_ssl_support(void) return; } - /*Now check to see what kind of certificate we have*/ - /*Do we have a certificate at all?*/ - remote_cert = SSL_get_peer_certificate(ssl_connection); - if (remote_cert == NULL){ - if (handle_certificate_problem(ssl_connection) < 0){ - g_printerr("Certificate not found and abort\n"); - g_free(http_query); - g_free(url); - g_free(cmd); - g_free(dpip_tag); - close(network_socket); - fclose(F_stdin); - SSL_free(ssl_connection); - SSL_CTX_free(ssl_context); - return; - } - g_printerr("Certificate not found but continuing\n"); - } else { - /* We have no use for the certificate, so call free. This is - * important if we go to convert this to a server, or if we - * use the app. on a machine without protected memory. - */ - X509_free(remote_cert); - remote_cert = NULL; - /*Check to see the verification on the certificate*/ - if (SSL_get_verify_result(ssl_connection) != X509_V_OK){ - if (handle_certificate_problem(ssl_connection) < 0){ - g_printerr("Certificate not verified and abort\n"); - g_free(http_query); - g_free(url); - g_free(cmd); - g_free(dpip_tag); - close(network_socket); - fclose(F_stdin); - SSL_free(ssl_connection); - SSL_CTX_free(ssl_context); - return; - } - g_printerr("Certificate not verified but continuing\n"); - } + /*Use handle error function to decide what to do*/ + if (handle_certificate_problem(ssl_connection) < 0){ + g_printerr("Certificate not found and abort\n"); + g_free(root_url); + g_free(http_query); + g_free(url); + g_free(cmd); + g_free(dpip_tag); + close(network_socket); + fclose(F_stdin); + SSL_free(ssl_connection); + SSL_CTX_free(ssl_context); + return; } - g_printerr("Do http stuff here\n"); - /*Send query we want*/ SSL_write(ssl_connection, http_query, strlen(http_query)); - g_printerr("Sending: %s\n", http_query); /*Analyse response from server*/ @@ -268,6 +251,7 @@ static void yes_ssl_support(void) /*Begin cleanup of all resources used*/ g_printerr("Resource cleanup\n"); + g_free(root_url); g_free(http_query); g_free(url); g_free(cmd); @@ -318,6 +302,7 @@ static int get_network_connection(gchar url_look_up = url + url_offset; } + root_url = g_strdup(url_look_up); hp=gethostbyname(url_look_up); /*url_look_uip no longer needed, so free if neccessary*/ @@ -355,59 +340,143 @@ static int get_network_connection(gchar static int handle_certificate_problem(SSL * ssl_connection) { gint response_number; - long ssl_st; - int retval = -1; + int retval; + char buf[4096]; ## retval was set to -1 for security. The safest way for doing checks ## is to assume everything is forbidden unless explicitly allowed. ## That's way the functions were rearranged this way. X509 * remote_cert; remote_cert = SSL_get_peer_certificate(ssl_connection); - if (remote_cert == NULL){ + if(remote_cert == NULL){ /*Inform user that remote system cannot be trusted*/ printf("<dpi cmd='dialog' msg='%s' alt1='%s' alt2='%s'>", - "The remote system is NOT presenting a certificate.\n" - "This site CAN NOT be trusted. Sending data is NOT SAFE.\n" - "What do I do?", + "The remote system is not presenting a certificate and cannot be trused", "Continue", "Cancel" ); fflush(stdout); ## Reversing the new message's text? ## This's waht made me think this patch wasn't generated as intended. - /*Read in user response*/ + /*Read in user response*/ response_number = dialog_get_answer_number(); - - /* Abort on anything but "Continue" */ - if (response_number == 1) - retval = 0; - - } else { - X509_free(remote_cert); - /*Figure out why (and if) the remote system can't be trusted*/ - ssl_st = SSL_get_verify_result(ssl_connection); - switch (ssl_st) { - case X509_V_OK: /*Everything is Kosher*/ - retval = 0; - break; - default: /*Need to add more options later*/ - printf("<dpi cmd='dialog' msg='%s' alt1='%s' alt2='%s'>", - "The remote system presented a certificate that" - " can NOT be verified.\n" - "This site CAN NOT be trusted. Sending data is NOT SAFE.\n" - "What do I do?", - "Continue", "Cancel" - ); - fflush(stdout); - - response_number = dialog_get_answer_number(); - - /* Abort on anything but "Continue" */ - if (response_number == 1) - retval = 0; - break; + + switch(response_number){ + case 1: /*Continue*/ + return 0; + case 2: /*Cancel*/ + return -1; + default: /*Safety - abort*/ + return -1; + } ## This switch statements were removed so the function has one clear ## flow of execution with a single exit, and not multiple exits ## (this is easier to understand, and less prone to errors) + }else{ + /*Figure out if (and why) the remote system can't be trusted*/ + retval = SSL_get_verify_result(ssl_connection); + switch (retval){ + case X509_V_OK: /*Everything is Kosher*/ + X509_free(remote_cert); + remote_cert = 0; + return 0; + case X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT: + /*Either self signed and untrusted*/ + /*Extract CN from certificate name information*/ + strncpy(buf, (strstr(remote_cert->name, "/CN=") + 4), + strstr(strstr(remote_cert->name, "/CN=") + 4, "/") - + strstr(remote_cert->name, "/CN=")- 4); + /*Add terminating NULL*/ + buf[strstr(strstr(remote_cert->name, "/CN=") + 4, "/") - + strstr(remote_cert->name, "/CN=")- 3] = 0; + printf("<dpi cmd='dialog' msg='%s%s' alt1='%s' alt2='%s' alt3='%s'>", + "The remote certificate is self-signed and untrusted:\n For address:", + buf, "Continue", "Cancel", "Trust Certificate" + ); + fflush(stdout); + response_number = dialog_get_answer_number(); + switch(response_number){ + case 1: + X509_free(remote_cert); + remote_cert = 0; + return 0; + case 2: + X509_free(remote_cert); + remote_cert = 0; + return -1; + case 3: + /*Save certificate to a file here and recheck chain*/ + /*Potential security problems because we are writing filesystem*/ + save_certificate_home(remote_cert); + X509_free(remote_cert); + remote_cert = 0; + return 1; + default: + X509_free(remote_cert); + remote_cert = 0; + return -1; + } + default: /*Need to add more options later*/ + printf("<dpi cmd='dialog' msg='%s%i' alt1='%s' alt2='%s'>", + "The remote certificate cannot be verified", + retval, "Continue", "Cancel" + ); + fflush(stdout); + response_number = dialog_get_answer_number(); + X509_free(remote_cert); + remote_cert = 0; + switch(response_number){ + case 1: + return 0; + case 2: + return -1; + default: + return -1; + } ## Ditto as above. } } return retval; } +int save_certificate_home(X509 * cert){ + char buf[4096]; + + FILE * fp = NULL; + unsigned int i = 0; + + /*Attempt to create .dillo/certs blindly - check later*/ + snprintf(buf,4096,"%s/.dillo/", + get_environment_variable_value(env, "HOME") + ); + mkdir(buf, 01777); + snprintf(buf,4096,"%s/.dillo/certs/", + get_environment_variable_value(env, "HOME") + ); + mkdir(buf, 01777); + + do{ + snprintf(buf,4096,"%s/.dillo/certs/%x.%u\0", + get_environment_variable_value(env, "HOME"), + X509_subject_name_hash(cert), i + ); ## All get_environment_variable_value() can be avoided + + fp=fopen(buf, "r"); + if(fp == NULL){ + /*File name doesn't exist so we can use it safely*/ + fp=fopen(buf, "w"); + if(fp == NULL){ + g_printerr("Unable to open cert save file in home dir\n"); + return 1; + }else{ + PEM_write_X509(fp, cert); + fclose(fp); + g_printerr("Wrote certificate\n"); + return 0; + } + }else{ + fclose(fp); + } + i++; + } while( i < 1024 ); + + return 1; +} + + ## Please use "if (" and "} else {" #else @@ -480,9 +549,9 @@ static void no_ssl_support(void) /*---------------------------------------------------------------------------*/ -int main(void) +int main(int argc, char ** argv, char ** envp) { - + env=envp; #ifdef ENABLE_SSL yes_ssl_support(); #else -- Cheers Jorge.- PS: Do you prefer me to make the changes for you to sync with CVS? :) PS2: Patches of this size can be sent directly to me.