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

Martin Bjorklund <mbj@tail-f.com> Wed, 06 November 2019 13:28 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 F1EE1120904 for <netconf@ietfa.amsl.com>; Wed, 6 Nov 2019 05:28:56 -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 6GI4Lv6bleZI for <netconf@ietfa.amsl.com>; Wed, 6 Nov 2019 05:28:54 -0800 (PST)
Received: from mail.tail-f.com (mail.tail-f.com [46.21.102.45]) by ietfa.amsl.com (Postfix) with ESMTP id 071F0120C6F for <netconf@ietf.org>; Wed, 6 Nov 2019 05:28:53 -0800 (PST)
Received: from localhost (unknown [173.38.220.41]) by mail.tail-f.com (Postfix) with ESMTPSA id A41361AE018B; Wed, 6 Nov 2019 14:28:51 +0100 (CET)
Date: Wed, 06 Nov 2019 14:28:22 +0100 (CET)
Message-Id: <20191106.142822.2117534105126283386.mbj@tail-f.com>
To: kent+ietf@watsen.net
Cc: netconf@ietf.org
From: Martin Bjorklund <mbj@tail-f.com>
In-Reply-To: <0100016e39631e46-c007fd65-2e51-47a6-bd27-764f5257a16c-000000@email.amazonses.com>
References: <20191104.111319.869021723410428870.mbj@tail-f.com> <0100016e39631e46-c007fd65-2e51-47a6-bd27-764f5257a16c-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/DMenOqEk0jgP-N2ImvsC_2RL6Lw>
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: Wed, 06 Nov 2019 13:29:01 -0000

Hi,

Kent Watsen <kent+ietf@watsen.net> wrote:
> Hi Martin,
> 
> 
> > 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";

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?

> > 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
           |     +--:(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
     |     +--:(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


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.

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.


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.


> > 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.


> > 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.

> > 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, as you
> suggesting the 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?


/martin