Re: [netconf] client identification in ietf-netconf-server

Martin Bjorklund <mbj@tail-f.com> Mon, 11 November 2019 10:00 UTC

Return-Path: <mbj@tail-f.com>
X-Original-To: netconf@ietfa.amsl.com
Delivered-To: netconf@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 722F412083E for <netconf@ietfa.amsl.com>; Mon, 11 Nov 2019 02:00:49 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.9
X-Spam-Level:
X-Spam-Status: No, score=-1.9 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, SPF_HELO_NONE=0.001, SPF_PASS=-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 MPn3RHUJ4AfD for <netconf@ietfa.amsl.com>; Mon, 11 Nov 2019 02:00:46 -0800 (PST)
Received: from mail.tail-f.com (mail.tail-f.com [46.21.102.45]) by ietfa.amsl.com (Postfix) with ESMTP id 03F7F120059 for <netconf@ietf.org>; Mon, 11 Nov 2019 02:00:45 -0800 (PST)
Received: from localhost (unknown [173.38.220.41]) by mail.tail-f.com (Postfix) with ESMTPSA id 51C7F1AE018B; Mon, 11 Nov 2019 11:00:43 +0100 (CET)
Date: Mon, 11 Nov 2019 11:00:13 +0100
Message-Id: <20191111.110013.2019956803552089416.mbj@tail-f.com>
To: kent+ietf@watsen.net
Cc: netconf@ietf.org
From: Martin Bjorklund <mbj@tail-f.com>
In-Reply-To: <0100016e4d8323b0-2a182947-485d-43e6-908c-13bc5ad2f210-000000@email.amazonses.com>
References: <0100016e39631e46-c007fd65-2e51-47a6-bd27-764f5257a16c-000000@email.amazonses.com> <20191106.142822.2117534105126283386.mbj@tail-f.com> <0100016e4d8323b0-2a182947-485d-43e6-908c-13bc5ad2f210-000000@email.amazonses.com>
X-Mailer: Mew version 6.8 on Emacs 25.2
Mime-Version: 1.0
Content-Type: Text/Plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Archived-At: <https://mailarchive.ietf.org/arch/msg/netconf/F2t7pEuRj_LCfin7Q_VPg-WERXA>
Subject: Re: [netconf] client identification in ietf-netconf-server
X-BeenThere: netconf@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: NETCONF WG list <netconf.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/netconf>, <mailto:netconf-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/netconf/>
List-Post: <mailto:netconf@ietf.org>
List-Help: <mailto:netconf-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/netconf>, <mailto:netconf-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 11 Nov 2019 10:00:50 -0000

Hi,

Kent Watsen <kent+ietf@watsen.net> wrote:
> 
> Hi Martin,
> 
> [Not trimming down because too much context would be lost.]
> 
> 
> >>> The ietf-netconf-server module has this:
> >>> 
> >>> grouping netconf-server-grouping {
> >>>   ...
> >>>   container client-identification {
> >>>     ...
> >>>     container cert-maps {
> >>>       when "../../../../tls";
> >>>       uses x509c2n:cert-to-name;
> >>>       ...
> >>>     }
> >>>   }
> >>> }
> >>> 
> >>> Note the "when" expression.  This means that the grouping has a strong
> >>> depency on where is it used.  We should try to avoid such a design.
> >> 
> >> 
> >> Would this be better?   
> >> 
> >> OLD
> >>       when "../../../../tls";
> >> 
> >> NEW
> >> 	if-feature "tls-listen or tls-call-home";
> > 
> > Yes, but see below.
> > 
> > 
> >>> But should't this cert-to-name list be available when x509-certs are
> >>> used also with SSH?
> >> 
> >> Hmmm.  I'd assumed that, with RFC 6187, the username was still passed
> >> as its own field, but I see this in Section 4:
> >> 
> >>   For the purposes of user authentication, the mapping between
> >>   certificates and user names is left as an implementation and
> >>   configuration issue for implementers and system administrators.
> > 
> > If the username was used as identification it would mean that with a
> > valid cert I could present myself as any user!
> > 
> >> So you may be right about that.  I only ever looked at RFC 6187 from
> >> the perspective of the server presenting an IDevID certificate.  But,
> >> assuming it's true, then perhaps this:
> >> 
> >> NEWEST:
> >> 	if-feature "tls-listen or tls-call-home or sshcmn:ssh-x509-certs";
> > 
> > Ok.
> > 
> > This gives:
> > 
> >  grouping netconf-server-grouping {
> >    description ...;
> >    container client-identification {
> >      description
> >        "Specifies a mapping through which clients MAY be identified
> >         (i.e., the NETCONF username) from a supplied certificate.
> >         Note that a client MAY alternatively be identified via an
> >         alternate authentication scheme.";
> >      container cert-maps {
> >        if-feature "tls-listen or tls-call-home or sshcmn:ssh-x509-certs";
> 
> Yes.
> 
> > But since the description of the "client-identification" says that it
> > is used only with certificates, perhaps that container's name should
> > reflect this, and the if-feature statement moved to that container?
> > Perhaps:
> > 
> >    container client-cert-identification
> >      if-feature "tls-listen or tls-call-home or sshcmn:ssh-x509-certs";
> > 
> > and also perhaps remove 'cert-maps', and use the cert-to-name grouping
> > directly here?
> 
> Good.  My only hesitation is that someday there may be a need for
> another way to identify clients, but that sounds too far out (even for
> me) to squabble over.  But a better name is needed.
> "cert-based-client-identification" would be more accurate, but that
> seems overly long.  Looking at a snippet of config might help...
> 
>       netconf-server-parameters : {
>         something-here : [
>           {
>             cert-to-name : { ... }
>             cert-to-name : { ... },
>             ...
>             cert-to-name : { ... }
>           }
>         ]
>       }
> 
> How about "cert-to-name-mappings"?  ( know, almost the same length,
> but half the number of syllables!).  But that name leaves out the word
> "identity", which is may be important in security circles, so maybe
> "client-identity-mappings"?

I think this name is as generic as "client-identification".  The best
so far imo is "cert-based-client-identification".  A bit long, but
descripitive.

> This seems pretty good, right?  (I
> renamed it to "client-identity-mappings" in both ietf-netconf-server
> and ietf-restconf-server)
> 
> 
> >>> The current data model for ssh specifies certs on
> >>> a per-user basis. But this requires lots of configuration in the case
> >>> that the cert encodes the user name (even though the name is in the
> >>> cert you have to configure each user on each device).  I suggest we
> >>> align the model for SSH with the TLS model for cert identification.
> >> 
> >> We certainly want to factor out configuration where possible.  I'd
> >> need to look into this more.  Perhaps you can send a diff?
> > 
> > Today we have under 'ssh-server-parameters/client-authentication':
> > 
> >  +--:(local) {local-client-auth-supported}?
> >     +--rw users
> >        +--rw user* [name]
> >           +--rw name            string
> >           +--rw password?       ianach:crypt-hash
> >           +--rw host-keys!
> >           |  +--rw (local-or-truststore)
> >           |     +--:(local) {local-definitions-supported}?
> >           |     |  +--rw local-definition
> >           |     |     +--rw host-key*                 ct:ssh-host-key
> >           |     |     +--rw cert*                     trust-anchor-cert-cms
> >           |     |     +---n certificate-expiration
> >           |     |        +-- expiration-date    yang:date-and-time
> 
> Not to take away from your point, but the previous three lines don't
> exist in the model.
> 
> >           |     +--:(truststore) {truststore-supported,ssh-host-keys}?
> >           |        +--rw truststore-reference?   ts:host-keys-ref
> >           +--rw ca-certs! {sshcmn:ssh-x509-certs}?
> >           |  +--rw (local-or-truststore)
> >           |     +--:(local) {local-definitions-supported}?
> >           |     |  +--rw local-definition
> >           |     |     +--rw cert*                     trust-anchor-cert-cms
> >           |     |     +---n certificate-expiration
> >           |     |        +-- expiration-date    yang:date-and-time
> >           |     +--:(truststore) {truststore-supported,x509-certificates}?
> >           |        +--rw truststore-reference?   ts:certificates-ref
> >           +--rw client-certs! {sshcmn:ssh-x509-certs}?
> >              +--rw (local-or-truststore)
> >                 +--:(local) {local-definitions-supported}?
> >                 |  +--rw local-definition
> >                 |     +--rw cert*                     trust-anchor-cert-cms
> >                 |     +---n certificate-expiration
> >                 |        +-- expiration-date    yang:date-and-time
> >                 +--:(truststore) {truststore-supported,x509-certificates}?
> >                 +--rw truststore-reference?   ts:certificates-ref
> > 
> > I think host-keys, ca-certs and client-certs should be moved out of
> > the user list:
> > 
> >  +--:(local) {local-client-auth-supported}?
> >     +--rw users
> >     |  +--rw user* [name]
> >     |     +--rw name            string
> >     |     +--rw password?       ianach:crypt-hash
> >     +--rw host-keys!
> >     |  +--rw (local-or-truststore)
> >     |     +--:(local) {local-definitions-supported}?
> >     |     |  +--rw local-definition
> >     |     |     +--rw host-key*                 ct:ssh-host-key
> >     |     |     +--rw cert*                     trust-anchor-cert-cms
> >     |     |     +---n certificate-expiration
> >     |     |        +-- expiration-date    yang:date-and-time
> 
> Again, not to take away from your point, but the previous three lines
> don't exist in the model.
> 
> >     |     +--:(truststore) {truststore-supported,ssh-host-keys}?
> >     |        +--rw truststore-reference?   ts:host-keys-ref
> >     +--rw ca-certs! {sshcmn:ssh-x509-certs}?
> >     |  +--rw (local-or-truststore)
> >     |     +--:(local) {local-definitions-supported}?
> >     |     |  +--rw local-definition
> >     |     |     +--rw cert*                     trust-anchor-cert-cms
> >     |     |     +---n certificate-expiration
> >     |     |        +-- expiration-date    yang:date-and-time
> >     |     +--:(truststore) {truststore-supported,x509-certificates}?
> >     |        +--rw truststore-reference?   ts:certificates-ref
> >     +--rw client-certs! {sshcmn:ssh-x509-certs}?
> >        +--rw (local-or-truststore)
> >           +--:(local) {local-definitions-supported}?
> >           |  +--rw local-definition
> >           |     +--rw cert*                     trust-anchor-cert-cms
> >           |     +---n certificate-expiration
> >           |        +-- expiration-date    yang:date-and-time
> >           +--:(truststore) {truststore-supported,x509-certificates}?
> >              +--rw truststore-reference?   ts:certificates-ref
> 
> I agree that "ca-certs" and "client-certs" should be pulled out (as
> they are in ietf-tls-server), but I'm unsure if "host-keys" can be, at
> least not unless we introduce something like "host-key-to-name" maps,
> right?
> 
> For now, I only pulled out "ca-certs" and "client-certs".

Hmm, I realize that I have misunderstood 'host-keys' here.  How
exactly is this supposed to be used?  This is the client
authentication part of a server.  How is the *host* key used here by
the server?   I mean, the client doesn't present a host key to the
server, so I don't understand what this is.


> > But also here I think that the choice "local-or-external" isn't
> > ideal.  I think that a system that implements some "external"
> > mechanism should/would augement this data model with specific nodes
> > for that mechanism.  As a simplistic example:
> > 
> >  augment /netconf-server/.../client-authentication {
> >    leaf use-host-keys-in-filesystem {
> >      leaf boolean;
> >    }
> >  }
> > 
> > In this case, requiring the client to configure both this new leaf and
> > "client-auth-defined-elsewhere" seems redundant and non-intuitive.
> 
> Agreed.
> 
> > Another case is a system that *always* use the filesystem host keys.
> > It would simply just always do that, and again, requiring the client
> > to configure "client-auth-defined-elsewhere" seems incorrect.
> 
> Agreed.
> 
> > So my suggestion is to remove the choice "local-or-external" and
> > remove the external case, and instead document that (i) systems may
> > use some other hard-wired mechanism or (ii) other modules can augment
> > this container with additional control parameters for other
> > mechanisms.
> 
> Agree in principle, but unsure about implementation.  One thing
> important to me you didn't mention is having the "local" configuration
> gated by a "feature" statement.  So, do we float the
> "local-client-auth-supported" (renamed appropriately) up to the
> "client-authentication" container?  If so, would that incorrectly
> cover the "supported-authentication-methods" descendent?
> Suggestions?

Perhaps just add the if-feature to the containers "users", "ca-certs",
"client-certs"?



> >>> For TLS, the data model has the following structure:
> >>> 
> >>> +--rw netconf-server
> >>>    +--rw listen! {ssh-listen or tls-listen}?
> >>>       +--rw idle-timeout?   uint16
> >>>       +--rw endpoint* [name]
> >>>          +--rw name         string
> >>>          +--rw (transport)
> >>>             ...
> >>>             +--:(tls) {tls-listen}?
> >>> 
> >>>   [ reset indentation to make the diagram easier to read ]
> >>> 
> >>>  +--rw tls
> >>>     +--rw tcp-server-parameters
> >>>     ...
> >>>     +--rw tls-server-parameters
> >>>     |  +--rw server-identity
> >>>           ...
> >>>     |  +--rw client-authentication!
> >>>     |  |  +--rw (required-or-optional)
> >>>     |  |  |  +--:(required)
> >>>     |  |  |  |  +--rw required?    empty
> >>>     |  |  |  +--:(optional)
> >>>     |  |  |     +--rw optional?    empty
> >>>     |  |  +--rw (local-or-external)
> >>>     |  |     +--:(local)  {local-client-auth-supported}?
> >>>     |  |     |  +--rw ca-certs!   {ts:x509-certificates}?
> >>>     |  |     |  |  +--rw (local-or-truststore)
> >>>     |  |     |  |     +--:(local)  {local-definitions-supported}?
> >>>     |  |     |  |     |  +--rw local-definition
> >>>     |  |     |  |     |     +--rw cert*   trust-anchor-cert-cms
> >>>     |  |     |  |     |     +---n certificate-expiration
> >>>     |  |     |  |     |        +-- expiration-date
> >>>     |  |     |  |     |                yang:date-and-time
> >>>     |  |     |  |     +--:(truststore)
> >>>     |  |     |  |              {truststore-supported,x509-certificates}?
> >>>     |  |     |  |        +--rw truststore-reference?
> >>>     |  |     |  |                ts:certificates-ref
> >>>     |  |     |  +--rw client-certs!  {ts:x509-certificates}?
> >>>     |  |     |     +--rw (local-or-truststore)
> >>>     |  |     |        +--:(local)  {local-definitions-supported}?
> >>>     |  |     |        |  +--rw local-definition
> >>>     |  |     |        |     +--rw cert*     trust-anchor-cert-cms
> >>>     |  |     |        |     +---n certificate-expiration
> >>>     |  |     |        |        +-- expiration-date
> >>>     |  |     |        |                yang:date-and-time
> >>>     |  |     |        +--:(truststore)
> >>>     |  |     |                 {truststore-supported,x509-certificates}?
> >>>     |  |     |           +--rw truststore-reference?
> >>>     |  |     |                   ts:certificates-ref
> >>>     |  |     +--:(external)
> >>>     |  |              {external-client-auth-supported}?
> >>>     |  |        +--rw client-auth-defined-elsewhere?
> >>>     |  |                empty
> >>>         ...
> >>>     +--rw netconf-server-parameters
> >>>        +--rw client-identification
> >>>           +--rw cert-maps
> >>>              +--rw cert-to-name* [id]
> >>>                 +--rw id             uint32
> >>>                 +--rw fingerprint
> >>>                 |       x509c2n:tls-fingerprint
> >>>                 +--rw map-type       identityref
> >>>                 +--rw name           string
> >> 
> >> 
> >> 
> >> 
> >>> It is not clear how this is used by the server to end up either with
> >>> an authenticated user name or failed authentication.
> >> 
> >> Okay, let's fix that.
> >> 
> >> 
> >>> First of all, how is the "required-or-optional" choice used in a
> >>> NETCONF server?  What happens if an operation configures this to
> >>> "optional"?  (side note: why is this a choice of empty leafs instead
> >>> of a leaf?)
> >> 
> >> Hmmm, this 'choice' seems unneeded for NETCONF.  The "choice" is
> >> coming from the ietf-tls-server, and a similar "choice" is in
> >> ietf-http-server. It was put there, in part, for RESTCONF, as
> >> user-auth can occur at either (or both!) protocol layers...
> > 
> > Ok.  Yes, the RESTCONF auth mechanism is interesting.  Let's discuss
> > that in a separate thread.
> 
> Okay.  For now, I'll leave the "required-or-optional" in both
> ietf-tls-server and ietf-http-server.  However, to address the issue
> that it can never apply to NETCONF, it seems that a possible strategy
> would be to move both instances to augmentations defined in
> ietf-restconf-server...
> 
> That said, to go along with some of your thinking from above, it's not
> clear how an application would consume the "required-or-optional"
> configuration.  Case in point, in the RESTCONF server based product
> I'm working on, the configuration for each client, which is defined
> outside the restconf-server-grouping tree, has descendants nodes like
> "http-password" and "tls-trust-anchor", with meanings that, if
> defined, then the client MUST present said auth credentials at that
> protocol-layer.  IIRC, the code doesn't check these flags at all.
> 
> So, rather than moving both "required-or-optional" instances to
> augmentations in ietf-restconf-server, maybe they can just be deleted?

Yes I think so, altough I haven't yet studied the restconf model in
detail.

> >>> Second, I assume that the idea is that the server uses the config
> >>> params in "local-or-external" and the certificate presented by the
> >>> client and after this step is either accepted or rejected.  It is not
> >>> clear what is supposed to happen if someone configures
> >>> "client-auth-defined-elsewhere".  I think it is better to not define
> >>> this case, but (perhaps) keep the choice and explain that other
> >>> modules can augment additional config params here for other
> >>> authentication mechanisms.
> >> 
> >> Well that's just the thing, the goal is to enable user-auth to NOT be
> >> defined here.  As the description statement in ietf-tls-server says:
> >> 
> >>          "Configuring credentials externally enables applications
> >>           to place client authentication with client definitions,
> >>           rather then in a part of a data model principally
> >>           concerned with configuring the TLS transport.";
> > 
> > I totally agree with this.  I am questioning the solution.  See above
> > for my proposal.
> 
> Ack.
> 
> 
> >>> Next, my guess is that the intention is that if the cert was accepted
> >>> in the step above, it is checked in cert-to-name to see if a user name
> >>> can be derived.
> >> 
> >> Yes.
> >> 
> >> 
> >>> In another thread you mentioned that if a local cert is configured, it
> >>> seems redundant to also configure the cert as a fingerprint in
> >>> cert-to-name.  I'm not sure about this.  But perhaps you can use the
> >>> same "map-type" and "name" leafs in the "client-cert" container?  It
> >>> is not as easy for the "truststore-reference"; perhaps you'd have to
> >>> augment the truststore with these leafs in this case.
> >> 
> >> In context, that statement I made before is a relatively minor
> >> objection.  That said, I don't understand your proposal, are you
> >> suggesting to recreate the essence of 'cert-to-name'?  Another idea I
> >> had was that the fingerprint could be in a "union" with also a
> >> truststore-reference, which is only mildly better...
> > 
> > Aha, now I understand your suggestion of making fingerprint optional.
> > I agree that this could work.  However, I assume it must be used with
> > care.  If you know for sure that a successful result from the
> > authentication mechanism means that CA cert X has been used, you can
> > save some typing by not configuring the fingerprint of X.  So the
> > question is if it is worth it?
> 
> Yes, saving typing is the gist of it, but I don't think handling with
> care is needed or, rather, it's no more care.  As I understand it, a
> fingerprint would be redundant in the common case, i.e., most configs
> would not have to define a fingerprint, so the optimization seems
> worth it to me.

It would be interesting to hear other opinions on this, esp. from
security people.  Personally I can accept both alternatives.

> Separately, be aware that calculating an x509c2n:tls-fingerprint is
> not a simple copy/paste.  That is, the command `openssl x509 -in
> CERT.pem -noout -sha256 -fingerprint` is close, but not exactly what
> is needed.

Right; you have to prefix this fingerprint with "04:".


/martin