Coder Social home page Coder Social logo

gnsi's People

Contributors

aashaikh avatar brianneville avatar bstoll avatar dang100 avatar dependabot[bot] avatar dplore avatar earies avatar gmacf avatar haussli avatar marcushines avatar mhines01 avatar morrowc avatar nmahabaleshwar avatar rlucus avatar robshakir avatar sourcequench avatar tomek-us avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

gnsi's Issues

Proposal to use the public key generated on the box for creating SSH host certificate

Some switches have the capability to generate private/public keys eliminating the necessity to generate them externally and transferring them to the switch. This avoids the potential security risk on transmitting private keys over the wire. Below is a proposal which helps operators utilize this capability.

Below will be the sequence of steps,

  1. Query the key generation capability of the switch. This is basically providing a key specification and checking if the switch can generate the public/private key.

    rpc CanGenerateKey(CanGenerateKeyRequest) returns (CanGenerateKeyResponse)

  2. Trigger generation of the key.

    rpc GenerateKey(GenerateKeyRequest) returns (GenerateKeyResponse)

  3. Poll to check if the key generation is complete. Key generation takes time, hence we need to poll the switch for key generation. If the key is ready, the public key portion of it should be returned else a simple Not ready.

    rpc GetPublicKey(GetPublicKeyRequest) returns (GetPublicKeyResponse)

  4. Sign the public key with the CA's private key and generate the host certificate.

  5. Install the generated host certificate onto the switch.

    rpc RotateHostCredentials(RotateHostCredentialsRequest) returns (RotateHostCredentialsResponse)

This is an existing RPC, we can just use the existing message ServerKeysRequest. The public/private key portions of the message need not be set.

If the operator does not wish to use this capability or if the switch does not support this capability, we can directly go to Step 5 and install the externally generated keys and certificates using the existing RPC.

Add gnsi.accounting is_truncated flags?

It is conceivable that an implementation may lack buffer space for some accounting messages, and we have seen this in practice. This might be more likely in embedded and IoT environments.

In theory, gnsi.accounting.CommandService.cmd_args (cmd too?) and GrpcService.payloads (and config_metadata) are potentially affected.
https://github.com/openconfig/gnsi/blob/main/accounting/acct.proto#L147-L151
https://github.com/openconfig/gnsi/blob/main/accounting/acct.proto#L174-L180

A more flexible abstraction for gNSI Certz

Currently, gNSI certz assumes the following:

Target (as seen from gNSI.certificate microservice point of view)
 |
 +-+ certificate (used by all gRPC services)
 | +- certificate (with public key)
 | +- private key
 |
 +-+ trust bundle (Certificate Authority certificates)
 | +- CA Root certificate
 | +- CA Intermediate Certificate
 |
 +-+ Certificate Revocation List bundle
   +- Certificate Revocation List A
   +- Certificate Revocation List B

 The above shown topology implies that every operation performed using this
 service changes a credential for all gRPC services at the same time.

I would like to propose a different abstraction here, based on the idea of an SSL profile which encapsulates all of the certs/bundle/CRLs.

An SSL profile would have the same structure as above:

SSL profile
 |
 +-+ certificate
 | +- certificate (with public key)
 | +- private key
 |
 +-+ trust bundle (Certificate Authority certificates)
 | +- CA Root certificate
 | +- CA Intermediate Certificate
 |
 +-+ Certificate Revocation List bundle
   +- Certificate Revocation List A
   +- Certificate Revocation List B

The name of the profile would have to be specified through a new string field ssl_profile which is added in the RotateCertificateRequest type.
If the ssl_profile field is left blank then the gNSI service should rotate the SSL profile which is currently in-use by the gNSI service.

An example of what this would look like in the certz.proto file:

// Request messages to rotate existing certificates on the target.
message RotateCertificateRequest {
  // If set to `true` the requested operation will succeed even if the
  // `version` is already in use (is not unique).
  // If set to `false` the requested operation will fail and the streaming RPC
  // will be closed with the `AlreadyExists` error if the `version` is already
  // in use (is not unique).
  // It is a no-op for the `finalize_rotation` request.
  bool force_overwrite = 1;

  // An identifier for the specific SSL profile 
  // (collection of certs/bundles/CRLs) 
  // which is being rotated through this stream
  // Leaving this field blank means that this stream
  // will rotate the SSL profile which is currently 
  // being used by the gNSI service.
  string ssl_profile = 2;

  // Request Messages.
  oneof rotate_request {
    GenerateCSRRequest generate_csr = 3;
    UploadRequest certificates = 4;
    FinalizeRequest finalize_rotation = 5;
  }
}

In the current proto spec, it is assumed that the system is pre-configured through the CLI so that all gRPC servers are using the same certs/bundles/CRLs.
For Arista systems, this means defining SSL profiles, assigning them names, then assigning these profiles to the gRPC servers that are started.

Compared to the current proto spec then, the only difference with these changes from the client's perspective would be that now they have to declare in their RotateCertificateRequest messages which SSL profile they are intending to rotate (or leave the field blank).

This brings advantage that multiple SSL profiles can be set up on the device and configured through gNSI.

  • For example in the case of grpctunnel, the client may want to use one SSL profile to secure the tunnel, and another SSL profile to secure the gNMI data within the tunnel. Supporting multiple SSL profiles in gNSI would allow this to be an option.

  • A client may also want to have one SSL profile used for all gRPC services, and another SSL profile used for some other non-gRPC service. They could then use gNSI as a general-purpose tool to rotate the certs on the system.

CRL Yang leafs only allow for one CRL per server

The proposed YANG below gives the option to provide metadata on one CRL.

        leaf certificate-revocation-list-bundle-version {
            type version;
            description
                "The version of the Certificate Revocation List bundle used by
                this gRPC server.";
        }
        leaf certificate-revocation-list-bundle-created-on {
            type created-on;
            description
                "The timestamp of the moment when the Certificate Revocation
                List bundle was created.";
        }

The certificate chain could be set up as rootCA -> intermediateCA1 -> intermediateCA2 -> leaf. We may have CRL's for each of these CA's but the YANG only allows for one of these CRL's to be in YANG.

credentialz.proto: the same field number in lines 485 and 496

principal_check and version:

  message AuthorizedPrincipalCheck {
    enum Tool {
      TOOL_UNSPECIFIED = 0;
      TOOL_HIBA = 1;
    }
    Tool tool = 1;
    // Options specified for the AuthorizedPrincipalCheck
    repeated Option options = 2;
  }

  // The system role account name (e.g. root). This account must exist.
  string account = 1;
  // How the system authorizes users, either by way of a hard-coded repeated
  // SshAuthorizedUser (authorized_users) or through an AuthorizedUserTool enum.

  // Authorizing users is either through a user keyed explicit list, or can be
  // dynamically determined using a PrincipalCheckTool, perhaps better known as
  // AuthorizedPrincipalCommand.
  oneof user_authorization {
    // Mapping of system user to authorized principals.
    SshAuthorizedUsers authorized_users = 2;
    // A tool and options for dynamic authorized principal checking.
    AuthorizedPrincipalCheck principal_check = 3;
  }
  // `version` contains versioning information that is controlled by
  // the credential manager and reported as-is by the telemetry reporting system
  // (ie, transparent to the device credential service).  Credential managers
  // should choose version strings as discrete as possible to ease alert
  // generation (eg, for credentials sourced from a bundle, the timestamp of
  // the bundle should be used but not the time when the credential is pushed to
  // a particular switch).  Also, such version strings should be persisted by
  // the devices onto non-volatile memory for preservation across system
  // reboots.
  string version = 3;
  // `created_on` contains information when the credentials were created.
  // This information is controlled by the credential manager and reported as-is
  // by the telemetry reporting system (ie, transparent to the device credential
  // service).  Credential managers should use the timestamp of the moment when
  // credential was created, not the time when the credential is pushed to
  // a particular switch).
  // Also, this timestamp should be persisted by the devices onto non-volatile
  // memory for preservation across system reboots.
  // `created_on` is a timestamp: the number of seconds since
  // January 1st, 1970 00:00:00 GMT.
  uint64 created_on = 4;
}

Certz: Need clarification on usage of ExistingEntity

As part of ExistingEntity(https://github.com/openconfig/gnsi/blob/main/certz/certz.proto#L580)message do we also send version and created on fields?.Here it's expected us to just copy the mentioned entity from existing profile. So is it implied that we copy version and created on fields as well for that particular entity from the profile mentioned in the request?

so my question is do we need to any version field checking for this type of rotate request? if yes then how?
Do we need to check versions between the destination profile entity(if present) and source profile entity? for example:

  1. profile being rotated has a CertficateChain with version "v1.1".
  2. profile from which the candidate CertficateChain needs to be copied also has "v1.1" so we reject this request.
    If thats the case then version and created on field should not be sent as part of ExistingEntity request.
    https://github.com/openconfig/gnsi/blob/main/certz/certz.proto#L280
    https://github.com/openconfig/gnsi/blob/main/certz/certz.proto#L542

Accounting proto definition changes for Service issues

Setup the proto content to provide both:

  1. inbound connect to RPC for stream out of Accounting data
  2. plan on the service also supporting the reverse: "send to endpoint X"
    2b) define how you'd configure that
    2c) probably that is 'copy / pasta syslog yang model stuff'
  3. define how messages are handled if the service is not attached:
    3a) throw away messages
    3b) store in cicular buffer (how large?)
    3c) spool to disk/storage for later re-broadcast
    3d) other
  4. update README with some of the details above, and expected operational model.

Need clarity on credentialz.proto host certificate rotation

The ServerKeysRequest message has auth_artifacts that holds a field for 'certificate'. But there are no fields to know the type of certificate. So while writing this new certificate to the file system, how do we establish the file name ? whether it should be /etc/ssh/ssh_host_rsa_key-cert.pub or /etc/ssh/ssh_host_ecdsa-cert.pub?
Is the RPC handler supposed to parse the certificate as in ssh-keygen -L -f and determine the type and do this?

Need clarity on trust_bundle structure in certz.proto

I have following questions regarding the trust_bundle mentioned in certz.proto.

  1. trust_bundle is mentioned to be a single CertificateChain, instead shouldn't it be multiple CertificateChains or just a bundle of certificates? The reason I am saying that is, consider the case of two gnsi clients:

    • client1 with it's certificate as client1_cert which is issued by RootCA1. client1_cert <--- RootCA1
    • client2 with it's certificate as client2_cert which is issued in the following way: client2_cert <--- InterCA2 <--- RootCA2
    • For both the gnsi clients to be able to connect to Target, the SSL profile which is being used by Target should contain both RootCA1, InterCA2 and RootCA2 as part of it's trust_bundle. But from the current trust_bundle definition either RootCA1 or (InterCA2<---RootCA2) can be present in the SSL profile, so only one of those clients can connect to Target at any point of time. I think in a general situation, at any point of time the Target should be able to connect with various gnsi clients which have certificates issued by different independent Root/Intermediate CAs.
  2. Can a SSL profile contain more than one trust_bundle? From https://github.com/openconfig/gnsi/blob/main/certz/certz.proto#L80, it appears that only one trust_bundle is supported, but there is a possibility to mention multiple trust_bundle entities in a single UploadRequest, in that case which trust_bundle should the Target install if SSL profile should contain only 1 trust_bundle?

gnsi.accounting.v1 SessionInfo.system_address

Which address should this be if there are multiple or multiple address families (AFs)? Some NOS specify source-address, and what becomes the "system address", by an interface ("source-address interface loopback0") rather than defining the address specifically. Which address is used is usually the primary address for the AF matching the destination address for an out-bound connection.

The current definition of SessionInfo.system_address does not permit more than one address to be specified and does not dictate the behavior of which address should be used if multiple addresses or AFs exist.

One fix is to dictate that the primary address of the AF matching the AF of the gnsi client be used, unless one does not exist. A better fix is to change this to a repeated field and include all the primary system address of all AFs.

Version handling in credentialz.proto

In gNSI, under authz, pathz, certz there seems to be an explicit mention to respond with a "ALREADY_EXISTS"/"AlreadyExists" when an RPC requests with the same version is being pushed again.
There seems to be a 'force_overwrite' flag that when set would ignore the above and allow the request even if the version is already in use.

There seems to be no such definition in credentialz.proto for the version string. Is this accidental or intentional? Is version and timestamp info only for telemetry data in credentialz? OR
Does credentialz also need to check the version in the incoming request and drop the request if the version already exists ? If so, is there a plan to add a 'force_overwrite' flag in credentialz.proto ?

Also, should version information from bootstrap configuration be persisted? or is it only required for Rotate RPC requests that come later after initial boot configuration?

Certz should permit delivery of a certificate bundle as a PKCS#7 encoded string.

Neither delivery as 'certificate' (because that requires items a user of a CA may not have for a CA cert bundle) nor as 'certificateChain' (because the certificates which make up the trust bundle are not chains, nor are they chained to something) are appropriate for some trust-bundle having users.

Permit a new type of message, a string which is a pkcs#7 encoded thing to be delivered to the endpoint.

If credentialz is enabled, other credential configuration should be disabled

For the operational use case where a 'security actor' is in control of credentials and a 'second actor' is in control of all other device configuration, it is desirable that the 'second actor' can perform gNMI SetRequest replace operations without overwriting or conflicting with gNSI managed state. One place where there is overlap between gNMI managed config and gNSI is credentialz.

To enable this use case, should we introduce a method in gNSI which will instruct the device to disable configuration of credentials via gNMI? When disabled, should the device silently ignore replace requests for paths within /openconfig-system:system/aaa/authentication/config?

Thoughts?

Description for gnsi.accounting.CommandService.cmd and cmd_args seem too vague

The description of CommandService.cmd and cmd_args seem too vague here:
https://github.com/openconfig/gnsi/blob/main/accounting/acct.proto#L147-L151

Like TACACS+ & RADIUS, the accounting should be the fully expanded command and its arguments. eg: if a user enters
"sh ver det"
this would be expanded to
"show version detail"

How are arguments split? is a '|' or other punctuation handled specially?

Would there be special handling for the types in #94 ?

Vague documentation leads to variations in implementations and unnecessary document searching that are annoyances to operators and implementers, imiho.

Please clarify queries related to acct.proto file

Could you please clarify below queries related to acct.proto file. Thank you.

  1. As per the proto file, ACK will be sent for a group of accounting records. Since, the connection is over a grpc channel that ensures delivery of the messages, what is the necessity for sending these ACKs?

  2. In addition to the ACK sent for a group of accounting records, there is also an ACK sent at least every 60 seconds and the connection gets reset if an ACK is not received for 120 seconds. What is the rationale behind sending this keepalive ACK since there is already ACKs at lower layers (grpc/tcp)?

  3. When a record request is received with last message received timestamp, all the messages after the timestamp need to be sent out. This happens when the connection is alive and not during connection initiation. Are there any scenarios where collector asks for the past timestamp because there are chances of retransmission of same packets by accounting service?

  4. When a record request is received with zero time, all the messages in the system need to be transmitted. What is the number of messages that we are looking to store in the system?

  5. Is it expected to store the messages in the system even after the message is successfully sent out?

  6. For accounting push, when the config gets added should the accounting service send a dummy record with just the timestamp to the collector. What will be the response from the collector for this dummy message with timestamp?

  7. Required functionality could be achieved with accounting pull alone and there seems to be no necessity for accounting push. For accounting push, accounting service needs to do a dial out and collector IP/port config, TLS certificate params has to be added under accounting service config and accounting service has to read the config and initiate a dial out connection which might be unnecessary.

Certz: Need clarification on usage of ExistingEntity

As part of ExistingEntity(https://github.com/openconfig/gnsi/blob/main/certz/certz.proto#L580)message do we also send version and created on fields?.Here it's expected us to just copy the mentioned entity from existing profile. So is it implied that we copy version and created on fields as well for that particular entity from the profile mentioned in the request?

so my question is do we need to any version field checking for this type of rotate request? if yes then how?
Do we need to check versions between the destination profile entity(if present) and source profile entity? for example:

  1. profile being rotated has a CertficateChain with version "v1.1".
  2. profile from which the candidate CertficateChain needs to be copied also has "v1.1" so we reject this request?
    If thats the case then version and created on field should not be sent as part of ExistingEntity request.
    https://github.com/openconfig/gnsi/blob/main/certz/certz.proto#L280
    https://github.com/openconfig/gnsi/blob/main/certz/certz.proto#L542

Need a 'class/group' field in the user credentials

As part of user accounts under AuthorizedKeysRequest->AccountCredentials and AuthorizedUsersRequest->UserPolicy, a 'class' field is required to ADD a "new" user account. The 'class' field is of type string that defines a class name. The 'class' is used to associate the user account with the 'class'. The 'class' defines the permissions for the 'user' account. In order to handle "new" user account creation with a 'credentials' configuration pushed through bootz (bootz.proto), a user 'class' is required. Or an acceptable default 'class' will have to be used, where the "default" class has the necessary permissions that can be applied to all user accounts created through RPCs via gnsi-credentialz. Request is to make provision in credentialz.proto to be able to specify a user 'class' under "AccountCredentials" and "UserPolicy".

gnsi.accounting.SessionInfo is lacking in various ways

I presume the local/remote fields are for the connection that triggered the accounting message.

system_address should either be local_address, matching local_port, or both system_address and local_address should exist, the latter being the address that the client used as the destination address (which could be any interface on the device). These fields might hold the same address, which is fine, and clearly having the loopback ("system address") of the device (or VRF) is useful.

What is SessionInfo.layer4_proto? Is that OSI layer 4, so TCP/UDP/SCTP? Add comments.

It is not clear what channel_id contains. Looking at ssh docs/etc, a channel id appears to be an integer; rfc4254 S3. Is this correct? The field should be commented appropriately for what the field contains, imiho.

authz ProbeResponse action enum

Is it correct to assume that ACTION_UNSPECIFIED should never be returned in a ProbeResponse message?
I.e. it was added as an enum best practice and does not carry any logical meaning (such as an implicit "no match / deny" rule vs an explicit deny match for ACTION_DENY).

gnsi/authz/authz.proto

Lines 226 to 232 in 26b2e73

message ProbeResponse {
// Action is the defined action for an gRPC-level Authorization Policy.
enum Action {
ACTION_UNSPECIFIED = 0;
ACTION_DENY = 1;
ACTION_PERMIT = 2;
}

cc @morrowc @tomek-US

Certz does not have counters per-profile

certz supports multiple ssl profiles (RotateCertificateRequest.ssl_profile_id). It also maintains counters exposed as
augment /oc-sys:system/oc-sys-grpc:grpc-servers/oc-sys-grpc:grpc-server/oc-sys-grpc:state:

This structure does not appear to be per-ssl_profile_id. So,

  1. it is global, which is not logical given its fields, or
  2. it needs a ssl_profile_id field and a list keyed by that id, in yang parlance.

2 seems to be the answer, but I'm not sure what implementation to suggest here.

Is the cert key comment correct?

gnsi/certz/certz.proto

Lines 61 to 62 in 3a36ea3

// client-generated CSR. In the former case the client has to provide
// coresponding private key with the signed certificate.

Must it provide the certificate key when the CSR is target generated (as in the current version), or should this be when the certificate is CSR is client generated? I think s/former/latter/ is the intended text.

Use of json for policy in authz without a protobuf alternative is obtuse

Use of json for policy in authz without a protobuf alternative is obtuse. Why use json at all? And, why not offer a native protobuf alternative?

Use of JSON seems inconsistent and unnecessary. If the server wishes to use JSON internally, that is its prerogative, but should not be part of the API, or the user should have the option of using either.

My suggestion is that at the very least, a native protobuf version should be supported. This is not my call though; @tomek-US probably needs to decide this.

This is a split of issues/99; #99 (comment)

certz default profiles

  1. Does the default profile have to be named "gNxI"?
    The spec implies so, but is there a reason to require a specific profile name to be used by all implementations? Usually, openconfig is not opinionated on the naming schemes. Also, this is not a new feature - there are existing implementations that support default profiles for grpc-based services, so the renaming will introduce an NBC change for all customers and deployments.

  2. .proto and the readme have slightly different definitions of the rotate request behavior with an absent ssl_profile_id field: proto says to rotate the current profile

    gnsi/certz/certz.proto

    Lines 287 to 291 in a80d61c

    // An identifier for the specific SSL profile (collection of
    // certs/bundles/CRLs) which is being rotated through this stream.
    // Leaving this field blank means that this stream will rotate the SSL profile
    // which is currently being used by the gNSI service.
    string ssl_profile_id = 2;

    and the readme refers to the GnXi profile by name

    gnsi/certz/README.md

    Lines 44 to 46 in a80d61c

    the gNxI server. Its ID is `gNxI` but when the `ssl_profile_id` field in the
    `RotateCertificateRequest` message is not set (or set to an empty string) it
    also refers this SSL profile.

  3. is it required to support certz rotation for the default profile? can the cert rotation be supported only for profiles added via certZ?
    e.g. in situations where the default profile uses idevID/IAK, it may not be trivial to implement the said rotation

Inconsistency between documentation and protobuf definition for groups with members

Description:
The documentation in authorization-README.md mentions that groups can have members. However, upon reviewing the authorization.proto file, it appears that groups are defined without support for members.

Steps to Reproduce:

Visit the documentation page authorization-README.md.
Observe the section mentioning groups with members.
Navigate to the protobuf definition file authorization.proto.
Notice that groups are defined without any provision for members.

snippet:

group {
name: "family-group"
members { name: "stevie" }
members { name: "brian" }
}

Is members supported under groups ..?

Specification around timeout for FinalizeRequest

All the sub-projects under gNSI have a similar flow in the Rotate() which expects FinalizeRequest after the test/validation stage.
The client performs the test/validation parallel to the existing Rotate() stream.
All good if the client sends FinalizeRequest.

How long should the server wait if the client does not send FinalizeRequest?

Having a vendor-specific wait defeats the purpose of vendor neutrality with respect to timeout.
At the same time, having infinite wait on the server side is impractical because if a buggy client forgets to send 'FinalizeRequest' and keeps the stream open, it stays in the same state forever and no new Rotate() can be performed since the first Rotate() is still alive.
This also depends on the time it takes for the client to perform a test/validation.

Either of the below should be fine but this needs to be part of the spec:

  1. Have timeout defined to handle buggy clients.
  2. (OR) Assume that the client always sends FinalizeRequest and have an infinite wait on the server side.

Any specification/guideline in this regard will be helpful for both the server and client implementors.

Need clarity on authentication-policy under certz.proto

Hi,

I need some clarification on authentication policy. what exactly will be send as part of rotation of AuthenticationPolicy? will it be the some sort of certificate identifier(like sha-1 signature or RDN) for issuing CA which will be allowed to sign the leaf certificate for that particular SSL profile?

Thanks,
Manik

Clarify expectation of trust_bundle with regards to mTLS

When confiuring an SSL proile on the switch for mTLS there is generally two thing in particular done

  1. A particular CA is marked as trusted. This will be used to authenticate the clients certificate and does not necessarily have to be part of the server certificate chain.
  2. A certificate chain is formed on the server with a leaf certificate and 0 or more intermediate certs.

The proto comments contains the following

// Target (as seen from gNSI.certificate microservice point of view)
// |
// +-+ certificate (used by all gRPC services)
// | +- certificate (with public key)
// | +- private key
// |
// +-+ trust bundle (Certificate Authority certificates)
// | +- CA Root certificate
// | +- CA Intermediate Certificate

This would indicate to me that all CA's as part of the trust bundle are only to be marked as trusted and the server will only have one certificate used in it's chain.

I think a reasonable change to this would be to change

  oneof entity {
    Certificate certificate = 3;
    CertificateBundle trust_bundle = 4;
    CertificateRevocationListBundle certificate_revocation_list_bundle = 5;
  }

to

  oneof entity {
    Certificate leaf_certificate = 3;
    CertificateBundle intermediate_certificates = 4;
    CertificateBundle trust_bundle = 5;
    CertificateRevocationListBundle certificate_revocation_list_bundle = 6;
  }

The private key would be optional for all certs in the chain except for the leaf certificate. This is already marked as optional so we are good here.

Clarify usage of ID field in CRL

Certificate Revocation Lists has the following definition.

// A certificate revocation list (CRL)
message CertificateRevocationList {
  // Type of the CRL.
  CertificateType type = 1;
  // CRL encoding type.
  CertificateEncoding encoding = 2;

  // Actual CRL.
  // The exact encoding depends upon the type of CRL.
  // for X509, this should be a PEM encoded CRL.
  bytes certificate_revocation_list = 3;

  // ID of this CRL, which is the CRL file hash.
  string id = 4;
}

It is not clear what the purpose of the ID is here. The comment indicates it is a file hash, is it referring to sha256sum, x509 hash etc.

What is the expected usage of this ID. Is this supposed to be the name of the CRL in the filesystem? If so, I feel like it might be better to leave this as an implementation detail up to the server. Or if this really is wanted, then should we have a similar field in Certificate, which we currently don't have?

Or perhaps the hash is used for some form of validation where we, the server, ensure the OpenSSL hash is the same as the hash provided in the spec, this also seems suboptimal as hashing algorithms could be subject to change.

It might be reasonable to remove this field altogether and let the server decide how it wants to name the CRL on the box.

Rotation of hostkeys/certificates using key generated on the switch

The current message ServerKeysRequest does not allow rotating and using a hostkey which has been generated on the switch. This is an important use case which was also present in the original specification. Hence we wanted to propose the below changes to the proto which allows setting both the ssh hostkeys and certificates in a consistent manner.

  1. Change the name and contents of GenerateKeysRequest to the below since the message is no longer just instructing to trigger key generation on the switch.
message CreateKeysRequest {
    oneof req {
        KeyGen key_params = 1;
        bytes private_key = 2;
    }
}
  1. Change the contents of AuthenticationArtifacts to the below
message AuthenticationArtifacts {
    oneof artifact {
        PublicKey hostkey_public_key = 1;
        bytes certificate = 2;
    }
}

Below will be the new workflow for the different scenarios

Rotation of ssh host keys using externally generated keys

  1. Client sends CreateKeysRequest with the desired private key. Server replies back with CreateKeysResponse which has the public key corresponding to the private key.
stream.Send(
    RotateHostCredentialsRequest {
        create_key_request: CreateKeysRequest {
            private_key: "A....=",
        }
    }
)
  1. Client sends AuthenticationArtifacts with the public key which was received in the previous step. This will add the hostkey to config.
stream.Send(
    RotateHostCredentialsRequest {
        server_keys: ServerKeysRequest {
            auth_artifacts: AuthenticationArtifacts {
                hostkey_public_key: "A....=";
            }
            version: "v1.0",
            created_on: 3214451134,
        }
    }
)

Rotation of ssh host keys using keys generated on the switch

  1. Client sends CreateKeysRequest with the key specification. Server generates the key and replies back with CreateKeysResponse which has the public key corresponding to the generated private key.
stream.Send(
    RotateHostCredentialsRequest {
        create_key_request: CreateKeysRequest {
            key_params: KEY_GEN_SSH_KEY_TYPE_RSA_2048,
        }
    }
)
  1. Client sends AuthenticationArtifacts with the public key which was received in the previous step. This will add the hostkey to config.
stream.Send(
    RotateHostCredentialsRequest {
        server_keys: ServerKeysRequest {
            auth_artifacts: AuthenticationArtifacts {
                hostkey_public_key: "A....=";
            }
            version: "v1.0",
            created_on: 3214451134,
        }
    }
)

Rotation of ssh host keys and certificates using using externally generated keys

  1. Client sends CreateKeysRequest with the desired private key. Server replies back with CreateKeysResponse which has the public key corresponding to the private key.
stream.Send(
    RotateHostCredentialsRequest {
        create_key_request: CreateKeysRequest {
            private_key: "A....=",
        }
    }
)
  1. Client generates a certificate using the public key and sends AuthenticationArtifacts with the certificate. This will add the hostkey and certificate to config as an atomic operation.
stream.Send(
    RotateHostCredentialsRequest {
        server_keys: ServerKeysRequest {
            auth_artifacts: AuthenticationArtifacts {
                certificate: "A....=";
            }
            version: "v1.0",
            created_on: 3214451134,
        }
    }
)

Rotation of ssh host keys and certificates using keys generated on the switch

  1. Client sends CreateKeysRequest with the key specification. Server generates the key and replies back with CreateKeysResponse which has the public key corresponding to the generated private key.
stream.Send(
    RotateHostCredentialsRequest {
        create_key_request: CreateKeysRequest {
            key_params: KEY_GEN_SSH_KEY_TYPE_RSA_2048,
        }
    }
)
  1. Client generates a certificate using the public key and sends AuthenticationArtifacts with the certificate. This will add the hostkey and certificate to config as an atomic operation.
stream.Send(
    RotateHostCredentialsRequest {
        server_keys: ServerKeysRequest {
            auth_artifacts: AuthenticationArtifacts {
                certificate: "A....=";
            }
            version: "v1.0",
            created_on: 3214451134,
        }
    }
)

authz: Clarity needed for few operations DURING Rotate() in progress

In the authz spec, it is mentioned that the pushed policy becomes active immediately as below:

Because the policy uploaded during the gNSI.authz.Rotate() call becomes active
immediately, the gNSI.authz.Probe() can be used to check if the uploaded
policy provides the expected response without attempting performing the
(potentially destructive) RPC in question while the gNSI.authz.Rotate() is
still active (the stream is opened and the Finalize message has not been sent
yet.

This is fine for Probe() RPC because it has to be tested before finalize.

When the Rotate() is in progress (After upload and before finalize) what is the expectation for the below two use case:

  1. A real RPC such as /gnmi.gNMI/Get or /gnmi.gNMI/Subscribe. Should this be authorized based on the newly pushed sandbox policy (which has not yet received finalize maybe because the test phase is taking time) or the current active policy. Is the "Step 3 (optional): Test/Validation by the client." performed only using Probe() RPC or does the controller fires the actual RPC also to perform the test?
  2. What should Get() RPC return in this intermediate state? Unlike pathz, the GetRequest does not take the PolicyInstance parameter.

Clarity needed (in a comment) for acctz RecordRequest keepalives

Since the keepalives are not ACKs of any sort, clarity should be added about the timestamp in the KA RecordRequest.
What should it be set to?
Should the server ignore it? Else what is the behavior? If it is ignored, how does a client stop the stream and restart it from a particular timestamp.
Also reference issues/77

authz default behavior should not be vendor prerogative

From authz/README.md, this is a bad choice, imo:

"A default gRPC-level Authorization Policy for every active gRPC service.

The initial default gRPC-level Authorization Policy can either allow access to all RPCs or deny access to all RPCs except for the gNSI.authz family of RPCs."

The default behavior should be prescribed and it probably should be 'permit' for the element of least surprise. Vendors choosing their default leads to operators needing to remember which vendor does what, like what is the default at the end of route-map vs a policy-statement, and be sure to have policy appropriate for each.

Also, this default policy makes no mention of the disposition of an authorization if a policy is defined and matches neither deny_rules or allow_rules.

And, there is no mention in the prot of the json schema for rules, and the README points to an MR rather than the actual proto.

And, why are rules in json rather than just a proto? Or, why isnt there a method to just use a normal proto w/o json serialization?

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.