pqc-thunderbird / rnp

Manual clone of the repository https://github.com/rnpgp/rnp
Other
0 stars 0 forks source link

Fix choking of RNP on unknown algorithm in public key #55

Open falko-strenzke opened 7 months ago

falko-strenzke commented 7 months ago

Fix the problem indicated at https://tests.sequoia-pgp.org/#Mock_PQ_subkey

TJ-91 commented 7 months ago

There already is the submissive flag. If it is active, a signature or subkey that produces failures is skipped. When importing keys on the command-line, this is done by adding --permissive to the command. When importing our backwards-compatible key (ECDSA+ECDH+MLKEM) and setting the permissive flag, the ECC keys are imported as intended.

Thunderbird sets this flag to false in several calls, an example from RNP.jsm:

  async importSecKeyBlockImpl(
    win,
    passCB,
    keepPassphrases,
    keyBlockStr,
    permissive = false,
    limitedFPRs = []
  ) {
    return this._importKeyBlockWithAutoAccept(
      win,
      passCB,
      keepPassphrases,
      keyBlockStr,
      false,
      true,
      null,
      permissive,
      limitedFPRs
    );
  },

We should probably present the problem to RNP and Thunderbird and ask what they would like to do about it. If simply defaulting to permissive = true is not acceptable, one could add a separate "skip keys with unknown algorithm IDs" flag that defaults to true (narrower than skipping on all errors).

@falko-strenzke what do you think?

falko-strenzke commented 7 months ago

I think the default behaviour of RNP would have to be "permissive" with respect to unknown algorithm IDs. Otherwise the default behaviour is violating the crypto-refresh requirements. The test interop test suite will then continue to indicate the error.

@ni4 Please look at this issue that we would like to see fixed in RNP.

ni4 commented 7 months ago

@falko-strenzke

Otherwise the default behaviour is violating the crypto-refresh requirements.

We do not claim crypto-refresh support at the moment so I don't see a problem here.

The test interop test suite will then continue to indicate the error.

Don't see a problem here as well - while test suite seems to be good thing to have, it's not something obligatory to follow.

Maybe I miss some details, but why should we by default support the importing of the stuff, which is not expected to be supported and is not known? If user wants to import it, then he may take steps to make it work - like cutting out unexpected subkey or setting the permissive flag.

falko-strenzke commented 7 months ago

The problem that we see is that if some widespread libraries do not support unknown algorithms, this is an argument to not allow PQC with v4 keys. This was already used as an argument ( I think on the openpgp-email list). So we thought that this is worth fixing in RNP, so that from the next release RNP will not choke if later it is confronted with a certificate that contains a v4 PQC subkey.

ni4 commented 6 months ago

@falko-strenzke RNP is aimed to be general-purpose OpenPGP library, where user may control behaviour, in this case via permissive import control. The next level of control would be user's ability to cut out unneded subkey. Regarding argument in openpgp mailing list - I don't agree with many argments which raise in that list. P.S. If openpgp list would care about widespread libraries then they would not dramatically change format in 2021 for almost no real reason :-)