[Bug 29018] Allow interactive TLS certificate verification

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=29018

Simon McVittie <simon.mcvittie at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |http://people.collabora.co.
                   |                            |uk/~cosimoc/tls-connection-
                   |                            |spec/
             Status|NEW                         |ASSIGNED
           Keywords|                            |patch
         AssignedTo|telepathy-bugs at lists.freede |cosimoc at gnome.org
                   |sktop.org                   |

--- Comment #6 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-07-29 06:08:31 PDT ---
Commit 2c33eaae4 affects Connection API, but it is indeed an obvious copypasta
fix. Please cherry-pick it into master, Reviewed-by: me.

The API seems the right shape.

Authn.TLSCertificate
====================

While we're on the subject of TLS, could you answer the question I asked in
<https://bugs.freedesktop.org/show_bug.cgi?id=23931#c6> (whether Limit_Exceeded
and Insecure should be enum members or not), then we can add Limit_Exceeded and
Insecure codes? TLS_Certificate_Reject_Reason contains a new member
Insecure_Algorithm that doesn't correspond to a D-Bus error name yet; we should
unify the naming one way or the other.

I'm not sure that TLS_Certificate_Reject_Reason needs both None and Other;
what's the difference between them? Could they usefully be merged, perhaps with
the merged version called Unknown? I think "Unknown" should be numerically
zero.

Either the methods should gain a Certificate prefix, or the signals should lose
it. I'd vote for the signals losing their prefix: the usual namespace-collision
reasons don't apply here, because the TLSCertificate is an independent object.

CertificateVerified() should be [Certificate]Accepted(): the user could accept
a certificate about which nothing has been verified (leap of faith), and the
naming should be consistent with Accept(). Similarly,
TLS_Certificate_State_Verified should be ..._Accepted.

The Accepted signal should be documented as "The _State_ of this certificate
has changed to Accepted", with a hyperlink (<tp:member-ref>) to the property.

The Rejected signal should be documented with a hyperlink to State too. The
Rejected signal's Reason argument should be documented as "The new value of
_RejectReason_", with a hyperlink.

RejectReason should have rationale, defined behaviour in non-Rejected states,
and a hyperlink to State, something like this:

    RejectReason: u (TLS_Certificate_Reject_Reason)
        If the _State_ is Rejected, the reason why the certificate was
        rejected.

        | Clients that do not understand the _RejectError_, which may be
        | implementation-specific, can use this property to classify
        | rejection reasons into common categories.

        Otherwise, this property is not meaningful, and SHOULD be set to
        Unknown.

If we want clients to be able to state-recover the rejection reason, there
should be RejectError and RejectDetails properties:

    RejectError: s (DBus_Error_Name)
        If the _State_ is Rejected, the reason why the certificate
        was rejected; this MAY correspond to the _RejectReason_, or MAY
        be a more specific D-Bus error name, perhaps implementation-specific.

        If the State is not Rejected, this property is not meaningful,
        and SHOULD be set to an empty string.

    RejectDetails: a{sv} (String_Variant_Map)
        If the _State_ is Rejected, additional information about why the
        certificate was rejected.

        If the State is not Rejected, this property is not meaningful,
        and SHOULD be set to an empty map.

and the arguments to the Rejected signal should be documented as "The new value
of _Foo_". If we don't care about state recovery for some reason, it would be
consistent to delete the RejectReason property too.

If the RejectError, RejectDetails properties are added, then I'd be inclined to
document the parameters to Reject like "The new value of _RejectError_", and
move the list of well-known RejectDetails keys to the RejectDetails
documentation.

There should be a "debug-message" detail, the same as for Connection.

In "user-requested", instead of "Whether the error was...", I'd prefer "True if
the error was...". "rejectal" should be "rejection" and "opposed to" should be
"as opposed to" (but I'd prefer it worded as "...rejection of the certificate;
False if there was an unrecoverable error..." anyway).

If we're going to define "expected-hostname", I think it should be more like
this:

    expected-hostname (s)
        If the rejection reason is Hostname_Mismatch, the hostname that the
        server was expected to have.

    certificate-hostname (s)
        If the rejection reason is Hostname_Mismatch, the hostname of the
        certificate that was presented.

        | For instance, if you try to connect to gmail.com but are presented
        | with a TLS certificate issued to evil.example.org, the error
        | details for Hostname_Mismatch SHOULD include:
        |
        | { "expected-hostname": "gmail.com",
        |   "certificate-hostname": "evil.example.org" }

Ideally, Connection.ConnectionError should also include these:

    expected-hostname (s), certificate-hostname (s)
        The same details defined in _TLSCertificate.DRAFT.RejectDetails_

and the description of HostnameMismatch in errors.xml should refer to them too.

TLS_Certificate_State_None would perhaps make more sense as
TLS_Certificate_State_Pending?

> The RAW PEM-encoded trust chain of this TLS certificate. 

RAW isn't an acronym, I think you mean "raw". I don't think non-X.509
certificates can be PEM-encoded, so this doesn't really make sense; we should
specify an encoding for PGP keys, perhaps like this?

    Certificate_Data - ay
        The raw data contained in a TLS certificate.

        For X.509 certificates (_CertificateType_ = "x509"), this MUST be in
        PEM format (Base64-encoded DER enclosed in
        "-----BEGIN CERTIFICATE-----" and "-----END CERTIFICATE-----" markers.

        For PGP certificates (_CertificateType_ = "pgp"), this MUST be a
        binary OpenPGP key as defined by section 11.1 of _RFC 4880_.

    CertificateChainData - aay (Certificate_Data[])
        One or more TLS certificates forming a trust chain, each encoded as
        specified by _Certificate_Data_.

Would DER or ASN.1 be a better form for X.509 certificates? The PEM format
seems rather poorly-specified (I couldn't find a good reference for it), but if
it's the de facto standard and things typically understand it better than DER,
keep it. I'd prefer to be able to cite chapter and verse of an RFC for this
sort of thing, though.

Chan.T.ServerTLSConnection
==========================

> Channels of this kind are never requested, are anonymous

"always have _Requested_ = False, _TargetHandleType_ = None and _TargetHandle_
= 0, and cannot be requested with methods such as _CreateChannel_" with
hyperlinks, please.

> MAY just close the channel

Please hyperlink to Close.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.