Hi Paul,
I still have to review this. Will try to do so in the next week or so
-Ekr
On Thu, Apr 19, 2018 at 6:30 AM, Paul Wouters <[email protected]
<mailto:[email protected]>> wrote:
On Thu, 7 Sep 2017, Eric Rescorla wrote:
Eric,
Are all your concerns raised in the AD review met by version -28 ?
The diff since your last review:
https://tools.ietf.org/rfcdiff?url2=draft-ietf-trans-rfc6962-bis-28.txt&url1=draft-ietf-trans-rfc6962-bis-26.txt
<https://tools.ietf.org/rfcdiff?url2=draft-ietf-trans-rfc6962-bis-28.txt&url1=draft-ietf-trans-rfc6962-bis-26.txt>
Paul
Date: Thu, 7 Sep 2017 10:41:45
From: Eric Rescorla <[email protected] <mailto:[email protected]>>
Cc: "[email protected] <mailto:[email protected]>" <[email protected]
<mailto:[email protected]>>
To: Ben Laurie <[email protected] <mailto:[email protected]>>
Subject: Re: [Trans] AD Review: draft-ietf-trans-rfc6962-bis-26.txt
On Thu, Sep 7, 2017 at 3:24 AM, Ben Laurie <[email protected]
<mailto:[email protected]>> wrote:
On 5 September 2017 at 18:46, Eric Rescorla <[email protected]
<mailto:[email protected]>> wrote:
On Tue, Sep 5, 2017 at 9:04 AM, Ben Laurie
<[email protected] <mailto:[email protected]>> wrote:
On 4 September 2017 at 00:17, Eric Rescorla
<[email protected] <mailto:[email protected]>> 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
<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
<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
<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
[email protected] <mailto:[email protected]>
https://www.ietf.org/mailman/listinfo/trans
<https://www.ietf.org/mailman/listinfo/trans>
_______________________________________________
Trans mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/trans
