no SSL certificate verify

[Ruan Linqi]

Hi developers:
    We made a large scale security static analysis on several open source projects, and found some mistakes in moonshot-gss-eap_0.9.5,In the @libeap/src/crypto/Tls_openssl.c:2255:
   static struct wpabuf * openssl_handshake(struct tls_connection
   *conn, const struct wpabuf *in_data,int server)
{
        [...]
 if (server)
  res = SSL_accept(conn->ssl);
 else
  res = SSL_connect(conn->ssl);
       [...]
}

  You create SSL connect and then start to execute read/write operation without verify certificate,which can lead to MITM attack and cause leakage of sensitive data.We recommand you add cert verify operation such as SSL_CTX_set_verify or SSL_get_peer_certificate to guarantee the security.

[Adam Bishop (adam-omega)]

Can you share more information on this, such as the tool you used for static analysis or more detailed output? Ideal would be the code path that your tool believes exhibits the behaviour.

libeap's internal method tls_connection_set_verify() should be called to set the verification callback for the context before SSL_connect() or SSL_accept() is reached - if there is a code path that makes this not be the case, it's not immediately obvious.

[Ruan Linqi (shoppingruan)]

According to OpenSSL document, a correct certificate chain validation
pattern is like this:

const SSL_METHOD *method;
SSL_CTX *ctx;
SSL *ssl;
[...]
method = TLSv1_client_method(); //select protocol
[...]
ctx = SSL_CTX_new(method); //Create CTX
[...]
ssl = SSL_new(ctx); //Create SSL
[...]
//set SSL_VERIFY_PEER flag for certificate chain validation during handshake
SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER,...);
[...]
SSL_connect(ssl); //Start handshake

   SSL_CTX_set_verify sets the verification flags for CTX but
SSL_set_verify sets the verification flags for SSL. libeap's internal
method tls_connection_set_verify() contains the method SSL_set_verify(). I
think use method SSL_CTX_set_verify() is a correct way?

2017-04-01 2:33 GMT+08:00 Adam Bishop <email address hidden>:

> Can you share more information on this, such as the tool you used for
> static analysis or more detailed output? Ideal would be the code path
> that your tool believes exhibits the behaviour.
>
> libeap's internal method tls_connection_set_verify() should be called to
> set the verification callback for the context before SSL_connect() or
> SSL_accept() is reached - if there is a code path that makes this not be
> the case, it's not immediately obvious.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1677947
>
> Title:
> no SSL certificate verify
>
> Status in moonshot-gss-eap package in Ubuntu:
> New
>
> Bug description:
> Hi developers:
> We made a large scale security static analysis on several open
> source projects, and found some mistakes in moonshot-gss-eap_0.9.5,In the
> @libeap/src/crypto/Tls_openssl.c:2255:
> static struct wpabuf * openssl_handshake(struct tls_connection
> *conn, const struct wpabuf *in_data,int server)
> {
> [...]
> if (server)
> res = SSL_accept(conn->ssl);
> else
> res = SSL_connect(conn->ssl);
> [...]
> }
>
> You create SSL connect and then start to execute read/write
> operation without verify certificate,which can lead to MITM attack and
> cause leakage of sensitive data.We recommand you add cert verify
> operation such as SSL_CTX_set_verify or SSL_get_peer_certificate to
> guarantee the security.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/ubuntu/+source/moonshot-gss-eap/+bug/1677947/+
> subscriptions
>

[Jouni Malinen (jkmaline)]

SSL_CTX_set_verify() is used to modify the default value in an SSL_CTX instance and that will apply to every SSL instance created from that SSL_CTX. SSL_set_verify() is used to set the parameter for each SSL instance. Either call can be used in general to do the same.

SSL_CTX_set_verify() would be appropriate if the same parameters would apply to all SSL handshake instances, but that is not the case with an EAP server that handles both EAP-TLS (require client certificate validation) and EAP-TTLS/PEAP (do not require client certificate validation). SSL_set_verify() for each SSL instance is the way to go in such cases as can be seen in the implementation here.

In other words, this report does not look valid to me and it does not identify any real issue.