Re: [Trans] AD Review: draft-ietf-trans-rfc6962-bis-26.txt

[Eric Rescorla <[e-mail]>]

On Thu, Sep 7, 2017 at 3:24 AM, Ben Laurie <[e-mail]> wrote:

>
>
> On 5 September 2017 at 18:46, Eric Rescorla <[e-mail]> wrote:
>
>>
>>
>> On Tue, Sep 5, 2017 at 9:04 AM, Ben Laurie <[e-mail]> wrote:
>>
>>>
>>>
>>> On 4 September 2017 at 00:17, Eric Rescorla <[e-mail]> wrote:
>>>
>>>> Hi folks,
>>>>
>>>> Please find enclosed the first cut of my AD review of this draft.
>>>>
>>>> Note: the original of this review is on Phabricator at:
>>>>
>>>>   https://mozphab-ietf.devsvcdev.mozaws.net/D13
>>>>
>>>> If you want to see comments in context -- which is a lot easier -- you
>>>> can go there. Also, you can create an account and respond inline if
>>>> you like.  If you elect to, let me know if you run into problems.
>>>>
>>>> -Ekr
>>>>
>>>>
>>>> Note: I have not yet reviewed the algorithms in S 2.1. I plan to do
>>>> that separately, but figured it would be useful to provide the rest of
>>>> my review on the assumption that the changes to that section will be
>>>> modest if any.
>>>>
>>>>
>>>> High-Level:
>>>>
>>>> 1. This document makes a variety of claims about the assurances that
>>>> clients get that only obtain if some as-yet-to-be-specified
>>>> third-party verifiability mechanism is implemented. For instance, in
>>>> the intro:
>>>>
>>>>   Certificate Transparency aims to mitigate the problem of misissued
>>>>   certificates by providing append-only logs of issued certificates.
>>>>   The logs do not need to be trusted because they are publicly
>>>>   auditable.
>>>>
>>>>
>>>> As the extensive discussion following Richard Barnes's and my previous
>>>> comments should make clear, this is only a property of CT if you also
>>>> have some mechanism for third-party verifiability of STHs, and this
>>>> document does not supply that. In the actually deployed -- we can
>>>> debate deployable separately the deployability of some of the
>>>> proposals for how to get this-- versions of CT, what clients get is
>>>> SCTs, which are effectively countersignatures and in fact do require
>>>> trusting the logs. This is implicitly acknowledged by proposals that
>>>> RPs only accept certificates with >1 SCT.
>>>>
>>>
>>> The purpose of multiple SCTs is to avoid the death of a single log
>>> causing the death of a large number of certificates. It is not about trust.
>>>
>>
>> That seems like the reason for the server to offer it, but not for the
>> client to require it.
>>
>> Line 763
>>>>    Maximum Chain Length:  The longest chain submission the log is
>>>>       willing to accept, if the log chose to limit it.
>>>> Nit: chooses
>>>>
>>>
>>> Past tense seems correct?
>>>
>>
>> I'm willing to dert this to the RFC Editor.
>>
>>
>>>
>>>
>>>>
>>>>
>>>> Line 785
>>>>    accepted trust anchor, using only the chain of intermediate CA
>>>>    certificates provided by the submitter.
>>>> Why is this a 2119 MUST? It seems wise, but not necessarily a
>>>> conformance requirement
>>>>
>>>
>>> "To avoid being overloaded by invalid submissions"
>>>
>>
>> There seem like one way to prevent this, but not the only one.
>>
>>
>>
>>> Line 816
>>>>    anchor used to verify the chain (even if it was omitted from the
>>>>    submission).  The log MUST present this chain for auditing upon
>>>>    request (see Section 5.6).  This prevents the CA from avoiding blame
>>>> What happens in cases of multiple chains. For instance, say that the
>>>> submitter provides superfluous certificates?
>>>>
>>>
>>> Not allowed.
>>>
>>
>> Hmm...  So your theory is that the submitter does path construction
>>
>
> Yep - and that's borne out by practice.
>

OK, well, then it would help if the document were clearer on this point.

>
>>

>
>>>
>>>>
>>>>
>>>> Line 837
>>>>        opaque LogID<2..127>;
>>>> This seems to be the first use of the TLS specification language, but I
>>>> don't see a cite. Please provide one,
>>>>
>>>
>>> See s1.2.
>>>
>>
>> OK. Thanks.
>>
>>
>>
>>>
>>>> Line 926
>>>>        opaque TBSCertificate<1..2^24-1>;
>>>> NIT: there's no actual way a TBSCertificate can be 1 byte.
>>>>
>>>>
>>>> Line 948
>>>>    "tbs_certificate".  The length of the "issuer_key_hash" MUST match
>>>>    HASH_SIZE.
>>>> Is this true? What happens if we have two CAs that share a key?
>>>>
>>>
>>> Eh?
>>>
>>
>> Say that a CA spins up two subordinates that happen to share the same
>> key. I agree it's foolish.
>>  return an empty
>>
>>
>>
>>>> Line 1522
>>>>    permissible.  These entries SHALL be sequential beginning with the
>>>>    entry specified by "start".
>>>> How does the client know which of the above two cases has occurred?
>>>>
>>>
>>> The response includes an STH, which says how big the tree is. Probably
>>> should be the latest one known to that server.
>>>
>>>
>>>>
>>>>
>>>> Line 1552
>>>>    TLS servers MUST use at least one of the three mechanisms listed
>>>>    below to present one or more SCTs from one or more logs to each TLS
>>>> This needs to somehow be clear that it only applies to TLS servers that
>>>> are compliant with this specification, as it's not a new requirement on all
>>>> TLS servers.
>>>>
>>>
>>> Surely its the other way round: i.e. new requirements on all TLS servers
>>> have to be made clear?
>>>
>>
>> I'm not sure what you mean. This document does not get to require that
>> all TLS servers do CT. Was that your intent?
>>
>
> What I meant is that if every time you mention a thing you have to say
> "but only if you are conforming to this RFC" then you a) make the whole
> thing a lot more cumbersome, b) are stating the obvious (i.e. that you only
> have to conform to the RFC if you have decided to conform to the RFC).
>

I don't think it's merely stating the obvious in that we have *other* RFCs
which actually do attempt to retrospectively impose requirements on all
uses of TLS. See, for instance: https://tools.ietf.org/html/rfc7465. And
this is a different story, in that we are not (I assume) going to go around
saying that TLS stacks that don't do CT are not TLS conformant.

I think the text I would use here would be "CT-using TLS servers MUST..."


Surely if you are making a new requirement for TLS compliance then you have
> to explicitly say so?
>

Well, my point is that the way that this document is written in fact does
so, and so you have to look at "Updates" to find out that that's not the
case.

-Ekr


>
>

>
>>
>> -EKr
>>
>>
>>>
>>>> Line 1595
>>>>    been struck off for misbehavior, has had a key compromise, or is not
>>>>    known to the TLS client).  For example:
>>>> Maybe replace "For example:" with "Some ways this can happen are..."
>>>>
>>>>
>>>> Line 1599
>>>>       misissuance from clients.  Including SCTs from different logs
>>>>       makes it more difficult to mount this attack.
>>>> Assuming that the server is malicious, why would it include multiple
>>>> SCTs? It seems like requiring multiple SCTs does in fact provide this
>>>> defense, but that's not an argument for servers to provide multiples.
>>>>
>>>>
>>>> Line 1627
>>>>              SerializedTransItem trans_item_list<1..2^16-1>;
>>>>          } TransItemList;
>>>> Structurally, it's kind of a mess to have this be the place that you
>>>> make TransItems self-contained (by having a defined length field). What
>>>> about other places I might want to concatenate TransItems. Why don't you
>>>> instead make TransItem self-contained, like so:
>>>>
>>>> struct {
>>>>           VersionedTransType versioned_type;
>>>>           uint16 length;   // NEW
>>>>           select (versioned_type) {
>>>>               case x509_entry_v2: TimestampedCertificateEntryDataV2;
>>>>               case precert_entry_v2: TimestampedCertificateEntryDataV2;
>>>>               case x509_sct_v2: SignedCertificateTimestampDataV2;
>>>>               case precert_sct_v2: SignedCertificateTimestampDataV2;
>>>>               case signed_tree_head_v2: SignedTreeHeadDataV2;
>>>>               case consistency_proof_v2: ConsistencyProofDataV2;
>>>>               case inclusion_proof_v2: InclusionProofDataV2;
>>>>           } data;
>>>>       } TransItem;
>>>> This is pretty much the universal TLS convention.
>>>>
>>>>
>>>> Line 1649
>>>> 6.4.  transparency_info TLS Extension
>>>> This extension appears not to have any explicit support for CT entries
>>>> for intermediate certs. Am I just supposed to glue together all the
>>>> TransItems?
>>>>
>>>>
>>>> Line 1651
>>>>    Provided that a TLS client includes the "transparency_info" extension
>>>>    type in the ClientHello, the TLS server SHOULD include the
>>>> You need to provide an actual definition of what the client includes,
>>>> and having the server ignore the contents is bad mojo. TLS convention is
>>>> for the client to include an empty extension and the server to validate
>>>> that it is in fact empty.
>>>>
>>>>
>>>> Line 1654
>>>>    "transparency_info" extension in the ServerHello with
>>>>    "extension_data" set to a "TransItemList".  The TLS server SHOULD
>>>>    ignore any "extension_data" sent by the TLS client.  Additionally,
>>>> IMPORTANT: The normative language here is kind of confusing. It SHOULD
>>>> include the extension but if it's included, it MUST consist of
>>>> TransItemList, no? And surely only SHOU
>>>> Also, I'm not sure this is the right logic. If the server knows that it
>>>> has the SCT information in the certificate or in OCSP, why SHOULD It send
>>>> this extension. I would think, rather that servers should aim to send
>>>> information at most once, so that it should only send the extension if it
>>>> contains information that's not in the cert/OCSP. as it pretty much has to
>>>> send those anyway. Otherwise, don't we just end up in a world where if this
>>>> info is in OCSP and certs, it's always sent twice, because the client
>>>> doesn't know where the info is, and so has to always offer the extension.
>>>>
>>>>
>>>> Line 1658
>>>>    session is resumed, since session resumption uses the original
>>>>    session information.
>>>> Does this mean the client MUST abort the handshake if the server
>>>> includes it?
>>>>
>>>>
>>>> Line 1668
>>>>    o  The TLS client includes the "transparency_info" extension type in
>>>>       the ClientHello.
>>>> This condition is non-sensical, because if the client *doesn't* include
>>>> the extension, the server cannot send the transparency_info extension at
>>>> all.
>>>>
>>>>
>>>> Line 1722
>>>> 8.  Clients
>>>> Given the imminent standardization of TLS 1.3, you need to somehow
>>>> provide a mapping for client-side CT for that, I think
>>>>
>>>>
>>>> Line 1739
>>>>    view.  The exact mechanisms will be in separate documents, but it is
>>>>    expected there will be a variety.
>>>> Given the somewhat science fictional status of Gossip, this entire
>>>> paragraph should be stricken
>>>>
>>>>
>>>> Line 1747
>>>>    MUST implement all of the three mechanisms by which TLS servers may
>>>>    present SCTs (see Section 6).  TLS clients MAY also accept SCTs via
>>>>    the "status_request_v2" extension ([RFC6961]).  TLS clients that
>>>> IMPORTANT: This also needs to be rewritten so it makes clear it's not a
>>>> general levy on TLS clients.
>>>>
>>>> Line 1770
>>>>    In addition to normal validation of the server certificate and its
>>>>    chain, TLS clients SHOULD validate each received SCT for which they
>>>>    have the corresponding log's parameters.  To validate an SCT, a TLS
>>>> IMPORTANT: Why is this a SHOULD and not a MUST? If you support CT at
>>>> all, why would you not do this?
>>>>
>>>> Line 1791
>>>>    TLS clients MUST NOT consider valid any SCT whose timestamp is in the
>>>>    future.
>>>> What's the reason for this? If your clock is slightly wrong, this is
>>>> going to cause new certs to fail even if they otherwise would have
>>>> succeeded (because the notBefore and notAfter are more conservative).
>>>>
>>>>
>>>> Line 1800
>>>>    will disclose to the log which TLS server the client has been
>>>>    communicating with.
>>>> IMPORTANT: This "Note" just mentions in passing a huge privacy issue.
>>>> You need to be a lot clearer about this.
>>>>
>>>> Line 1823
>>>>    "transparency_info" and "status_request" TLS extensions in the
>>>>    ClientHello.
>>>> IMPORTANT: This is not consistent with the requirements on the server.
>>>> Trying to reconstruct the reasoning here, the client can only decide
>>>> that the server is noncompliant if it has given the server a chance to send
>>>> the SCTs by every mechanism., otherwise the server might just want to send
>>>> the SCT some other way. However, if servers can optionally ignore
>>>> transparency_info (it's a SHOULD above), then you can have two compliant
>>>> implementations with the server having a CT-compliant cert and yet the
>>>> client declares it noncompliant. To fix this, you need to require the
>>>> server to respond to "transparency_info"
>>>>
>>>>
>>>> Line 1831
>>>>    "CachedObject" of type "ct_compliant" in the "cached_info" extension.
>>>>    The "hash_value" field MUST be 1 byte long with the value 0.
>>>> You should explain why this is one byte long (that the PDU is defined
>>>> as having a minimum length of 1). Also the server should be required to
>>>> check it.
>>>>
>>>>
>>>> Line 1842
>>>>    watches.  It may also want to keep copies of entire logs.  In order
>>>>    to do this, it should follow these steps for each log:
>>>> Why is this not a 2119 SHOULD?
>>>>
>>>> Also, what does "in order to do this" refer to? Clearly not how to keep
>>>> copies.... Presumably, how to poll the log.
>>>>
>>>>
>>>> Line 1864
>>>>    8.  Either:
>>>> IMPORTANT: You seem to be missing there part where you actually look at
>>>> the entries to verify that they don't contain bogus data (e.g.,
>>>> certificates for your domain). I get that it's implicit here, but given
>>>> that you provide an algorithm, that should be an explicit stage.
>>>> This is a pretty odd algorithm. If I understand it correctly, 1-4 are
>>>> setup steps and then 5-9 is supposed to be repeated, but I could just do
>>>> this once and stop at 4.
>>>>
>>>>
>>>> Line 1912
>>>>    STHs it receives, ensure that each entry can be fetched and that the
>>>>    STH is indeed the result of making a tree from all fetched entries.
>>>> IMPORTANT: How do you verify MMD?
>>>>
>>>> Line 1944
>>>>    If it should become necessary to deprecate an algorithm used by a
>>>>    live log, then the log should be frozen as specified in Section 4.13
>>>>    and a new log should be started.  Certificates in the frozen log that
>>>> RFC 2119 SHOULD? Isn't this a MUST, though?
>>>>
>>>>
>>>> Line 1958
>>>>    "transparency_info" TLS extension.  IANA should update this extension
>>>>    type to point at this document.
>>>> IMPORTANT: You'll need to fill in the new field specified in
>>>> https://tlswg.github.io/draft-ietf-tls-iana-registry-updates
>>>> /#rfc.section.6
>>>>
>>>> Line 2009
>>>>    |                                | ECDSA (NIST P-256) |             |
>>>>    |                                | with HMAC-SHA256   |             |
>>>>    |                                |                    |             |
>>>> Why are you defining both algorithms?
>>>>
>>>>
>>>> Line 2150
>>>>    (with the intention of actually running a CT log that will be
>>>>    identified by the allocated Log ID).
>>>> This seems like it's not a great thing to be asking an expert to do, as
>>>> it seems to require business arrangements. Is it really that valuable to
>>>> save a few bytes here?
>>>>
>>>>
>>>> Line 2163
>>>>    that the log has misbehaved, which will be discovered when the SCT is
>>>>    audited.  A signed timestamp is not a guarantee that the certificate
>>>>    is not misissued, since appropriate monitors might not have checked
>>>> IMPORTANT: This is not correct, because the client does not know that
>>>> the monitors are verifying the data that it is. See my general comments on
>>>> public verifiability above.
>>>>
>>>> Line 2182
>>>>    operating correctly.  As a log is allowed to serve an STH that's up
>>>>    to MMD old, the maximum period of time during which a misissued
>>>>    certificate can be used without being available for audit is twice
>>>> Nit: up to the MMD old
>>>>
>>>>
>>>> Line 2211
>>>>    compute the proofs from the log) or communicate with the log via
>>>>    proxies.
>>>> This also seems quite handwavy in light of the facts on the ground.
>>>>
>>>>
>>>> Line 2237
>>>>       and STHs can be stored by the log and served to other clients that
>>>>       submit the same certificate or request the same STH.
>>>> This needs to be expanded. Who is this risk against? The log or someone
>>>> else? If the log, what's the logs incentive?
>>>>
>>>>
>>>> Line 2243
>>>>    reduce the effectiveness of an attack where a CA and a log collude
>>>>    (see Section 6.1).
>>>> See my comments in 6.1 about this.
>>>>
>>>>
>>>> _______________________________________________
>>>> Trans mailing list
>>>> [e-mail]
>>>> https://www.ietf.org/mailman/listinfo/trans
>>>>
>>>>
>>>
>>
>