[Cbor] Review of draft-ietf-cose-cbor-encoded-cert-02

Russ Housley <housley@vigilsec.com> Wed, 15 December 2021 23:08 UTC

Return-Path: <housley@vigilsec.com>
X-Original-To: cbor@ietfa.amsl.com
Delivered-To: cbor@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id E7B743A0962 for <cbor@ietfa.amsl.com>; Wed, 15 Dec 2021 15:08:29 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.898
X-Spam-Level:
X-Spam-Status: No, score=-1.898 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, SPF_HELO_NONE=0.001, SPF_NONE=0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id NLTUXA-W9T9G for <cbor@ietfa.amsl.com>; Wed, 15 Dec 2021 15:08:27 -0800 (PST)
Received: from mail.smeinc.net (mail.smeinc.net [209.135.209.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id CEB7F3A0958 for <cbor@ietf.org>; Wed, 15 Dec 2021 15:08:26 -0800 (PST)
Received: from localhost (localhost [127.0.0.1]) by mail.smeinc.net (Postfix) with ESMTP id 43A3D300C4E for <cbor@ietf.org>; Wed, 15 Dec 2021 18:08:29 -0500 (EST)
X-Virus-Scanned: amavisd-new at mail.smeinc.net
Received: from mail.smeinc.net ([127.0.0.1]) by localhost (mail.smeinc.net [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id OGxZ8hYCzdSM for <cbor@ietf.org>; Wed, 15 Dec 2021 18:08:27 -0500 (EST)
Received: from a860b60074bd.fios-router.home (pool-141-156-161-153.washdc.fios.verizon.net [141.156.161.153]) by mail.smeinc.net (Postfix) with ESMTPSA id 93943300B20 for <cbor@ietf.org>; Wed, 15 Dec 2021 18:08:27 -0500 (EST)
From: Russ Housley <housley@vigilsec.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.21\))
Message-Id: <80BF4A6F-9AB7-4F0D-95CB-FAFB2AFE112A@vigilsec.com>
Date: Wed, 15 Dec 2021 18:08:23 -0500
To: cbor@ietf.org
X-Mailer: Apple Mail (2.3445.104.21)
Archived-At: <https://mailarchive.ietf.org/arch/msg/cbor/mObLerzdV1uIu18kmcM5vsaMzgg>
Subject: [Cbor] Review of draft-ietf-cose-cbor-encoded-cert-02
X-BeenThere: cbor@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "Concise Binary Object Representation \(CBOR\)" <cbor.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/cbor>, <mailto:cbor-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/cbor/>
List-Post: <mailto:cbor@ietf.org>
List-Help: <mailto:cbor-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/cbor>, <mailto:cbor-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 15 Dec 2021 23:08:30 -0000

I agreed to review this document during the session at IETF 112.

Russ

= = = = = = = =

Document: draft-ietf-cose-cbor-encoded-cert-02
Reviewer: Russ Housley
Review Date: 2021-12-15

Major Concerns:

I think that the document should just define C509.  I would like to see
Sections 4, 5, and 6 moved to other documents.  If there is a desire to
include CRLs, the section needs to be expanded for CRL extensions and
CRL entry extensions.  The most important of these is CRL number.  I
really would like to see some implementation and deployment experience
to guide the specification of these other objects.

Section 3 says:

   The re-encoding does not work for BER encoded certificates.

This is not quite correct.  One can always convert a BER-encoded
certificate to a DER-encoded certificate.  Then, the DER-encoded
certificate can be converted to C509.

Section 3.1 says:

   o  signature.  The 'signature' field is always the same as the
      'signatureAlgorithm' field and therefore omitted from the CBOR
      encoding.

Does this mean that the signature algorithm identifier is not covered
by the signature for natively signed C509 certificates?  There needs
to be some way to provide integrity protection for this identifier.
RFC 6211 talks about the reasons for the integrity protection in a
different context.
 
Section 3.3 includes a bullet for subjectAltName, and in that part of
the document it talks about hardwareModuleName from RFC 4108.  I think
that the AcpNodeName from RFC 8994 probably needs a similar encoding.
Also, if EAI is successful, then SmtpUTF8Mailbox from RFC 8398 will
also benefit from a shorter encoding.


Minor Concerns:

In Section 1, the text says:

   ... Two variants are defined using the same CBOR
   encoding and differing only in what is being signed:

Yes, this is true, but this is the place to say that the two variants
are not interchangeable or interoperable.  As a result, I wonder if
giving names to the two variants would be useful.  Which one is C509?
The phrase "natively signed C509 certificates" seems to suggest that
a short name for the variant is needed.

Section 3: please add a reference for ASN.1.


Nits:

Throughout: s/DER encoded/DER-encoded/

The Abstract includes:

   ... The CBOR
   encoding supports a large subset of RFC 5280 and all certificates
   compatible with the RFC 7925, IEEE 802.1AR (DevID), CNSA, RPKI, GSMA
   eUICC, and CA/Browser Forum Baseline Requirements profiles. ...
   
I suggest this text be dropped.  It is already in the Introduction. In
the Abstract you cannot provide references for these profiles.

The Abstract includes:

   ... When
   used to re-encode DER encoded X.509 certificates, the CBOR encoding
   can in many cases reduce the size of RFC 7925 profiled certificates
   with over 50%. ...

I suggest rewording:

   ... A 50% reduction in size can be achieved for DER-encoded X.509
   certificates that meet the profile in RFC 7925. ...

Section 3, first para: Is there any reason to list the profiles again?
Listing them once in the Introduction seems adequate.

Section 3.2 has two mostly unrelated subsections.  I suggest that you
have two sections: 3.2 for Encoding of subjectPublicKey, and Section 3.3
for Encoding of issuerSignatureValue.

Section 3.3, bullet about Authority Key Identifier extension:
  s/certIssuer/authorityCertIssuer/
  s/certSerialNumberm/authorityCertSerialNumber/