Patchwork [4,of,4] tests: add basic tests for SMTP over SSL

login
register
mail settings
Submitter Yuya Nishihara
Date June 8, 2016, 2:23 p.m.
Message ID <597e8dac9badd05a6633.1465395782@mimosa>
Download mbox | patch
Permalink /patch/15438/
State Accepted
Headers show

Comments

Yuya Nishihara - June 8, 2016, 2:23 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1464358718 -32400
#      Fri May 27 23:18:38 2016 +0900
# Node ID 597e8dac9badd05a66333fbbee66a584f17d0c3e
# Parent  85f6121625534279562bee84a97886c54af5a7d5
tests: add basic tests for SMTP over SSL

SSL handling in mail.py wasn't covered by our test suite, therefore it was
sometimes broken. This patch introduces pretty minimal tests that only cover
the default path. We can extend it later.

Tested with python 2.6.9 and 2.7.11 on Debian sid.
Sean Farley - June 8, 2016, 7:45 p.m.
Yuya Nishihara <yuya@tcha.org> writes:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1464358718 -32400
> #      Fri May 27 23:18:38 2016 +0900
> # Node ID 597e8dac9badd05a66333fbbee66a584f17d0c3e
> # Parent  85f6121625534279562bee84a97886c54af5a7d5
> tests: add basic tests for SMTP over SSL
>
> SSL handling in mail.py wasn't covered by our test suite, therefore it was
> sometimes broken. This patch introduces pretty minimal tests that only cover
> the default path. We can extend it later.
>
> Tested with python 2.6.9 and 2.7.11 on Debian sid.

Looks fairly good to me.
Gregory Szorc - June 8, 2016, 10:03 p.m.
On Wed, Jun 8, 2016 at 7:23 AM, Yuya Nishihara <yuya@tcha.org> wrote:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1464358718 -32400
> #      Fri May 27 23:18:38 2016 +0900
> # Node ID 597e8dac9badd05a66333fbbee66a584f17d0c3e
> # Parent  85f6121625534279562bee84a97886c54af5a7d5
> tests: add basic tests for SMTP over SSL
>
> SSL handling in mail.py wasn't covered by our test suite, therefore it was
> sometimes broken. This patch introduces pretty minimal tests that only
> cover
> the default path. We can extend it later.
>
> Tested with python 2.6.9 and 2.7.11 on Debian sid.
>

This series LGTM. It likely bitrots test-https.t in the patch I sent last
night. I think this series should take precedence. I'll rebase my work when
this is pushed to somewhere I can pull from.


>
> diff --git a/tests/test-patchbomb-tls.t b/tests/test-patchbomb-tls.t
> new file mode 100644
> --- /dev/null
> +++ b/tests/test-patchbomb-tls.t
> @@ -0,0 +1,89 @@
> +#require serve ssl
> +
> +Set up SMTP server:
> +
> +  $ CERTSDIR="$TESTDIR/sslcerts"
> +  $ cat "$CERTSDIR/priv.pem" "$CERTSDIR/pub.pem" >> server.pem
> +
> +  $ python "$TESTDIR/dummysmtpd.py" -p $HGPORT --pid-file a.pid -d \
> +  > --tls smtps --certificate `pwd`/server.pem
> +  listening at localhost:$HGPORT
> +  $ cat a.pid >> $DAEMON_PIDS
> +
> +Ensure hg email output is sent to stdout:
> +
> +  $ unset PAGER
> +
> +Set up repository:
> +
> +  $ hg init t
> +  $ cd t
> +  $ cat <<EOF >> .hg/hgrc
> +  > [extensions]
> +  > patchbomb =
> +  > [email]
> +  > method = smtp
> +  > [smtp]
> +  > host = localhost
> +  > port = $HGPORT
> +  > tls = smtps
> +  > EOF
> +
> +  $ echo a > a
> +  $ hg commit -Ama -d '1 0'
> +  adding a
> +
> +Utility functions:
> +
> +  $ DISABLECACERTS=
> +  $ try () {
> +  >   hg email $DISABLECACERTS -f quux -t foo -c bar -r tip "$@"
> +  > }
> +
> +Our test cert is not signed by a trusted CA. It should fail to verify if
> +we are able to load CA certs:
> +
> +#if defaultcacerts
> +  $ try
> +  this patch series consists of 1 patches.
> +
> +
> +  (?i)abort: .*?certificate.verify.failed.* (re)
> +  [255]
> +#endif
> +
> +  $ DISABLECACERTS="--config devel.disableloaddefaultcerts=true"
> +
> +Without certificates:
> +
> +  $ try --debug
> +  this patch series consists of 1 patches.
> +
> +
> +  (using smtps)
> +  sending mail: smtp host localhost, port * (glob)
> +  (verifying remote certificate)
> +  warning: certificate for localhost not verified (set
> hostsecurity.localhost:certfingerprints=sha256:62:09:97:2f:97:60:e3:65:8f:12:5d:78:9e:35:a1:36:7a:65:4b:0e:9f:ac:db:c3:bc:6e:b6:a3:c0:16:e0:30
> or web.cacerts config settings)
> +  sending [PATCH] a ...
> +
> +With global certificates:
> +
> +  $ try --debug --config web.cacerts="$CERTSDIR/pub.pem"
> +  this patch series consists of 1 patches.
> +
> +
> +  (using smtps)
> +  sending mail: smtp host localhost, port * (glob)
> +  (verifying remote certificate)
> +  sending [PATCH] a ...
> +
> +With invalid certificates:
> +
> +  $ try --config web.cacerts="$CERTSDIR/pub-other.pem"
> +  this patch series consists of 1 patches.
> +
> +
> +  (?i)abort: .*?certificate.verify.failed.* (re)
> +  [255]
> +
> +  $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Augie Fackler - June 10, 2016, 1:34 a.m.
On Wed, Jun 08, 2016 at 11:23:02PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1464358718 -32400
> #      Fri May 27 23:18:38 2016 +0900
> # Node ID 597e8dac9badd05a66333fbbee66a584f17d0c3e
> # Parent  85f6121625534279562bee84a97886c54af5a7d5
> tests: add basic tests for SMTP over SSL

Queued these, thanks.

>
> SSL handling in mail.py wasn't covered by our test suite, therefore it was
> sometimes broken. This patch introduces pretty minimal tests that only cover
> the default path. We can extend it later.
>
> Tested with python 2.6.9 and 2.7.11 on Debian sid.
>
> diff --git a/tests/test-patchbomb-tls.t b/tests/test-patchbomb-tls.t
> new file mode 100644
> --- /dev/null
> +++ b/tests/test-patchbomb-tls.t
> @@ -0,0 +1,89 @@
> +#require serve ssl
> +
> +Set up SMTP server:
> +
> +  $ CERTSDIR="$TESTDIR/sslcerts"
> +  $ cat "$CERTSDIR/priv.pem" "$CERTSDIR/pub.pem" >> server.pem
> +
> +  $ python "$TESTDIR/dummysmtpd.py" -p $HGPORT --pid-file a.pid -d \
> +  > --tls smtps --certificate `pwd`/server.pem
> +  listening at localhost:$HGPORT
> +  $ cat a.pid >> $DAEMON_PIDS
> +
> +Ensure hg email output is sent to stdout:
> +
> +  $ unset PAGER
> +
> +Set up repository:
> +
> +  $ hg init t
> +  $ cd t
> +  $ cat <<EOF >> .hg/hgrc
> +  > [extensions]
> +  > patchbomb =
> +  > [email]
> +  > method = smtp
> +  > [smtp]
> +  > host = localhost
> +  > port = $HGPORT
> +  > tls = smtps
> +  > EOF
> +
> +  $ echo a > a
> +  $ hg commit -Ama -d '1 0'
> +  adding a
> +
> +Utility functions:
> +
> +  $ DISABLECACERTS=
> +  $ try () {
> +  >   hg email $DISABLECACERTS -f quux -t foo -c bar -r tip "$@"
> +  > }
> +
> +Our test cert is not signed by a trusted CA. It should fail to verify if
> +we are able to load CA certs:
> +
> +#if defaultcacerts
> +  $ try
> +  this patch series consists of 1 patches.
> +
> +
> +  (?i)abort: .*?certificate.verify.failed.* (re)
> +  [255]
> +#endif
> +
> +  $ DISABLECACERTS="--config devel.disableloaddefaultcerts=true"
> +
> +Without certificates:
> +
> +  $ try --debug
> +  this patch series consists of 1 patches.
> +
> +
> +  (using smtps)
> +  sending mail: smtp host localhost, port * (glob)
> +  (verifying remote certificate)
> +  warning: certificate for localhost not verified (set hostsecurity.localhost:certfingerprints=sha256:62:09:97:2f:97:60:e3:65:8f:12:5d:78:9e:35:a1:36:7a:65:4b:0e:9f:ac:db:c3:bc:6e:b6:a3:c0:16:e0:30 or web.cacerts config settings)
> +  sending [PATCH] a ...
> +
> +With global certificates:
> +
> +  $ try --debug --config web.cacerts="$CERTSDIR/pub.pem"
> +  this patch series consists of 1 patches.
> +
> +
> +  (using smtps)
> +  sending mail: smtp host localhost, port * (glob)
> +  (verifying remote certificate)
> +  sending [PATCH] a ...
> +
> +With invalid certificates:
> +
> +  $ try --config web.cacerts="$CERTSDIR/pub-other.pem"
> +  this patch series consists of 1 patches.
> +
> +
> +  (?i)abort: .*?certificate.verify.failed.* (re)
> +  [255]
> +
> +  $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/tests/test-patchbomb-tls.t b/tests/test-patchbomb-tls.t
new file mode 100644
--- /dev/null
+++ b/tests/test-patchbomb-tls.t
@@ -0,0 +1,89 @@ 
+#require serve ssl
+
+Set up SMTP server:
+
+  $ CERTSDIR="$TESTDIR/sslcerts"
+  $ cat "$CERTSDIR/priv.pem" "$CERTSDIR/pub.pem" >> server.pem
+
+  $ python "$TESTDIR/dummysmtpd.py" -p $HGPORT --pid-file a.pid -d \
+  > --tls smtps --certificate `pwd`/server.pem
+  listening at localhost:$HGPORT
+  $ cat a.pid >> $DAEMON_PIDS
+
+Ensure hg email output is sent to stdout:
+
+  $ unset PAGER
+
+Set up repository:
+
+  $ hg init t
+  $ cd t
+  $ cat <<EOF >> .hg/hgrc
+  > [extensions]
+  > patchbomb =
+  > [email]
+  > method = smtp
+  > [smtp]
+  > host = localhost
+  > port = $HGPORT
+  > tls = smtps
+  > EOF
+
+  $ echo a > a
+  $ hg commit -Ama -d '1 0'
+  adding a
+
+Utility functions:
+
+  $ DISABLECACERTS=
+  $ try () {
+  >   hg email $DISABLECACERTS -f quux -t foo -c bar -r tip "$@"
+  > }
+
+Our test cert is not signed by a trusted CA. It should fail to verify if
+we are able to load CA certs:
+
+#if defaultcacerts
+  $ try
+  this patch series consists of 1 patches.
+  
+  
+  (?i)abort: .*?certificate.verify.failed.* (re)
+  [255]
+#endif
+
+  $ DISABLECACERTS="--config devel.disableloaddefaultcerts=true"
+
+Without certificates:
+
+  $ try --debug
+  this patch series consists of 1 patches.
+  
+  
+  (using smtps)
+  sending mail: smtp host localhost, port * (glob)
+  (verifying remote certificate)
+  warning: certificate for localhost not verified (set hostsecurity.localhost:certfingerprints=sha256:62:09:97:2f:97:60:e3:65:8f:12:5d:78:9e:35:a1:36:7a:65:4b:0e:9f:ac:db:c3:bc:6e:b6:a3:c0:16:e0:30 or web.cacerts config settings)
+  sending [PATCH] a ...
+
+With global certificates:
+
+  $ try --debug --config web.cacerts="$CERTSDIR/pub.pem"
+  this patch series consists of 1 patches.
+  
+  
+  (using smtps)
+  sending mail: smtp host localhost, port * (glob)
+  (verifying remote certificate)
+  sending [PATCH] a ...
+
+With invalid certificates:
+
+  $ try --config web.cacerts="$CERTSDIR/pub-other.pem"
+  this patch series consists of 1 patches.
+  
+  
+  (?i)abort: .*?certificate.verify.failed.* (re)
+  [255]
+
+  $ cd ..