NFD » ndn-cxx

Free functions also could be good. My only objective is to have a simple and straightforward syntax for signing operation.

can we remove "signing" prefix? It would be more meaningful to write

keyChain.sign(packet, byCertificate(certName));
keyChain.sign(packet, byIdentity(identityName));
keyChain.sign(packet, byKey(keyName));

It would a little clutter ndn:: namespace, but more clear.

Answer to note-4:

No, they look weird in ndn namespace.

I could put them into ndn::security::signing namespace, but the application needs to manually import them with using.

How do I get the keyName from the keyId?

I have not noticed that before, but signingByKey should accept key name. Identity and "id" are irrelevant for the purpose of signing.

Should KeyChain and other unit tests (modified in #2871) be updated to use signing helpers, instead of creating SigningInfo with the long syntax?

Junxiao Shi wrote:

Should KeyChain and other unit tests (modified in #2871) be updated to use signing helpers, instead of creating SigningInfo with the long syntax?

I would agree with this proposal, as it would simply the test commands. However, it would also require the inclusion of an extra header in the unit test (signing-helpers.hpp).

I forgot about Junxiao's question when I closed this issue. Could it please be changed to Resolved until all questions are sorted out?

what is the harm of not doing so?

I see no reason to do any modifications, as the code is equivalent. The new code can use either, though I would suspect developers would choose to use helpers.

Looking into these functions for another task I am not sure why do we need to supply the name. Shouldn't the interface be like this:

SigningInfo
signingBy(const Name& identity);

SigningInfo
signingBy(const KeyChain& key);

SigningInfo
signingBy(const IdentityCertificate& certificate);

Like this the functions would be more usefull, because at this point these functions only abstract the caller from the enum.

Because identity, key, and certificates all are identified by name. I think what you're suggesting would require additional lookup to keychain, with code size only increasing

At this point we are just using only the identity and the certificate signers and we call the certificate like this:

  security::SigningInfo signingInfo;
  if (!certificate.getName().empty()) {
    signingInfo = signingByCertificate(certificate.getName());
  }

What I was proposing was to remove this logic from every place we are trying to sign and move it to the helper.

The KeyChain at this point have no call, will it be used? How are we getting the name from the KeyChain when we want to call the signer using the signingByKey?

Reply to note-24:

The quoted snippet comes from a \deprecated function to support legacy API.

New code shouldn't load Certificate in order to construct SigningInfo.

Yes you are right, so this code will be moved to the application right? Because the setInterestFilter will be receiving the SigningInfo object already.

Looking at what was said in note-25 I think that it make even more sense to have override helper functions instead of having one function per type.

But if you guys think that this logic should be passed to the application then we can close this discussion

Reply to note-26:

Application shouldn't load Certificate either.

The input that application provides is Name (it is what user supplies either in configuration or command line parameters). Any looking up procedures will be done inside the keychain during thr signing (how to optimize the number of lookups is another good question).

Neither key or certificate can really be used directly for signing operations. To sign one needs to find a corresponding private key, for which we are using name of identity/key/certificate, just in a slightly different ways.