[Edbrowse-dev] [PATCH] Implement the "novs" directive.

[Edbrowse-dev] [PATCH] Implement the "novs" directive.

[Karl Dahlke]

This looks good.
You could submit it and I would be fine with that.

I do have a couple small questions though.
The first is just style / consistency.
You use strcasecmp for I guess string compare case insensitive.
A library function I didn't know about?
I use throughout stringEqualCI, which I wrote myself.
Should we always use one, or the other?
Mine is part of a suite of such CI functions, like mem compare CI and so on,
I'm sure not all have library equivalents.

You took the time to add sites in a growing dynamic list,
which is right, sometimes I was too lazy to do that.
Like nojs, I store 100 sites for no javascript and that's it.
Any time you, or I, feel like making this dynamic, feel free.
Same for email accounts, though I can't imagine anyone having
more than 100 of those.
And maybe other static lists floating around.

When you check for novs match you need an exact match.
Not so for nojs.
whatever.com will also have no javascript for subdomains
like foo.whatever.com.
See the code in main.c that does that.
Should we do the same for novs?
I would think so; if there's no cert for whatever.com
then there is for sure no cert for foo.whatever.com.


Karl Dahlke

Re: [Edbrowse-dev] [PATCH] Implement the "novs" directive.

[Chris Brannon]

> Mine is part of a suite of such CI functions, like mem compare CI and so on,

Aha, I had forgotten about that function!  The standard library has a
memcmp, but not an equivalent memcasecmp.  For consistency, I don't have
a problem using stringEqualCI.  It's your call.

> You took the time to add sites in a growing dynamic list,

I'm happy to look into replacing the other static lists at some
point.  Dynamic arrays in C are clunky, so it's just typing.

> I would think so; if there's no cert for whatever.com
> then there is for sure no cert for foo.whatever.com.

You're probably right.  My inclination is to be cautious and require an
exact match though, and I'm not sure why.

-- Chris

Re: [Edbrowse-dev] [PATCH] Implement the "novs" directive.

[Adam Thompson]

[-- Attachment #1: Type: text/plain, Size: 752 bytes --]

On Fri, Sep 12, 2014 at 02:34:13PM -0700, Chris Brannon wrote:
> > You took the time to add sites in a growing dynamic list,
> 
> I'm happy to look into replacing the other static lists at some
> point.  Dynamic arrays in C are clunky, so it's just typing.
They can be, but as you say, it's a fairly mechanical change usually.
That being said, I just managed to pull the latest edbrowse (I'm moving house
and am relying on my mobile phone for internet and it's only just started
working), and noticed lines like:
novs_hosts = allocZeroMem(novs_hosts_max);

May be I'm missing something but shouldn't that really be:
novs_hosts = allocZeroMem(novs_hosts_max*sizeof(char *));

Given that novs_hosts is a char** variable?

Cheers,
Adam.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Edbrowse-dev] [PATCH] Implement the "novs" directive.

[Chris Brannon]

Adam Thompson <arthompson1990@gmail.com> writes:

> novs_hosts = allocZeroMem(novs_hosts_max*sizeof(char *));

Yes it should.  Thank you for catching it!

-- Chris

[Edbrowse-dev] [PATCH] Implement the "novs" directive.

[Christopher Brannon]

---
 doc/usersguide.html | 19 ++++++++++++-------
 src/buffers.c       |  2 --
 src/eb.h            |  2 +-
 src/eb.p            |  1 +
 src/fetchmail.c     |  5 ++---
 src/main.c          | 16 ++++++++++------
 src/sendmail.c      |  3 ---
 src/url.c           | 35 +++++++++++++++++++++++++++++++++++
 8 files changed, 61 insertions(+), 22 deletions(-)

diff --git a/doc/usersguide.html b/doc/usersguide.html
index ed365d8..add72db 100644
--- a/doc/usersguide.html
+++ b/doc/usersguide.html
@@ -2537,7 +2537,9 @@ Some browsers don't have this feature at all, so it's not the end of the world,
 but in general it's a good idea to verify your secure connections,
 unless it prevents you from getting to a website whose authenticity you accept at face value. 
 In that case you can use the vs command to turn the feature off. 
-This is a toggle command; type vs again to turn the feature on.
+This is a toggle command; type vs again to turn the feature on. 
+For another method of disabling verification on a site-by-site basis,
+see the novs directive in the configuration file.
 
 <P>
 Never send sensitive information,
@@ -3071,6 +3073,15 @@ A couple of youtube pages will consume 4 meg of javascript, so don't aim low unl
 If you spend all day browsing, you better aim high, because edbrowse could unceremoniously exit if it runs out of javascript space.
 
 <P>
+novs = somesite.com
+<P>
+Indicate hostnames for which SSL certificate verification should never
+be performed.&nbsp;
+This directive is useful for sites that use self-signed certificates,
+since these cannot be verified.&nbsp;
+It should probably not be used for anything serious, such as a site that
+is going to receive your credit card number.
+<P>
 inserver = pop3.some-domain.com
 <br>
 inport = 110
@@ -3110,12 +3121,6 @@ nofetch
 Do not fetch mail from this account through the -f option.
 
 <P>
-nocert
-<P>
-This account uses secure connections, i.e. pop3s or smtps,
-but there is no certificate for the secure server.
-
-<P>
 login = eklhad
 <br>
 password = secret
diff --git a/src/buffers.c b/src/buffers.c
index dcd862c..b2f52fc 100644
--- a/src/buffers.c
+++ b/src/buffers.c
@@ -3172,8 +3172,6 @@ et_go:
 
 	if (stringEqual(line, "vs")) {
 		verifyCertificates ^= 1;
-		curl_easy_setopt(curl_handle, CURLOPT_SSL_VERIFYPEER,
-				 verifyCertificates);
 		if (helpMessagesOn || debugLevel >= 1)
 			i_puts(verifyCertificates + MSG_CertifyOff);
 		return eb_true;
diff --git a/src/eb.h b/src/eb.h
index 13f279c..09f27c5 100644
--- a/src/eb.h
+++ b/src/eb.h
@@ -159,7 +159,7 @@ struct MACCOUNT {		/* pop3 account */
 	char *inurl, *outurl;
 	int inport, outport;
 	uchar inssl, outssl;
-	char nofetch, nocert;
+	char nofetch;
 };
 extern struct MACCOUNT accounts[];	/* all the email accounts */
 extern int maxAccount;		/* how many email accounts specified */
diff --git a/src/eb.p b/src/eb.p
index 7bbb250..578090a 100644
--- a/src/eb.p
+++ b/src/eb.p
@@ -70,6 +70,7 @@ char *encodePostData(const char *s);
 char *decodePostData(const char *data, const char *name, int seqno);
 void decodeMailURL(const char *url, char **addr_p, char **subj_p,
 		   char **body_p);
+void addNovsHost(char *host);
 CURLcode setCurlURL(CURL *h, const char *url);
 
 /* sourcefile=auth.c */
diff --git a/src/fetchmail.c b/src/fetchmail.c
index b5d375a..9943cb5 100644
--- a/src/fetchmail.c
+++ b/src/fetchmail.c
@@ -328,7 +328,7 @@ static struct eb_curl_callback_data callback_data = {
 };
 
 static CURL *newFetchmailHandle(const char *mailbox, const char *username,
-				const char *password, int do_certs)
+				const char *password)
 {
 	CURLcode res;
 	CURL *handle = curl_easy_init();
@@ -344,7 +344,6 @@ static CURL *newFetchmailHandle(const char *mailbox, const char *username,
 	if (res != CURLE_OK)
 		i_printfExit(MSG_LibcurlNoInit);
 
-	curl_easy_setopt(handle, CURLOPT_SSL_VERIFYPEER, do_certs);
 	res = curl_easy_setopt(handle, CURLOPT_USERNAME, username);
 	if (res != CURLE_OK) {
 		ebcurl_setError(res, mailbox);
@@ -493,7 +492,7 @@ int fetchMail(int account)
 
 	mailstring = initString(&mailstring_l);
 	CURL *mail_handle =
-	    newFetchmailHandle(mailbox_url, login, pass, !a->nocert);
+	    newFetchmailHandle(mailbox_url, login, pass);
 	res_curl = count_messages(mail_handle, mailbox_url, &message_count);
 	if (res_curl != CURLE_OK)
 		goto fetchmail_cleanup;
diff --git a/src/main.c b/src/main.c
index a889316..8181f97 100644
--- a/src/main.c
+++ b/src/main.c
@@ -166,7 +166,7 @@ static void readConfigFile(void)
 		"adbook", "ipblack", "maildir", "agent",
 		"jar", "nojs", "spamcan",
 		"webtimer", "mailtimer", "certfile", "datasource", "proxy",
-		"linelength", "localizeweb", "jspool",
+		"linelength", "localizeweb", "jspool", "novs",
 		0
 	};
 
@@ -662,6 +662,15 @@ putc:
 				jsPool = 1000;
 			continue;
 
+		case 32:
+			if (*v == '.')
+				++v;
+			q = strchr(v, '.');
+			if (!q || q[1] == 0)
+				i_printfExit(MSG_ERBC_DomainDot, ln, v);
+			addNovsHost(v);
+			continue;
+
 		default:
 			i_printfExit(MSG_ERBC_KeywordNYI, ln, s);
 		}		/* switch */
@@ -682,11 +691,6 @@ nokeyword:
 			continue;
 		}
 
-		if (stringEqual(s, "nocert") && mailblock == 1) {
-			act->nocert = 1;
-			continue;
-		}
-
 		if (*s == '\x82' && s[1] == 0) {
 			if (mailblock == 1) {
 				++maxAccount;
diff --git a/src/sendmail.c b/src/sendmail.c
index 9ff87d6..5b311a3 100644
--- a/src/sendmail.c
+++ b/src/sendmail.c
@@ -745,7 +745,6 @@ static CURL *newSendmailHandle(const struct MACCOUNT *account,
 			       const char *outurl, const char *reply,
 			       struct curl_slist *recipients)
 {
-	int do_certs = verifyCertificates && !account->nocert;
 	CURLcode res = CURLE_OK;
 	CURL *handle = curl_easy_init();
 	if (!handle) {
@@ -766,8 +765,6 @@ static CURL *newSendmailHandle(const struct MACCOUNT *account,
 		goto new_handle_cleanup;
 	}
 
-	curl_easy_setopt(handle, CURLOPT_SSL_VERIFYPEER, do_certs);
-
 	if (account->outssl == 2)
 		curl_easy_setopt(handle, CURLOPT_USE_SSL, CURLUSESSL_ALL);
 
diff --git a/src/url.c b/src/url.c
index 492913a..49540fc 100644
--- a/src/url.c
+++ b/src/url.c
@@ -1006,6 +1006,38 @@ const char *findProxyForURL(const char *url)
 	return findProxyInternal(getProtURL(url), getHostURL(url));
 }				/* findProxyForURL */
 
+static char **novs_hosts;
+size_t novs_hosts_avail;
+size_t novs_hosts_max;
+
+void addNovsHost(char *host)
+{
+	if (novs_hosts_max == 0) {
+		novs_hosts_max = 32;
+		novs_hosts = allocZeroMem(novs_hosts_max);
+	} else if (novs_hosts_avail >= novs_hosts_max) {
+		novs_hosts_max *= 2;
+		novs_hosts = reallocMem(novs_hosts, novs_hosts_max);
+	}
+	novs_hosts[novs_hosts_avail++] = host;
+}				/* addNovsHost */
+
+/* Return true if the cert for this host should be verified. */
+static eb_bool mustVerifyHost(const char *host)
+{
+	size_t i;
+
+	if (!verifyCertificates)
+		return eb_false;
+
+	for (i = 0; i < novs_hosts_avail; i++) {
+		if (!strcasecmp(host, novs_hosts[i])) {
+			return eb_false;
+		}
+	}
+	return eb_true;
+}				/* mustVerifyHost */
+
 CURLcode setCurlURL(CURL * h, const char *url)
 {
 	const char *proxy = findProxyForURL(url);
@@ -1013,6 +1045,9 @@ CURLcode setCurlURL(CURL * h, const char *url)
 		proxy = "";
 	else
 		debugPrint(3, "proxy %s", proxy);
+	const char *host = getHostURL(url);
 	curl_easy_setopt(h, CURLOPT_PROXY, proxy);
+	curl_easy_setopt(curl_handle, CURLOPT_SSL_VERIFYPEER,
+			 mustVerifyHost(host));
 	return curl_easy_setopt(h, CURLOPT_URL, url);
 }				/* setCurlURL */
-- 
1.8.5.5