Gnus development mailing list
 help / color / mirror / Atom feed
* [Patch] Make tls.el support certificate verification
@ 2007-09-16 23:08 Elias Oltmanns
  2007-09-24  7:12 ` Simon Josefsson
  0 siblings, 1 reply; 16+ messages in thread
From: Elias Oltmanns @ 2007-09-16 23:08 UTC (permalink / raw)
  To: ding; +Cc: emacs-devel

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

Hi all,

since there is a copy of tls.el in gnus but the emacs22 copy is moved to
lisp/net, I'm not quite sure as to who is ultimately maintaining it.
Hence, I'm sending this to both lists.

Please find attached a patch (to current gnus trunk) that adds all it
needs to facilitate the certificate verification features of gnutls-cli
and openssl.

Regards,

Elias


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: tls.patch --]
[-- Type: text/x-patch, Size: 2880 bytes --]

Index: lisp/tls.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/tls.el,v
retrieving revision 7.14
diff -u -r7.14 tls.el
--- lisp/tls.el	9 Apr 2007 23:44:06 -0000	7.14
+++ lisp/tls.el	16 Sep 2007 15:40:39 -0000
@@ -82,6 +82,38 @@
   :type 'regexp
   :group 'tls)
 
+(defcustom tls-checktrust nil
+  "Indicate if certificates should be checked against trusted root certs.
+If this is `ask', the user can decide whether to accept an untrusted
+certificate. You may have to adapt `tls-program' in order to make this feature
+work properly, i.e., to ensure that the external program knows about the
+root certificates you consider trustworthy. An appropriate entry in .emacs
+might look like this:
+(setq tls-program
+      '(\"gnutls-cli --x509cafile /etc/ssl/certs/ca-certificates.crt -p %p %h\"
+	\"gnutls-cli --x509cafile /etc/ssl/certs/ca-certificates.crt -p %p %h --protocols ssl3\"
+	\"openssl s_client -connect %h:%p -CAfile /etc/ssl/certs/ca-certificates.crt -no_ssl2\"))"
+  :type '(choice (const :tag "Always" t)
+		 (const :tag "Never" nil)
+		 (const :tag "Ask" ask))
+  :group 'tls)
+
+(defcustom tls-untrusted "- Peer's certificate is NOT trusted\\|Verify return code: \\([^0] \\|.[^ ]\\)"
+  "*Regular expression indicating failure of TLS certificate verification.
+The default is what GNUTLS's \"gnutls-cli\" or OpenSSL's
+\"openssl s_client\" return in the event of unsuccessful verification."
+  :type 'regexp
+  :group 'tls)
+
+(defcustom tls-hostmismatch "# The hostname in the certificate does NOT match"
+  "*Regular expression indicating a host name mismatch in certificate.
+When the host name specified in the certificate doesn't match the name of the
+host you are connecting to, gnutls-cli issues a warning to this effect. There
+is no such feature in openssl. Set this to nil if you want to ignore host name
+mismatches."
+  :type 'regexp
+  :group 'tls)
+
 (defcustom tls-certtool-program (executable-find "certtool")
   "Name of  GnuTLS certtool.
 Used by `tls-certificate-information'."
@@ -156,6 +188,25 @@
 		 (if done "done" "failed"))
 	(if done
 	    (setq done process)
+	  (delete-process process))))
+    (when done
+      (save-excursion
+	(set-buffer buffer)
+	(when
+	    (or
+	     (and tls-untrusted
+		  (progn
+		    (goto-char (point-min))
+		    (re-search-forward tls-untrusted nil t))
+		  (not (yes-or-no-p
+			(format "The certificate presented by `%s' is NOT trusted. Accept anyway? " host))))
+	     (and tls-hostmismatch
+		  (progn
+		    (goto-char (point-min))
+		    (re-search-forward tls-hostmismatch nil t))
+		  (not (yes-or-no-p
+			(format "Host name in certificate doesn't match `%s'. Connect anyway? " host)))))
+	  (setq done nil)
 	  (delete-process process))))
     (message "Opening TLS connection to `%s'...%s"
 	     host (if done "done" "failed"))

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch] Make tls.el support certificate verification
  2007-09-16 23:08 [Patch] Make tls.el support certificate verification Elias Oltmanns
@ 2007-09-24  7:12 ` Simon Josefsson
  2007-09-24 16:27   ` Reiner Steib
  0 siblings, 1 reply; 16+ messages in thread
From: Simon Josefsson @ 2007-09-24  7:12 UTC (permalink / raw)
  To: Elias Oltmanns; +Cc: ding, emacs-devel

Elias Oltmanns <eo@nebensachen.de> writes:

> Hi all,
>
> since there is a copy of tls.el in gnus but the emacs22 copy is moved to
> lisp/net, I'm not quite sure as to who is ultimately maintaining it.
> Hence, I'm sending this to both lists.

Hi!  Thanks for the patch.  I think it should be installed in both
CVS's, although perhaps tls.el should be removed from the Gnus
repository.  Others on the ding list, which Emacs version does Gnus in
CVS require?  If that emacs version has tls.el, I'm not sure it makes
sense to keep tls.el in the Gnus repository.

> Please find attached a patch (to current gnus trunk) that adds all it
> needs to facilitate the certificate verification features of gnutls-cli
> and openssl.

I did not test it, but it looks good.  Possibly the CA/client
certificate should get its own variable instead?  But that's not
important.

However, we need a copyright assignment to be able to use your patch
since it was over 10 lines of code.  I'll send it privately.

/Simon

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch] Make tls.el support certificate verification
  2007-09-24  7:12 ` Simon Josefsson
@ 2007-09-24 16:27   ` Reiner Steib
  2007-09-25 14:42     ` Simon Josefsson
  0 siblings, 1 reply; 16+ messages in thread
From: Reiner Steib @ 2007-09-24 16:27 UTC (permalink / raw)
  To: Elias Oltmanns; +Cc: ding, emacs-devel

On Mon, Sep 24 2007, Simon Josefsson wrote:

> I think it should be installed in both CVS's, although perhaps
> tls.el should be removed from the Gnus repository.  Others on the
> ding list, which Emacs version does Gnus in CVS require?

See (info "(gnus)Emacsen") or...

,----[ (info "(gnus-coding)Gnus Coding Style") ]
| 1.2 Compatibility
| =================
| 
| No Gnus and Gnus 5.10.10 and up should work on:
|    * Emacs 21.1 and up.
| 
|    * XEmacs 21.4 and up.
| 
|    Gnus 5.10.8 and below should work on:
|    * Emacs 20.7 and up.
| 
|    * XEmacs 21.1 and up.
`----

> If that emacs version has tls.el, I'm not sure it makes sense to
> keep tls.el in the Gnus repository.

Emacs 21 doesn't have it, so I think we should keep it.

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch] Make tls.el support certificate verification
  2007-09-24 16:27   ` Reiner Steib
@ 2007-09-25 14:42     ` Simon Josefsson
  2007-11-08 18:44       ` Elias Oltmanns
  0 siblings, 1 reply; 16+ messages in thread
From: Simon Josefsson @ 2007-09-25 14:42 UTC (permalink / raw)
  To: Elias Oltmanns; +Cc: ding, emacs-devel

Reiner Steib <reinersteib+gmane@imap.cc> writes:

> On Mon, Sep 24 2007, Simon Josefsson wrote:
>
>> I think it should be installed in both CVS's, although perhaps
>> tls.el should be removed from the Gnus repository.  Others on the
>> ding list, which Emacs version does Gnus in CVS require?
>
> See (info "(gnus)Emacsen") or...
>
> ,----[ (info "(gnus-coding)Gnus Coding Style") ]
> | 1.2 Compatibility
> | =================
> | 
> | No Gnus and Gnus 5.10.10 and up should work on:
> |    * Emacs 21.1 and up.
> | 
> |    * XEmacs 21.4 and up.
> | 
> |    Gnus 5.10.8 and below should work on:
> |    * Emacs 20.7 and up.
> | 
> |    * XEmacs 21.1 and up.
> `----
>
>> If that emacs version has tls.el, I'm not sure it makes sense to
>> keep tls.el in the Gnus repository.
>
> Emacs 21 doesn't have it, so I think we should keep it.

Right, thanks.  Let's wait for copyright papers and then apply the patch
in both CVS's.

/Simon



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch] Make tls.el support certificate verification
  2007-09-25 14:42     ` Simon Josefsson
@ 2007-11-08 18:44       ` Elias Oltmanns
  2007-11-08 19:52         ` Reiner Steib
  0 siblings, 1 reply; 16+ messages in thread
From: Elias Oltmanns @ 2007-11-08 18:44 UTC (permalink / raw)
  To: ding; +Cc: emacs-devel

Simon Josefsson <simon@josefsson.org> wrote:
> Reiner Steib <reinersteib+gmane@imap.cc> writes:
>
>> On Mon, Sep 24 2007, Simon Josefsson wrote:
>>
>>> I think it should be installed in both CVS's, although perhaps
>>> tls.el should be removed from the Gnus repository.  Others on the
>>> ding list, which Emacs version does Gnus in CVS require?
[...]
>>> If that emacs version has tls.el, I'm not sure it makes sense to
>>> keep tls.el in the Gnus repository.
>>
>> Emacs 21 doesn't have it, so I think we should keep it.
>
> Right, thanks.  Let's wait for copyright papers and then apply the patch
> in both CVS's.

Papers have been signed and receipt has been acknowledged.  Please
apply.

Could someone please look at [1] and [2] as well?  These patches are bug
fixes and recent bug reports wrt agentised servers (see [3]) seem to be
related.

Regards,

Elias

[1] <http://permalink.gmane.org/gmane.emacs.gnus.general/65156>
[2] <http://permalink.gmane.org/gmane.emacs.gnus.general/65198>
[3] <http://permalink.gmane.org/gmane.emacs.devel/82665>




^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch] Make tls.el support certificate verification
  2007-11-08 18:44       ` Elias Oltmanns
@ 2007-11-08 19:52         ` Reiner Steib
  2007-11-16 17:22           ` Elias Oltmanns
  0 siblings, 1 reply; 16+ messages in thread
From: Reiner Steib @ 2007-11-08 19:52 UTC (permalink / raw)
  To: Elias Oltmanns; +Cc: ding, emacs-devel

On Thu, Nov 08 2007, Elias Oltmanns wrote:

> Simon Josefsson <simon@josefsson.org> wrote:
>> Let's wait for copyright papers and then apply the patch in both
>> CVS's.
>
> Papers have been signed and receipt has been acknowledged.  Please
> apply.

Thanks.

> Could someone please look at [1] and [2] as well?  These patches are bug
> fixes and recent bug reports wrt agentised servers (see [3]) seem to be
> related.

Could you please provide ChangeLog entries for these patches?

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch] Make tls.el support certificate verification
  2007-11-08 19:52         ` Reiner Steib
@ 2007-11-16 17:22           ` Elias Oltmanns
  2007-11-16 22:38             ` Reiner Steib
  0 siblings, 1 reply; 16+ messages in thread
From: Elias Oltmanns @ 2007-11-16 17:22 UTC (permalink / raw)
  To: emacs-devel; +Cc: ding

Hi Reiner,

Reiner Steib <reinersteib+gmane@imap.cc> wrote:
> On Thu, Nov 08 2007, Elias Oltmanns wrote:
>
>> Simon Josefsson <simon@josefsson.org> wrote:
>>> Let's wait for copyright papers and then apply the patch in both
>>> CVS's.
>>
>> Papers have been signed and receipt has been acknowledged.  Please
>> apply.
>
> Thanks.
>
>> Could someone please look at [1] and [2] as well?  These patches are bug
>> fixes and recent bug reports wrt agentised servers (see [3]) seem to be
>> related.
>
> Could you please provide ChangeLog entries for these patches?

Well, I sent ChangeLog entries a week ago (see [1] and [2]) but nothing
has shown up in cvs yet.  The same applies to my tls.el patch although
I'm probably to be blamed for that since I haven't provided a ChangeLog
entry for that one yet.  So, here it goes:

--8<---------------cut here---------------start------------->8---
* tls.el: Check certificates against trusted root certificates.  Also,
provide an option to check if GNU TLS complained about a mismatch
between the hostname provided in the certificate and the name of the
host connnecting to.  New (customizable) variables are: tls-checktrust,
tls-untrusted, tls-hostmismatch.
--8<---------------cut here---------------end--------------->8---

BTW: It has happened several times in the past that messages I sent to
the ding list went (seemingly) unnoticed.  This is particularly annoying
if the message actually contains a ready made patch to fix a bug and all
I'm asking for is to review the patch and tell me what's wrong with it
so it can be committed eventually.  Curiously enough, the message I
finally got a response to (the one that started this thread) was about
adding a new feature rather than fixing a bug in existing code.  It also
strikes me that this message went to both, the ding list as well as
emacs-devel.  This makes me wonder whether I should generally send
patches to the emacs-devel list rather than the ding list even if they
concern the gnus trunk.  Or should I just Cc one of the Gnus developers
instead?  In that case, is there a source where I can see who is
maintaining which part of Gnus?

Regards,

Elias

[1] <http://permalink.gmane.org/gmane.emacs.gnus.general/65609>
[2] <http://permalink.gmane.org/gmane.emacs.gnus.general/65611>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch] Make tls.el support certificate verification
  2007-11-16 17:22           ` Elias Oltmanns
@ 2007-11-16 22:38             ` Reiner Steib
  2007-11-16 23:07               ` Elias Oltmanns
  0 siblings, 1 reply; 16+ messages in thread
From: Reiner Steib @ 2007-11-16 22:38 UTC (permalink / raw)
  To: Elias Oltmanns; +Cc: ding, emacs-devel

On Fri, Nov 16 2007, Elias Oltmanns wrote:

> Well, I sent ChangeLog entries a week ago (see [1] and [2]) but nothing
> has shown up in cvs yet.  

Sorry for the delay, I didn't have a change do commit your changes
during the past week (I'm not at home and I don't have a reasonable
internet connection at the moment).

> BTW: It has happened several times in the past that messages I sent to
> the ding list went (seemingly) unnoticed.  

I have read your messages.  For tls.el, I hoped that Simon or someone
else can comment as I'm not familiar with it.  Similar for your agent
related patches (I don't even use the agent).

> This is particularly annoying if the message actually contains a
> ready made patch to fix a bug and all I'm asking for is to review
> the patch and tell me what's wrong with it so it can be committed
> eventually.  

If you don't get responses, please feel free to remind us about the
issue.  (Before your papers had been arrived, we couldn't install your
patches.)

> Curiously enough, the message I finally got a response to (the one
> that started this thread) was about adding a new feature rather than
> fixing a bug in existing code.  It also strikes me that this message
> went to both, the ding list as well as emacs-devel.

I don't think there is a strong correlation.

> This makes me wonder whether I should generally send patches to the
> emacs-devel list rather than the ding list even if they concern the
> gnus trunk.  

Now that the Gnus trunk is also in Emacs trunk you may send it
emacs-devel in this case as well.  But please keep ding copied too.

> Or should I just Cc one of the Gnus developers instead?  In that
> case, is there a source where I can see who is maintaining which
> part of Gnus?

You may Cc the maintainer listed in the file.

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch] Make tls.el support certificate verification
  2007-11-16 22:38             ` Reiner Steib
@ 2007-11-16 23:07               ` Elias Oltmanns
  2007-11-24 21:31                 ` Reiner Steib
  0 siblings, 1 reply; 16+ messages in thread
From: Elias Oltmanns @ 2007-11-16 23:07 UTC (permalink / raw)
  To: emacs-devel; +Cc: ding

Reiner Steib <reinersteib+gmane@imap.cc> wrote:
> On Fri, Nov 16 2007, Elias Oltmanns wrote:
>
>> Well, I sent ChangeLog entries a week ago (see [1] and [2]) but nothing
>> has shown up in cvs yet.  
>
> Sorry for the delay, I didn't have a change do commit your changes
> during the past week (I'm not at home and I don't have a reasonable
> internet connection at the moment).

Alright.  Thanks for clarifying.

[...]
>> This is particularly annoying if the message actually contains a
>> ready made patch to fix a bug and all I'm asking for is to review
>> the patch and tell me what's wrong with it so it can be committed
>> eventually.  
>
> If you don't get responses, please feel free to remind us about the
> issue.  (Before your papers had been arrived, we couldn't install your
> patches.)

Yes, I realise that and that's why I had hoped for a more timely
response to my ChangeLog entries since I had posted them after papers
had been signed.  Of course, there always is a chance for unfortunate
coincidences, so, thanks for your explanation.

[...]
>> This makes me wonder whether I should generally send patches to the
>> emacs-devel list rather than the ding list even if they concern the
>> gnus trunk.  
>
> Now that the Gnus trunk is also in Emacs trunk you may send it
> emacs-devel in this case as well.  But please keep ding copied too.

Actually, I'd prefer the ding list myself since it has much lower
traffic than emacs-devel.

Regards,

Elias

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch] Make tls.el support certificate verification
  2007-11-16 23:07               ` Elias Oltmanns
@ 2007-11-24 21:31                 ` Reiner Steib
  2007-11-25  0:35                   ` Elias Oltmanns
  2007-11-27 11:10                   ` Elias Oltmanns
  0 siblings, 2 replies; 16+ messages in thread
From: Reiner Steib @ 2007-11-24 21:31 UTC (permalink / raw)
  To: Elias Oltmanns; +Cc: emacs-devel, ding

On Sat, Nov 17 2007, Elias Oltmanns wrote:

> Reiner Steib <reinersteib+gmane@imap.cc> wrote:
>> Sorry for the delay, I didn't have a change do commit your changes
>> during the past week (I'm not at home and I don't have a reasonable
>> internet connection at the moment).
>
> Alright.  Thanks for clarifying.

Committed.  Thank you for your contributions.

Some remarks for future contributions:

- The  ChangeLog entries should be per function / per variable, e.g.:

	* tls.el (tls-certtool-program, tls-hostmismatch): New variables.
	(tls-checktrust): New variable.  Check if GNU TLS complained about a
	mismatch between the hostname provided in the certificate and the name
	of the host connnecting to.
	(open-tls-stream): Use them.  Check certificates against trusted root
	certificates.

- defcustoms don't need the leading "*" in the doc string.  (In many
  Emacs lisp files they still exist, but they will be removed
  eventually.)

- defcustoms should have a version tag.

See http://article.gmane.org/gmane.emacs.gnus.commits/5529 for my
cosmetic/style changes.

Would it be useful to add the strings suggested in the doc string of
`tls-checktrust' to `tls-program'?  Or provide them as custom options
for `tls-program'?

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch] Make tls.el support certificate verification
  2007-11-24 21:31                 ` Reiner Steib
@ 2007-11-25  0:35                   ` Elias Oltmanns
  2007-11-25 14:18                     ` Reiner Steib
  2007-11-27 11:10                   ` Elias Oltmanns
  1 sibling, 1 reply; 16+ messages in thread
From: Elias Oltmanns @ 2007-11-25  0:35 UTC (permalink / raw)
  To: emacs-devel; +Cc: ding

Reiner Steib <reinersteib+gmane@imap.cc> wrote:
[...]
> Some remarks for future contributions:
[...]

Thanks for those hints.

>
> See http://article.gmane.org/gmane.emacs.gnus.commits/5529 for my
> cosmetic/style changes.

Unfortunately, this link seems to be a dead end.

>
> Would it be useful to add the strings suggested in the doc string of
> `tls-checktrust' to `tls-program'?  Or provide them as custom options
> for `tls-program'?

Well, I wasn't quite sure about it at the time and I'm not any wiser
yet.  I'm using the examples given in the doc string in a Debian
environment but they need not work properly for other distributions or
OSes.  In fact, I think it is so hard to come up with sensible default
values that are actually worth making the effort that the best GNU
developers can do is to provide the facilities and sufficient
documentation to make use of them.  Distributors may or may not tweak
the default settings and give further advice to their users but even
they shouldn't enable tls-checktrust by default as this really should be
a decision consciously taken by the end user.  After all, the mail
server needn't have a certificate signed by one of the well known CAs
and may still be valid.  Besides, users might want to specify the set of
trusted root certificates depending on the server emacs is connecting
to.  All this seems to make proper documentation more important than
presetting any defaults.  Do you think the provided doc strings can
serve this purpose or should I squeeze in a few sentences somewhere
else?

Regards,

Elias

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch] Make tls.el support certificate verification
  2007-11-25  0:35                   ` Elias Oltmanns
@ 2007-11-25 14:18                     ` Reiner Steib
  2007-11-26 14:47                       ` Simon Josefsson
  0 siblings, 1 reply; 16+ messages in thread
From: Reiner Steib @ 2007-11-25 14:18 UTC (permalink / raw)
  To: Elias Oltmanns; +Cc: Simon Josefsson, ding, emacs-devel

On Sun, Nov 25 2007, Elias Oltmanns wrote:

> Reiner Steib <reinersteib+gmane@imap.cc> wrote:
>> See http://article.gmane.org/gmane.emacs.gnus.commits/5529 for my
>> cosmetic/style changes.
>
> Unfortunately, this link seems to be a dead end.

The web interface has some problems at the moment.  I'm sure it will
work again soon (i.e. when Lars fixes the problem).

In Gnus you may use these
<nntp://news.gmane.org/gmane.emacs.gnus.commits/5529>,
<news:E1Iw2Sy-0003hx-00@quimby.gnus.org>
(cf. `gnus-refer-article-method').

>> Would it be useful to add the strings suggested in the doc string of
>> `tls-checktrust' to `tls-program'?  Or provide them as custom options
>> for `tls-program'?
>
> Well, I wasn't quite sure about it at the time and I'm not any wiser
> yet.  I'm using the examples given in the doc string in a Debian
> environment but they need not work properly for other distributions or
> OSes.  

On openSUSE, I have the directory, but no file ca-certificates.crt:

$ cat /etc/SuSE-release 
openSUSE 10.2 (i586)
VERSION = 10.2
$ ls -F /etc/ssl/certs/
1e49180d.0@  843b6c51.0@  d4e39186.0@  Equifax-root1.pem  vsign1.pem
2edf7016.0@  878cf4c6.0@  ddc328ff.0@  expired/           vsign3.pem
56e607f4.0@  a3c60019.0@  demo/        f73e89fd.0@        vsignss.pem
594f1775.0@  aad3d04d.0@  eng1.pem     ICP-Brasil.pem     wellsfgo.pem
6adf0799.0@  argena.pem   eng2.pem     RegTP-5R.pem
6f5d9899.0@  argeng.pem   eng3.pem     RegTP-6R.pem
7651b327.0@  c33a80d4.0@  eng4.pem     thawteCb.pem
7a9820c1.0@  cdd7aee7.0@  eng5.pem     thawteCp.pem

> In fact, I think it is so hard to come up with sensible default
> values that are actually worth making the effort that the best GNU
> developers can do is to provide the facilities and sufficient
> documentation to make use of them.  Distributors may or may not
> tweak the default settings and give further advice to their users
> but even they shouldn't enable tls-checktrust by default as this
> really should be a decision consciously taken by the end user.

I think it makes sense to add them as customize choices (committed to
Gnus trunk):

	  ;; FIXME: add brief `:tag "..."' descriptions.
	  ;; See `tls-checktrust':
	  (const "gnutls-cli --x509cafile /etc/ssl/certs/ca-certificates.crt -p %p %h")
	  (const "gnutls-cli --x509cafile /etc/ssl/certs/ca-certificates.crt -p %p %h --protocols ssl3")
	  (const "openssl s_client -connect %h:%p -CAfile /etc/ssl/certs/ca-certificates.crt -no_ssl2")
	  ;; No trust check:
	  (const "gnutls-cli -p %p %h")
	  (const "gnutls-cli -p %p %h --protocols ssl3")
	  (const "openssl s_client -connect %h:%p -no_ssl2"))

Is this the right order?  Would someone like to add brief `:tag "..."'
descriptions for each entry?

> After all, the mail server needn't have a certificate signed by one
> of the well known CAs and may still be valid.  Besides, users might
> want to specify the set of trusted root certificates depending on
> the server emacs is connecting to.  All this seems to make proper
> documentation more important than presetting any defaults.  Do you
> think the provided doc strings can serve this purpose or should I
> squeeze in a few sentences somewhere else?

As tls.el is not specific to Gnus (it is in lisp/net in Emacs 22), it
should probably not be documented in gnus.texi, maybe in
smtpmail.texi, e.g. (info "(smtpmail)Authentication").

If there's no suitable manual, probably the best places are the doc
strings of `tls-program' and/or `tls-checktrust'.

Simon, any suggestion?  Opinions?

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch] Make tls.el support certificate verification
  2007-11-25 14:18                     ` Reiner Steib
@ 2007-11-26 14:47                       ` Simon Josefsson
  0 siblings, 0 replies; 16+ messages in thread
From: Simon Josefsson @ 2007-11-26 14:47 UTC (permalink / raw)
  To: Elias Oltmanns; +Cc: ding, emacs-devel

Reiner Steib <reinersteib+gmane@imap.cc> writes:

>> After all, the mail server needn't have a certificate signed by one
>> of the well known CAs and may still be valid.  Besides, users might
>> want to specify the set of trusted root certificates depending on
>> the server emacs is connecting to.  All this seems to make proper
>> documentation more important than presetting any defaults.  Do you
>> think the provided doc strings can serve this purpose or should I
>> squeeze in a few sentences somewhere else?
>
> As tls.el is not specific to Gnus (it is in lisp/net in Emacs 22), it
> should probably not be documented in gnus.texi, maybe in
> smtpmail.texi, e.g. (info "(smtpmail)Authentication").

Gnus isn't the best place to document it, but I'm not sure smtpmail is
better.  Will it be found by searching through the normal emacs manuals?
Maybe even writing a new tls.texi to discuss all TLS related stuff is
warranted.  I don't know what the policy on new *.texi files is, if we
have too many already or whether having one per elisp package is
actually a good idea.

/Simon

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch] Make tls.el support certificate verification
  2007-11-24 21:31                 ` Reiner Steib
  2007-11-25  0:35                   ` Elias Oltmanns
@ 2007-11-27 11:10                   ` Elias Oltmanns
  2007-11-28 22:05                     ` Reiner Steib
  2007-11-28 22:08                     ` Coding conventions (was: [Patch] Make tls.el support certificate verification) Reiner Steib
  1 sibling, 2 replies; 16+ messages in thread
From: Elias Oltmanns @ 2007-11-27 11:10 UTC (permalink / raw)
  To: emacs-devel; +Cc: ding

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

Reiner Steib <reinersteib+gmane@imap.cc> wrote:
> Committed.  Thank you for your contributions.

I am very sorry to say that I've just discovered a rather awkward
mistake in my patch.  The variable tls-checktrust isn't even referenced
anywhere, hence only parts of the advertised functionality are provided
so far.  Please find attached a patch to fix this issue.

I really am sorry for making all the fuss and then not getting it right
in the first place.

>
> Some remarks for future contributions:
[...]
> See http://article.gmane.org/gmane.emacs.gnus.commits/5529 for my
> cosmetic/style changes.

In the cvs trunk I can see that you made some adjustments to line breaks
as well.  What is the maximum line length in doc strings?  Also, is
there a comprehensive source for information about Emacs and Gnus
codingstyle and good practice?  And what is the canonical way to provide
ChangeLog entries if I have no commit privileges or, to put it another
way, the attached patch alright in this respect?

Regards,

Elias


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: tls.patch --]
[-- Type: text/x-patch, Size: 1544 bytes --]

Index: lisp/ChangeLog
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/ChangeLog,v
retrieving revision 7.1674
diff -u -r7.1674 ChangeLog
--- lisp/ChangeLog	25 Nov 2007 18:25:12 -0000	7.1674
+++ lisp/ChangeLog	27 Nov 2007 11:02:51 -0000
@@ -1,3 +1,9 @@
+2007-11-27  Elias Oltmanns  <eo@nebensachen.de>
+
+	* tls.el: (open-tls-stream): Actually consult tls-checktrust to see if
+	certs should be verified and what is to be done in the event of a
+	verification failure.
+
 2007-11-25  Romain Francoise  <romain@orebokech.com>
 
 	* gnus-msg.el (gnus-summary-reply): Delete extra paren.
Index: lisp/tls.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/tls.el,v
retrieving revision 7.18
diff -u -r7.18 tls.el
--- lisp/tls.el	25 Nov 2007 14:17:03 -0000	7.18
+++ lisp/tls.el	27 Nov 2007 11:02:52 -0000
@@ -229,12 +229,15 @@
 	(set-buffer buffer)
 	(when
 	    (or
-	     (and tls-untrusted
+	     (and tls-checktrust
 		  (progn
 		    (goto-char (point-min))
 		    (re-search-forward tls-untrusted nil t))
-		  (not (yes-or-no-p
-			(format "The certificate presented by `%s' is NOT trusted. Accept anyway? " host))))
+		  (or 
+		   (and (not (eq tls-checktrust 'ask))
+			(message "The certificate presented by `%s' is NOT trusted." host))
+		   (not (yes-or-no-p
+			 (format "The certificate presented by `%s' is NOT trusted. Accept anyway? " host)))))
 	     (and tls-hostmismatch
 		  (progn
 		    (goto-char (point-min))

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch] Make tls.el support certificate verification
  2007-11-27 11:10                   ` Elias Oltmanns
@ 2007-11-28 22:05                     ` Reiner Steib
  2007-11-28 22:08                     ` Coding conventions (was: [Patch] Make tls.el support certificate verification) Reiner Steib
  1 sibling, 0 replies; 16+ messages in thread
From: Reiner Steib @ 2007-11-28 22:05 UTC (permalink / raw)
  To: Elias Oltmanns; +Cc: ding, emacs-devel

On Tue, Nov 27 2007, Elias Oltmanns wrote:

> The variable tls-checktrust isn't even referenced anywhere, hence
> only parts of the advertised functionality are provided so far.
> Please find attached a patch to fix this issue.

Committed.  Thanks.

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Coding conventions (was: [Patch] Make tls.el support certificate verification)
  2007-11-27 11:10                   ` Elias Oltmanns
  2007-11-28 22:05                     ` Reiner Steib
@ 2007-11-28 22:08                     ` Reiner Steib
  1 sibling, 0 replies; 16+ messages in thread
From: Reiner Steib @ 2007-11-28 22:08 UTC (permalink / raw)
  To: Elias Oltmanns; +Cc: ding, emacs-devel

On Tue, Nov 27 2007, Elias Oltmanns wrote:

>> See http://article.gmane.org/gmane.emacs.gnus.commits/5529 for my
>> cosmetic/style changes.
>
> In the cvs trunk I can see that you made some adjustments to line breaks
> as well.  What is the maximum line length in doc strings?  

The first line should be short enough to fit into an apropos buffer
(with a width of 80 columns).  As for the other lines, I simply hit
M-q (with Emacs' default settings).  `M-x checkdoc RET' helps to fix
common mistakes for doc string and other style issues.

> Also, is there a comprehensive source for information about Emacs
> and Gnus codingstyle and good practice?  

For Gnus: `texi/gnus-coding.texi'.  As Gnus is part of Emacs, we try
to follow the conventions for Emacs.  E.g. (info "(elisp)Tips"), (info
"(elisp)Coding Conventions").  I don't think there's a comprehensive
source.

> And what is the canonical way to provide ChangeLog entries if I have
> no commit privileges or, to put it another way, the attached patch
> alright in this respect?

As the ChangeLog is modified very often, patches usually don't apply
properly.  Therefore it's better to provide the ChangeLog directly in
the message, e.g. ...

--8<---------------cut here---------------start------------->8---
2007-11-27  Elias Oltmanns  <eo@nebensachen.de>

	* tls.el (open-tls-stream): Actually consult tls-checktrust to see if
	certs should be verified and what is to be done in the event of a
	verification failure.
--8<---------------cut here---------------end--------------->8---

Note that the colon after "tls.el" was not correct.

Then the committer can simply Copy&Paste it and adjust the date.

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2007-11-28 22:08 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-09-16 23:08 [Patch] Make tls.el support certificate verification Elias Oltmanns
2007-09-24  7:12 ` Simon Josefsson
2007-09-24 16:27   ` Reiner Steib
2007-09-25 14:42     ` Simon Josefsson
2007-11-08 18:44       ` Elias Oltmanns
2007-11-08 19:52         ` Reiner Steib
2007-11-16 17:22           ` Elias Oltmanns
2007-11-16 22:38             ` Reiner Steib
2007-11-16 23:07               ` Elias Oltmanns
2007-11-24 21:31                 ` Reiner Steib
2007-11-25  0:35                   ` Elias Oltmanns
2007-11-25 14:18                     ` Reiner Steib
2007-11-26 14:47                       ` Simon Josefsson
2007-11-27 11:10                   ` Elias Oltmanns
2007-11-28 22:05                     ` Reiner Steib
2007-11-28 22:08                     ` Coding conventions (was: [Patch] Make tls.el support certificate verification) Reiner Steib

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).