4.0.11 - getshort fix - more tls autodetect

.gitignore

@@ -5,9 +5,9 @@ CVS
 .metadata
 .actionScriptProperties
 .#*
-world/net/place/_*.c
+src/autoconf/autom4te.cache
-world/net/include/place.i
 *~
+*.o
 *.swf
 *.swp
 *.rej

CHANGELOG

@@ -1,3 +1,12 @@
+2008-07-27 (lynX) (4.0.10)
+  + added ERR_TLS_NOT_DETECTED code to sys/tls.h
+  + added suitable message to tls_error()
+  - configure.in: use sysmalloc by default
+
+2008-07-20 (lynX) (4.0.9)
+  - TLS autodetect now comes with timer
+  - safer way to shutdown SSL
+
 2008-05-28 (fippo/lynX) (4.0.8)
   + new efuns:
 	string strftime(string, int default: F_TIME);

TODO

@@ -8,6 +8,13 @@ BUGS
 - Because of some funny bug the driver will probably not bind properly
   to _basic_host_IP as it also needs _basic_host_name and _basic_host_domain
   to be provided. Hopefully this shortcoming will soon be history.
+- what's the use of a traceback on stdout if the actual error goes to stderr?
+    Error loading place: *Error in loading object: 'place/testotest'.
+    Warning: place "/place/testotest" did not return ME on load()
+    2009.04.21 12:35:19 ERROR caught. Backtrace:
+    '          parse' in '      net/irc/user.c' (' net/irc/user#User') line 32
+    [...]
+    '          CATCH' in                        ('drivers/ldmud/library/library')
 
 WINDOWS SUPPORT
 + with the new __NO_SRV__ detection psyclpc compiles under cygwin but it
@@ -46,6 +53,8 @@ MISC IDEAS
 + fippo suggests to look into http://www.hpl.hp.com/personal/Hans_Boehm/gc/leak.html
 + DMSG_FLUSH for incremental debug messages  :)
 - for pike compatibility allow string[-1] just like string[<1]
++ would be nice if sprintf %O would indent data structure according to depth.
+  currently outputting a parsed xml node is quite confusing
 
 ENCRYPTION (see http://about.psyc.eu/Talk:Encryption)
 ? link libpurple to this for protocol supports and OTR hahahahaha

mudlib/sys/tls.h

@@ -84,4 +84,7 @@
 #define TLS_HASH_MD5       (6)
 #define TLS_HASH_RIPEMD160 (7)
 
+/* autodetect feature: when no TLS is found on socket, this code is returned */
+#define	ERR_TLS_NOT_DETECTED	-31337
+
 #endif /* LPC_TLS_H */

src/autoconf/configure.in

@@ -2711,7 +2711,7 @@ dnl
 
 AC_UPDATE_VAR(with_malloc)
 if test "x$with_malloc" = "x" || test "x$with_malloc" = "xdefault"; then
-    with_malloc="slaballoc"
+    with_malloc="sysmalloc"
     AC_TEXT_VAL_FROM_WITH(malloc)
 fi
 

src/comm.c

@@ -3043,9 +3043,14 @@ get_message (char *buff, size_t *len)
 #ifdef USE_TLS
             /* Special case for setting up a TLS connection: don't
              * attempt IO if the connection is still being set up.
+             * fippo suggests we should check here if the socket
+             * is readable, otherwise there is nothing to handshake.
+             * Currently we peek each second, we can optimize that.
              */
             if (ip->tls_status == TLS_HANDSHAKING)
             {
+/*		printf("tls: socket readable %d, writable %d\n", FD_ISSET(ip->socket, &readfds), 
+								 FD_ISSET(ip->socket, &writefds)); */
                 tls_continue_handshake(ip);
                 continue;
             }
@@ -3070,7 +3075,7 @@ get_message (char *buff, size_t *len)
 
 #ifdef USE_TLS
                 if (ip->tls_status != TLS_INACTIVE)
-                    l = tls_read(ip, ip->text + ip->text_end, (size_t)l);
+                    l = tls_read(ip, ip->text + ip->text_end, l);
                 else
 #endif
                     l = socket_read(ip->socket, ip->text + ip->text_end, (size_t)l);
@@ -3157,7 +3162,10 @@ get_message (char *buff, size_t *len)
 		 */
 		if (ip->is_binary) {
 		    memcpy(buff, ip->text, l);
-		    *len = l;
+		    *len = (size_t) l;
+ /* webdav attack makes these lines crash..
+  * still collecting evidence..
+  */
                     FD_CLR(ip->socket, &readfds);
 		    command_giver = ip->ob;
 		    return MY_TRUE;
@@ -3950,7 +3958,7 @@ new_player ( object_t *ob, SOCKET_T new_socket
     new_interactive->tls_session = NULL;
     new_interactive->tls_cb = NULL;
 # ifdef USE_EXPERIMENTAL
-    new_interactive->tls_autodetect = MY_TRUE;
+    new_interactive->tls_autodetect = ob == NULL? 9 : 0; /* give tls 9 seconds to start */
 # endif
     new_interactive->tls_want_peer_cert = MY_FALSE;
 #endif
@@ -5521,6 +5529,44 @@ telnet_neg (interactive_t *ip)
  * apparently at the time of compilation, line 5472 was the from == comparison
  * below. strange or funny, we never touched this part of code. so it must be
  * a quite old bug. anyway...  FIXME.
+
+ * same crash again, 2008-12-10. this time i look at it more closely.
+ *
+#0  telnet_neg (ip=0xa5a01a8) at comm.c:5533
+#1  0x080632b5 in get_message (
+    buff=0xbf96e3b2 "\002\002PÃG\vC\002¨É\226¿\001\001\200", len=0xbf976488)
+    at comm.c:3180
+#2  0x08053026 in backend () at backend.c:629
+#3  0x080a43a4 in main (argc=1936024636, argv=0x70697263) at main.c:681
+ *
+(gdb) print from
+$1 = 0x2acc67ca <Address 0x2acc67ca out of bounds>
+(gdb) print end
+$2 = 0x729e22c5 <Address 0x729e22c5 out of bounds>
+(gdb) print first
+No symbol "first" in current context.
+(gdb) print ip
+$5 = (interactive_t *) 0xa5a01a8
+(gdb) print ip->gobble_char
+$6 = 101 'e'
+(gdb) print *from
+Cannot access memory at address 0x2acc67ca
+(gdb) print ip->text
+$7 = "da hat sich verletzt.</description>\n<category>Sport</category>\n<pubDate>Tue, 09 Dec 2008 11:01:33 +0100</pubDate>\n<guid>http://www.spiegel.de/sport/fussball/0,1518,595301,00.html</guid>\n<content:encod"...
+(gdb) print ip->tn_end
+$8 = 544367982
+(gdb) print ip->text_end
+$9 = 1749295209
+(gdb) print &ip->text
+$10 = (char (*)[32770]) 0xa5a025c
+(gdb) print &(ip->text)[ip->tn_end]
+$12 = 0x2acc67ca <Address 0x2acc67ca out of bounds>
+
+ * alright. so here we have a case where tn_end and text_end have
+ * completely absurd values. how did they get into there. we could add
+ * some crazy paranoid checks here, but that's bad for performance and
+ * bad anyway. disabling telnet in net/http/fetch will be helpful, but
+ * i'd prefer to understand how we got into this problem.
  */
             if (*from == ip->gobble_char
                 || (*from == '\0' && ip->gobble_char == '\n')
@@ -9132,9 +9178,9 @@ f_enable_binary (svalue_t *sp)
         return sp; /* flow control hint */
     }
 
-    rc = (ip->is_binary != 0);
+    rc = (ip->is_binary != MY_FALSE);
     //if (privilege_violation4(STR_ENABLE_TELNET, sp->u.ob, NULL, 1, sp))
-    ip->is_binary = 1;
+    ip->is_binary = MY_TRUE;
 
     free_svalue(sp);
 
@@ -9391,7 +9437,7 @@ f_net_connect (svalue_t *sp)
          */
         outconn[n].socket = d;
         outconn[n].target = target;
-        outconn[n].curr_obj = ref_object(current_object, "net_conect");
+        outconn[n].curr_obj = ref_object(current_object, "net_connect");
 
         if (errno == EINPROGRESS)
         {

src/comm.h

@@ -288,7 +288,7 @@ struct interactive_s {
 #     define TLS_BROKEN       3  /* Error has occurred */
     CBool		   tls_want_peer_cert;
 # ifdef USE_EXPERIMENTAL
-    CBool		   tls_autodetect;
+    int			   tls_autodetect;
 # endif
     callback_t            *tls_cb;
 #endif

src/pkg-tls.c

@@ -719,39 +719,56 @@ tls_continue_handshake (interactive_t *ip)
  */
 
 {
-    int ret;
+    int ret = 1;
 
+#ifdef USE_PARANOIA
     if (ip->tls_status != TLS_HANDSHAKING)
         return 1;
-
+#endif
-    ret = 1;
-
 #ifdef HAS_OPENSSL
 # ifdef USE_EXPERIMENTAL
     /* detect non-tls data on a tls connection
      * heuristic taken from jabberds jadc2s
      */
-    if (ip->tls_autodetect == MY_TRUE) 
+    if (ip->tls_autodetect) {
-    {
+	char peek_buf = 0x44;
-	char peek_buf;
+
-	size_t peeked;
+	if (recv(ip->socket, &peek_buf, 1, MSG_PEEK) < 1) {
-	peeked = recv(ip->socket, &peek_buf, 1, MSG_PEEK);
+	     /*	peek failure: this part of code is run each 
-	if (peeked != -1) {
+		second until any character has appeared on the
-	    // FIXME: if peeking a single byte is not possible
+		socket. until now a non-tls connect to an autodetect
-	    // the we can probably return
+		socket could cause a crash, or at least a runtime
-	    switch(peek_buf & 0xff) {
+		openssl error. here's the fix. even better:
-	    case 0x16: /* TLSv1 most likely */
+		this implements a countdown. if after a certain
-	    case 0x80: /* SSLv2 most likely */
+		number of seconds no SSL/TLS has shown up on the
-	    case 0x00: /* SSLv2 most likely */
+		socket we pass 0x44 to the switch below which
-		break;
+		deallocates the tls session and goes for plaintext. --lynX
-	    default:
+	     */
-                SSL_free(ip->tls_session);
+#  ifdef DEBUG
-                ip->tls_session = NULL;
+	    debug_message("TLS autodetect %d: could not peek from socket\n",
-                ip->tls_status = TLS_INACTIVE;
+		ip->tls_autodetect);
-		ret = -31337;
+#  endif
-		break;
+	    if (--ip->tls_autodetect) return 0;
-	    }
+	}
-	    ip->tls_autodetect = MY_FALSE;
+#  ifdef DEBUG
+	debug_message("TLS autodetect called off. ret = %d, peek byte is %x\n",
+	    ret, (int)peek_buf);
+#  endif
+	/* disable autodetect on established plaintext connections */
+	ip->tls_autodetect = 0;
+
+	switch(peek_buf & 0xff) {
+	case 0x16: /* TLSv1 most likely */
+	case 0x80: /* SSLv2 most likely */
+	case 0x00: /* SSLv2 most likely */
+	    break;
+	case 0x44: /* special marker for autodetect timeout */
+	default:
+	    SSL_free(ip->tls_session);
+	    ip->tls_session = NULL;
+	    ip->tls_status = TLS_INACTIVE;
+	    ret = ERR_TLS_NOT_DETECTED;
+	    break;
 	}
     }
 # endif
@@ -962,6 +979,12 @@ v_tls_init_connection (svalue_t *sp, int num_arg)
         }
         if (ip->outgoing_conn) 
 	{
+#if 1
+	    /* circumvent a bug in google talk or java ssl
+	     * see also http://twistedmatrix.com/trac/changeset/25471
+	     */
+	    SSL_set_options(session, SSL_OP_NO_SSLv2 | SSL_OP_NO_TICKET);
+#endif
 	    /* simply using the contexts default would send old 
 	     * handshakes not containing new features like compression
 	     */
@@ -993,7 +1016,7 @@ v_tls_init_connection (svalue_t *sp, int num_arg)
 #endif /* SSL Package */
 
         ip->tls_status = TLS_HANDSHAKING;
-        ret = tls_continue_handshake(ip);
+	ret = tls_continue_handshake(ip);
 
         /* Adjust the return value of tls_continue_handshake() */
         if (ret == 1)
@@ -1254,13 +1277,18 @@ tls_deinit_connection (interactive_t *ip)
 	/* when an SSL_ERROR_SYSCALL is produced, calling
 	 * SSL_shutdown may lead to a crash. SSL_set_shutdown
 	 * seems to be safer.		--lynX
+	 *
+	 * update july: worse than that, SSL_shutdown has crashed
+	 * on me even when SSL_read returned an innocent '0'.
+	 * let's try SSL_set_shutdown by default. probably we
+	 * don't need that TLS_BROKEN flag after all.
 	 */
-	if (ip->tls_status == TLS_BROKEN) {
+/*	if (ip->tls_status == TLS_BROKEN) { */
 		SSL_set_shutdown(ip->tls_session,
 		    SSL_SENT_SHUTDOWN|SSL_RECEIVED_SHUTDOWN);
-	} else {
+/*	} else {
 		SSL_shutdown(ip->tls_session);
-	}
+	} */
         SSL_free(ip->tls_session);
         ip->tls_session = NULL;
     }
@@ -1331,15 +1359,17 @@ f_tls_error(svalue_t *sp)
     const char *text;
     int err = sp->u.number;
 
+#ifdef USE_EXPERIMENTAL
+    if (err == ERR_TLS_NOT_DETECTED) text = "Unencrypted connection detected";
+    else
+#endif
+    {
 #ifdef HAS_OPENSSL
-
+	text = ERR_error_string(-err, NULL);
-    text = ERR_error_string(-err, NULL);
-
 #elif defined(HAS_GNUTLS)
-
+	text = gnutls_strerror(err);
-    text = gnutls_strerror(err);
-
 #endif /* SSL Package */
+    }
 
     if (text)
     {

src/util/erq/srv.c

@@ -28,6 +28,15 @@ typedef union {
 #define T_SRV		33
 #endif
 
+#if 1	/* _getshort */
+u_int ns_get16(const u_char *src) {
+	register u_char *t_cp = (u_char *)(src);
+	u_int s = ((u_int16_t)t_cp[0] << 8)
+		| ((u_int16_t)t_cp[1]);
+	src += NS_INT16SZ;
+        return s;
+}
+#endif
 
 void freesrvhost ( struct srvhost * s )
 {
@@ -155,17 +164,17 @@ struct srvhost * getsrv( const char * domain,
 
 	cp += n;
 
-	/* FIXME: this code is probably bullshit for 64bit */
+	/* FIXME: this code is probably not useful for 64bit */
-	type = _getshort(cp);
+	type = ns_get16(cp);
 	cp += sizeof(u_short);
 
-	/* class = _getshort(cp); */
+	/* class = ns_get16(cp); */
 	cp += sizeof(u_short);
 
 	/* ttl = _getlong(cp); */
 	cp += sizeof(u_int);
 
-	dlen = _getshort(cp);
+	dlen = ns_get16(cp);
 	cp += sizeof(u_short);
 
 	if ( type != T_SRV ) {
@@ -173,13 +182,13 @@ struct srvhost * getsrv( const char * domain,
 	    continue;
 	}
 
-	pref = _getshort(cp);
+	pref = ns_get16(cp);
 	cp += sizeof(u_short);
 
-	weight = _getshort(cp);
+	weight = ns_get16(cp);
 	cp += sizeof(u_short);
 
-	port = _getshort(cp);
+	port = ns_get16(cp);
 	cp += sizeof(u_short);
 
 	n = dn_expand( msg, eom, cp, (char *)hostbuf, 256 );

src/version.sh

@@ -5,13 +5,13 @@
 # Maybe we should start using SVN instead? Ye ye yeh
 
 # The checkin date
-version_date=$(echo "\$Date: 2008/05/29 13:06:04 $$" | perl -pe 's%.*\b(\d+)/(\d+)/(\d+)\b.*%\1-\2-\3%')
+version_date=$(echo "\$Date: 2008/08/03 13:14:29 $$" | perl -pe 's%.*\b(\d+)/(\d+)/(\d+)\b.*%\1-\2-\3%')
 
 # The checkin time
-version_time=$(echo "\$Date: 2008/05/29 13:06:04 $$" | perl -pe 's%.*\b(\d+:\d+:\d+)\b.*%\1%')
+version_time=$(echo "\$Date: 2008/08/03 13:14:29 $$" | perl -pe 's%.*\b(\d+:\d+:\d+)\b.*%\1%')
 
 # The checkin revision
-version_revision=$(echo "\$Revision: 1.55 $" | sed -e 's/[$]Revision: \([0-9]*\.[0-9]*\) *\$/\1/')
+version_revision=$(echo "\$Revision: 1.58 $" | sed -e 's/[$]Revision: \([0-9]*\.[0-9]*\) *\$/\1/')
 
 # The version type: dev, stable, maintenance, release
 version_type="stable"
@@ -20,10 +20,10 @@ version_longtype="binarius"
 # A timestamp, to be used by bumpversion and other scripts.
 # It can be used, for example, to 'touch' this file on every build, thus
 # forcing revision control systems to add it on every checkin automatically.
-version_stamp="Thu May 29 15:01:57 CEST 2008"
+version_stamp="Tue Feb 24 14:02:45 CET 2009"
 
 # The version number information
 # Okay, LDMUD is using 3.x.x so to avoid conflicts let's just use 4.x.x
 version_major=4
 version_minor=0
-version_micro=8
+version_micro=11