The following patch provides proper (D)TLS shutdown and cleanup. It has
been tested using both command line tests as well as the existing TLS
test sub-suite.
The clean up is obviously just "nice for memory", but the session
closing on the DTLS side was missed as the pointer used to detect if it
was needed was the incorrect pointer and thus the TLS connection closure
never got triggered, which is bad for the other side since a
close_notify never gets sent.
--
Wes Hardaker
Please mail all replies to [email protected]
diff --git a/net-snmp/include/net-snmp/library/snmpTLSBaseDomain.h b/net-snmp/include/net-snmp/library/snmpTLSBaseDomain.h
index 247f964..c3e3f52 100644
--- a/net-snmp/include/net-snmp/library/snmpTLSBaseDomain.h
+++ b/net-snmp/include/net-snmp/library/snmpTLSBaseDomain.h
@@ -76,6 +76,8 @@ extern "C" {
int netsnmp_tlsbase_session_init(struct netsnmp_transport_s *,
struct snmp_session *sess);
int tls_get_verify_info_index(void);
+
+ void *netsnmp_tlsbase_free_tlsdata(_netsnmpTLSBaseData *tlsbase);
#ifdef __cplusplus
}
#endif
diff --git a/net-snmp/snmplib/transports/snmpDTLSUDPDomain.c b/net-snmp/snmplib/transports/snmpDTLSUDPDomain.c
index e56e8fa..a86d07f 100644
--- a/net-snmp/snmplib/transports/snmpDTLSUDPDomain.c
+++ b/net-snmp/snmplib/transports/snmpDTLSUDPDomain.c
@@ -126,6 +126,47 @@ static bio_cache *find_bio_cache(struct sockaddr_in *from_addr) {
return cachep;
}
+/* removes a single cache entry and returns SUCCESS on finding and
+ removing it. */
+static int remove_bio_cache(bio_cache *thiscache) {
+ bio_cache *cachep = NULL, *prevcache = NULL;
+ cachep = biocache;
+ while(cachep) {
+ if (cachep == thiscache) {
+
+ /* remove it from the list */
+ if (NULL == prevcache) {
+ /* at the first cache in the list */
+ biocache = thiscache->next;
+ } else {
+ prevcache->next = thiscache->next;
+ }
+
+ return SNMPERR_SUCCESS;
+ }
+ prevcache = cachep;
+ cachep = cachep->next;
+ }
+ return SNMPERR_GENERR;
+}
+
+/* frees the contents of a bio_cache */
+static void free_bio_cache(bio_cache *cachep) {
+/* These are freed by the SSL_free() call */
+/*
+ BIO_free(cachep->read_bio);
+ BIO_free(cachep->write_bio);
+*/
+ SNMP_FREE(cachep->write_cache);
+ netsnmp_tlsbase_free_tlsdata(cachep->tlsdata);
+}
+
+static void remove_and_free_bio_cache(bio_cache *cachep) {
+ remove_bio_cache(cachep);
+ free_bio_cache(cachep);
+}
+
+
/* XXX: lots of malloc/state cleanup needed */
#define DIEHERE(msg) { snmp_log(LOG_ERR, "%s\n", msg); return NULL; }
@@ -1094,6 +1135,8 @@ netsnmp_dtlsudp_close(netsnmp_transport *t)
3) If there is no open session associated with the tmSessionID, then
closeSession processing is completed.
*/
+
+
/* if we have any remaining packtes to send, try to send them */
if (NULL != cachep && cachep->write_cache_len > 0) {
int i = 0;
@@ -1141,8 +1184,22 @@ netsnmp_dtlsudp_close(netsnmp_transport *t)
sending a close_notify TLS Alert to inform the other side that
session cleanup may be performed.
*/
- if (NULL != tlsbase && NULL != tlsbase->ssl)
- SSL_shutdown(tlsbase->ssl);
+ if (NULL != cachep && NULL != cachep->tlsdata &&
+ NULL != cachep->tlsdata->ssl) {
+ DEBUGMSGTL(("dtlsudp", "closing SSL socket\n"));
+ SSL_shutdown(cachep->tlsdata->ssl);
+ }
+
+ /* (this will include the close_notify we maybe generated in step 4 */
+ if (cachep && BIO_ctrl_pending(cachep->write_bio) > 0) {
+ _netsnmp_send_queued_dtls_pkts(t, cachep);
+ }
+
+ if (NULL != cachep && NULL != cachep->tlsdata &&
+ NULL != cachep->tlsdata->ssl) {
+ DEBUGMSGTL(("dtlsudp", "freeing OpenSSL datad\n"));
+ remove_and_free_bio_cache(cachep);
+ }
return netsnmp_socketbase_close(t);
}
diff --git a/net-snmp/snmplib/transports/snmpTLSBaseDomain.c b/net-snmp/snmplib/transports/snmpTLSBaseDomain.c
index 4d0ed6d..db203b4 100644
--- a/net-snmp/snmplib/transports/snmpTLSBaseDomain.c
+++ b/net-snmp/snmplib/transports/snmpTLSBaseDomain.c
@@ -772,6 +772,40 @@ netsnmp_tlsbase_allocate_tlsdata(netsnmp_transport *t, int isserver) {
return tlsdata;
}
+void *netsnmp_tlsbase_free_tlsdata(_netsnmpTLSBaseData *tlsbase) {
+ if (!tlsbase)
+ return;
+
+ DEBUGMSGTL(("tlsbase","Freeing TLS Base data for a session\n"));
+
+ if (tlsbase->ssl)
+ SSL_free(tlsbase->ssl);
+
+ if (tlsbase->ssl_context)
+ SSL_CTX_free(tlsbase->ssl_context);
+ /* don't free the accept_bio since it's the parent bio */
+ /*
+ if (tlsbase->accept_bio)
+ BIO_free(tlsbase->accept_bio);
+ */
+ /* and this is freed by the SSL shutdown */
+ /*
+ if (tlsbase->accepted_bio)
+ BIO_free(tlsbase->accept_bio);
+ */
+
+ /* free the config data */
+ SNMP_FREE(tlsbase->securityName);
+ SNMP_FREE(tlsbase->addr_string);
+ SNMP_FREE(tlsbase->our_identity);
+ SNMP_FREE(tlsbase->their_identity);
+ SNMP_FREE(tlsbase->their_fingerprint);
+ SNMP_FREE(tlsbase->their_hostname);
+ SNMP_FREE(tlsbase->trust_cert);
+
+ /* free the base itself */
+ SNMP_FREE(tlsbase);
+}
int netsnmp_tlsbase_wrapup_recv(netsnmp_tmStateReference *tmStateRef,
_netsnmpTLSBaseData *tlsdata,
diff --git a/net-snmp/snmplib/transports/snmpTLSTCPDomain.c b/net-snmp/snmplib/transports/snmpTLSTCPDomain.c
index 60f325f..1cb5653 100644
--- a/net-snmp/snmplib/transports/snmpTLSTCPDomain.c
+++ b/net-snmp/snmplib/transports/snmpTLSTCPDomain.c
@@ -114,6 +114,25 @@ netsnmp_tlstcp_copy(netsnmp_transport *oldt, netsnmp_transport *newt)
oldtlsdata->accepted_bio = NULL;
oldtlsdata->ssl = NULL;
newtlsdata->ssl_context = NULL;
+
+ if (oldtlsdata->addr_string)
+ newtlsdata->addr_string = strdup(oldtlsdata->addr_string);
+ if (oldtlsdata->securityName)
+ newtlsdata->securityName = strdup(oldtlsdata->securityName);
+ if (oldtlsdata->our_identity)
+ newtlsdata->our_identity = strdup(oldtlsdata->our_identity);
+ if (oldtlsdata->their_identity)
+ newtlsdata->their_identity = strdup(oldtlsdata->their_identity);
+ if (oldtlsdata->their_fingerprint)
+ newtlsdata->their_fingerprint = strdup(oldtlsdata->their_fingerprint);
+ if (oldtlsdata->their_hostname)
+ newtlsdata->their_hostname = strdup(oldtlsdata->their_hostname);
+ if (oldtlsdata->trust_cert)
+ newtlsdata->trust_cert = strdup(oldtlsdata->trust_cert);
+ if (oldtlsdata->remote_addr)
+ memdup(&newtlsdata->remote_addr, oldtlsdata->remote_addr,
+ sizeof(netsnmp_indexed_addr_pair));
+
return 0;
}
@@ -454,30 +473,14 @@ netsnmp_tlstcp_close(netsnmp_transport *t)
sending a close_notify TLS Alert to inform the other side that
session cleanup may be performed.
*/
+
+ DEBUGMSGTL(("tlstcp", "Shutting down SSL connection\n"));
if (tlsdata->ssl) {
SSL_shutdown(tlsdata->ssl);
- tlsdata->ssl = NULL;
- }
-
- if (tlsdata->sslbio) {
- BIO_free(tlsdata->sslbio);
- tlsdata->sslbio = NULL;
- }
-
- if (tlsdata->accepted_bio) {
- BIO_free(tlsdata->accepted_bio);
- tlsdata->accepted_bio = NULL;
}
- SNMP_FREE(tlsdata->securityName);
- SNMP_FREE(tlsdata->our_identity);
- SNMP_FREE(tlsdata->their_identity);
- SNMP_FREE(tlsdata->their_hostname);
- SNMP_FREE(tlsdata->their_fingerprint);
- SNMP_FREE(tlsdata->trust_cert);
+ netsnmp_tlsbase_free_tlsdata(tlsdata);
- /* don't free the accept_bio since it's the parent bio */
- SNMP_FREE(tlsdata);
t->data = NULL;
return netsnmp_socketbase_close(t);
}
@@ -915,14 +918,12 @@ netsnmp_tlstcp_transport(const char *addr_string, int isserver)
if (!isserver && tlsdata && addr_string) {
/* search for a : */
if (NULL != (cp = strrchr(addr_string, ':'))) {
- strncpy(buf, addr_string, SNMP_MIN(cp-addr_string, sizeof(buf)-1));
- buf[SNMP_MIN(cp-addr_string, sizeof(buf)-1)] = '\0';
+ strncpy(buf, addr_string, sizeof(buf)-1);
} else {
/* else the entire spec is a host name only */
- strncpy(buf, addr_string,
- SNMP_MIN(strlen(addr_string), sizeof(buf)-1));
- buf[SNMP_MIN(strlen(addr_string), sizeof(buf)-1)] = '\0';
+ strncpy(buf, addr_string, sizeof(buf)-1);
}
+ buf[sizeof(buf)-1] = '\0';
tlsdata->their_hostname = strdup(buf);
}
------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:
Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd
_______________________________________________
Net-snmp-coders mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/net-snmp-coders
