Re: The netsec thread

[Jimmy Yuen Ho Wong <wyuenho@gmail.com>]

>> +** New function 'network-lookup-address-info'.
>> +This does IPv4 and/or IPv6 address lookups on hostnames.
>
> I'm assuming this will be removed again since we got the new getaddrinfo
> function?
>

Yep.

>>    "List of CA bundle location filenames or a function returning said list.
>> +If a file path contains glob wildcards, they will be expanded.
>
> Hm.  This seems like a good idea, but do we do this in other similar
> variables?  And would perhaps regexp syntax make more sense than glob
> syntax?
>

Nah. This has already been looked at weeks ago, no need to
over-engineer it further as I believe most people will think of glob
when dealing with file paths. Besides, Emacs's regex isn't
particularly nice unless Dan elects to retrofit PCRE into Emacs :P

The reason I need glob is IGTF's fetch-crl will put ~100s CRL PEM
files into the file system, it's very cumbersome to specify them
1-by-1.

>> -;;;###autoload
>> -(defcustom gnutls-min-prime-bits 256
>> -  ;; Several mail servers send fewer bits than the GnuTLS default.
>> -  ;; Currently, 256 appears to be a reasonable choice (Bug#11267).
>> +(defcustom gnutls-min-prime-bits nil
>
> As I've said before, I don't think this makes much sense.  But in any
> case, the variable is obsolescent (since GnuTLS has said so), so perhaps
> we should just mark it obsolete and tell people to use the priority
> string to control these things.
>

Sorry I got lost in that giant thread.
`gnutls_dh_set_prime_bits` is only deprecated on GnuTLS 3.1.7+. Are we
dropping support for all version < 3.1.7? I'd be super happy to do it
if that's the case and remove this var and the C code entirely.

>> -`low': Absolutely no checks are performed.
>> -`medium': This is the default level, should be reasonable for most usage.
>> -`high': This warns about additional things that many people would
>> -not find useful.
>> -`paranoid': On this level, the user is queried for most new connections.
>> +`low': Check for problems known before Edward Snowden.
>> +`medium': Default.  Suitable for most circumstances.
>> +`high': Warns about additional issues not enabled in `medium' due to
>> +compatibility concerns.
>
> I don't think it makes much sense to talk about Snowden as if that's
> something people are meant to understand.

I'd be happy to, but I have no idea what 'low means anymore. In fact,
if you think about impact as a 2-dimensional variable as opposed to
this linear scale, you'd have a quadrant defined by "impact" and
"compatibility". Hi-impact-hi-compat should be enabled without
question, which roughly corresponds to 'low. Hi-impact-lo-compat and
Lo-impact-hi-compat do not exist as a concept in NSM ATM.
Lo-impact-lo-compat is 'paranoid? But what would be in it and why
bother?

> And, as I've said before,
> `paranoid' should stay.
>

Eli's use case has already been taken cared of by
`nsm-trust-local-network`. `paranoid has been aliased to `high for
backward compatibility.

Robert do you still object to removing the `paranoid level? I've
removed that prompt that askes for permission on every TLS connection
due to crying-wolf effect.

If there isn't an objection from people who've found use for it, I'd
really like to try without 'paranoid on master later before declaring
it insufficient.


> Calling protocol checks "TLS" checks isn't future proof.  We've
> already had one politically motivated name change (from SSL to TLS)
> and we may have another.  And besides, many of these checks are also
> valid for SSL, so it's just confusing.
>

The TLS working group wasn't even willing to call TLS 1.3[1] TLS 2.0
even when it's a major departure from it. I doubt we need to worry
about extra work to change a name. YAGNI applies.

[1]: https://www.ietf.org/mail-archive/web/tls/current/msg20938.html

> Call them `nsm-protocol-check' and stick the -- back in.  And having
> the entire function name instead of just the bit after "check--" makes
> it more tedious for people to remove/add their own functions.
>

`nsm-tls-checks` is already a defcustom. It's super easy to add and
remove a function. You can defun whatever name you want and add to it,
and click [-] to remove. Using name mangling magic to fish out a check
function makes defcustom super-awkward, and AFAIK, no other emacs core
setting does it this way.

>> +(defun nsm-should-check (host)
>> +  "Determines whether NSM should check for TLS problems for HOST.
>> +
>> +If `nsm-trust-local-network' is or returns non-nil, and if the
>> +host address is a localhost address, a machine address, a direct
>> +link or a private network address, this function returns
>> +nil.  Non-nil otherwise."
>
> What do you mean by "machine address"?  The MAC address?  If you mean
> IP address, it's perfectly valid to have TLS on a non-named IP
> address.  1.0.0.1 does that for DNS over HTTPS last I heard, and
> that's definitely a service you should verify, well, everything on.
>

I mean 0.0.0.0/8. I'm not sure what the proper name is or if I even
need to deal with it. What do you think?

>
>> +(defun nsm-tls-check-rsa-kx (host port status &optional settings)
>
> In Emacs functions we try to avoid abbreviations unless they're very
> common.  kx is too obscure; say key-exchange instead.
>

Will do.

>> +Reference:
>> +
>> +Sheffer, Holz, Saint-Andre (May 2015).  \"Recommendations for Secure
>> +Use of Transport Layer Security (TLS) and Datagram Transport Layer
>> +Security (DTLS)\", \"(4.1.  General Guidelines)\"
>> +`https://tools.ietf.org/html/rfc7525\#section-4.1'"
>
> [...]
>
>> +GnuTLS authors (2018). \"GnuTLS Manual 4.3.3 Anonymous
>> +authentication\",
>> +`https://www.gnutls.org/manual/gnutls.html\#Anonymous-authentication'"
>
> Heh heh.  I like all the references, but perhaps it's a lot of URLs to
> keep updated?  Perhaps not?
>

These provide justification for the checks and make it easy for
everyone to fact-check my rationale. Most of them are just RFC URLs,
they never fail to resolve.

>> +          (let* ((accept-choices '((?a "always" "Accept this certificate this session and for all future sessions.")
>> +                                   (?s "session only" "Accept this certificate this session only.")
>> +                                   (?n "no" "Refuse to use this certificate, and close the connection.")
>> +                                   (?d "details" "See certificate details")))
>> +                 (details-choices '((?b "backward page" "See previous page")
>> +                                    (?f "forward page" "See next page")
>> +                                    (?n "next" "Next certificate")
>> +                                    (?p "previous" "Previous certificate")
>> +                                    (?q "quit" "Quit details view")))
>
> See previous messages about the UI.
>

Will update that later.


Thanks for reviewing!