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

[Kent Watsen <[e-mail]>]

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



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


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



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

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.  


> /martin
> 

Kent // contributor