Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Security Implementation #107

Open
wants to merge 67 commits into
base: main
Choose a base branch
from
Open

[WIP] Security Implementation #107

wants to merge 67 commits into from

Conversation

pulsejet
Copy link
Collaborator

I realise this might have gotten too out of hand to review effectively.

WIP summary:

  1. Refactor all signers to a separate package ndnd/std/security/signer
  2. More tests for signers
  3. Private key marshaling as a named NDN Data packet
  4. Add readable PEM encoding support for our keys and certs and tooling (ndnd sec; docs in docs/sec.md)
  5. Library and tooling support to sign new NDN certificates
  6. Removing separate signed interest signers for two reasons
    a. Let's forget signed Interest if we can
    b. Forget about preventing replay (nonce / seq / time fields)
  7. Simple keychain implementation that stores keys and certs as a bunch of PEM files in a folder. Associated tooling in ndnd sec
  8. LightVerSec implementation (wip)

Security tool usage (as of writing this)

Usage: ndnd sec [command]
  keygen               Generate a new NDN key pair
  sign-cert            Sign a new NDN certificate
                       
  keychain list        List keys in a keychain
  keychain import      Import keys or certs to a keychain
  keychain get-key     Get a key from a keychain
  keychain get-cert    Get a certificate from a keychain
                       
  pem-encode           Encode an NDN data to PEM representation
  pem-decode           Decode a PEM representation of an NDN data

Example PEM cert and key

-----BEGIN NDN CERT-----
Key: /ndn/KEY/%27%C4%B2%2A%9F%7B%81%27
Name: /ndn/KEY/%27%C4%B2%2A%9F%7B%81%27/ndn/v=1651246789556
SigType: ECDSA-SHA256
Validity: 2022-04-29 15:39:50 +0000 UTC - 2026-12-31 23:59:59 +0000 UTC

Bv0BSwcjCANuZG4IA0tFWQgIJ8SyKp97gScIA25kbjYIAAABgHX6c7QUCRgBAhkE
ADbugBVbMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEPuDnW4oq0mULLT8PDXh0
zuBg+0SJ1yPC85jylUU+hgxX9fDNyjlykLrvb1D6IQRJWJHMKWe6TJKPUhGgOT65
8hZyGwEDHBYHFAgDbmRuCANLRVkICCfEsiqfe4En/QD9Jv0A/g8yMDIyMDQyOVQx
NTM5NTD9AP8PMjAyNjEyMzFUMjM1OTU5/QECKf0CACX9AgEIZnVsbG5hbWX9AgIV
TkROIFRlc3RiZWQgUm9vdCAyMjA0F0gwRgIhAPYUOjNakdfDGh5j9dcCGOz+Ie1M
qoAEsjM9PEUEWbnqAiEApu0rg9GAK1LNExjLYAF6qVgpWQgU+atPn63Gtuubqyg=
-----END NDN CERT-----

-----BEGIN NDN KEY-----
Name: /ndn/alice/KEY/%CD%1B%1F%13Dv%F0%2B
SigType: Ed25519

BsoHGwgDbmRuCAVhbGljZQgDS0VZCAjNGx8TRHbwKxQDGAEJFUDreGcTiQ9PS5EV
JgZ6LKtFb0qTRWg2Vo9QeC+642ad0Amduz+O6qAc9T0Ti2hzUwWOjUZUvA+9AW2r
qh7UCc/kFiIbAQUcHQcbCANuZG4IBWFsaWNlCANLRVkICM0bHxNEdvArF0BQ5B6b
QoAr+QL/X3QMb3a1pS9F1bpEjJcw5NVE/eHSgO2vq/2RVd1NXL11L95vYeMwOVjW
tKUvla/XLA2jGVwI
-----END NDN KEY-----

@pulsejet pulsejet added the enhancement New feature or request label Jan 18, 2025
@pulsejet pulsejet self-assigned this Jan 18, 2025
@zjkmxy
Copy link
Member

zjkmxy commented Jan 18, 2025

Thank you for your contribution! I will use the long weekend to read it.

// Sign computes the signature value of a wire.
Sign(enc.Wire) ([]byte, error)
// Public returns the public key of the signer or nil.
Public() ([]byte, error)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to separate Signer with PrivateKey. Signer executes the signing action with a given key, its fields are related to the signing action (sequence number, timestamp, nonce, etc.), instead of the key itself.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, renaming.

sequence number, timestamp, nonce, etc.

Trying to eliminate these if possible. We shouldn't be using signed interest that can be replayed. SVSv3 is the first to move away.

Signer still will have needs the extra KeyLocator field / method.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm on starting the refactor, I realised this won't fit the SHA256 signer (since there is no key). Then we'd need either another name / implementation for SHA256 signer, or a separate signer implementation (want to avoid this complexity).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sequence number, timestamp, nonce, etc.

Trying to eliminate these if possible. We shouldn't be using signed interest that can be replayed. SVSv3 is the first to move away.

I don't follow. SeqNum/Nonce/Timestamp were designed to prevent replay attacks.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the intention is to eventually eliminate signed interest.

For now I don't plan to support any of these optional fields in NDNd.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

understood

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants