1hub / springburg

OpenPGP for .NET
MIT License
4 stars 0 forks source link

API: Key APIs #24

Open filipnavara opened 3 years ago

filipnavara commented 3 years ago

The key APIs are a bit of a mess - PgpKey, PgpPublicKey, PgpSecretKey, PgpPrivateKey, PgpKeyPair, etc. Same thing for the key rings and key ring bundles.

It's non obvious how these are bound together, when to use which and how to properly handle encoding them so someone doesn't accidentally send the secret key when they meant to send only the public one.

OpenPGPjs seems to have just one key object, which has subkeys tracked along. Key ring refers to what BC calls key ring bundle. The key object represents either a public key, secret key or public key with unlocked secret key (ie. public + private key). The model is far from perfect but it may have few things to learn from it. The obvious downside is that any usage that requires a private key needs to check that an unlocked key was passed in and it is enforced only through runtime checks.

One possible implementation would be to clearly define PgpPublicKey (public asymmetric key, user ids, certifications) and PgpPrivateKey (private asymmetric key only). PgpKeyPair would refer to bundle of PgpPublicKey and PgpPrivateKey. PgpSecretKey would only allow wrapping into / unwrapping from PgpKeyPair with password and getting PgpPublicKey without password. This would, however, still cause some duality for PgpPublicKey since it would hold additional attributes that are not strictly part of the key, and it would not be easy to express subkeys.

Another possible implementation would be to have PgpKey which would be holder for either public key or secret key, with user ids, certifications and possibly subkeys. It would expose methods for getting PgpPublicKey (only public asymmetric key), PgpPrivateKey (only private asymmetric key) with unlocking, and PgpKeyPair (PgpPublicKey + PgpPrivateKey). The downside is that the name PgpKey is quite misleading then. Some APIs also may want both the key/user certifications and PgpPublicKey as an input (eg. when encrypting message) which should preferably be expressed as one object.

filipnavara commented 3 years ago

We should cross-check with what gopenpgp and https://sequoia-pgp.org/ does.

filipnavara commented 3 years ago

Sequoia PGP seems to go with the following model:

filipnavara commented 3 years ago

I am currently leaning towards an API shape like this:

/// <summary>
/// Represents decoded key material for single public key and methods to perform
/// cryptographic operations with it.
/// </summary>
public abstract class PgpPublicKey
{
    protected PgpPublicKey(DateTime creationTime, int version = 4); // -- for versions 4+ we can calculate fingerprint and key id
    protected PgpPublicKey(byte[] fingerprint, long keyId, DateTime creationTime, int version = 4);
    public int Version { get; } // 3, 4, 5
    public byte[] Fingerprint { get; }
    public long KeyId { get; }
    public DateTime CreationTime { get; }
    public abstract PgpPublicKeyAlgorithm Algorithm { get; }
    public abstract bool Verify(ReadOnlySpan<byte> hash, ReadOnlySpan<byte> signature, PgpHashAlgorithm hashAlgorithm);
    public abstract byte[] Encrypt(ReadOnlySpan<byte> sessionData);
    public abstract byte[] Export();
    // CanEncrypt?, CanVerify?
}

// Implementations of PgpPublicKey (eg. RSAPublicKey, DSAPublicKey, etc.)

/// <summary>
/// Represents decoded key material for single private key and methods to perform
/// cryptographic operations with it.
///
/// Where knowledge of KeyId, Fingerprint, or key version is necessary the methods
/// should take PgpKeyPair as an input instead.
/// </summary>
public abstract class PgpPrivateKey
{
    protected PgpPrivateKey(byte[] fingerprint, long keyId);
    public byte[] Sign(ReadOnlySpan<byte> hash, PgpHashAlgorithm hashAlgorithm);
    public byte[] Decrypt(ReadOnlySpan<byte> encryptedSessionData);
}

public abstract class PgpExportablePrivateKey : PgpPrivateKey
{
    public byte[] Export(ReadOnlySpan<byte> password, S2kParameters s2kParameters);
}

// Implementations of PgpPrivateKey (eg. RSAPrivateKey, ...)

/// <summary>
/// Represents bundle of public key and private key.
/// </summary>
public class PgpKeyPair
{
    public PgpKeyPair(PgpPublicKey publicKey, PgpPrivateKey privateKey);
    public PgpKeyPair(AsymmetricAlgorithm asymmetricAlgorithm, DateTime creationTime, int version = 4);
    public int Version => PublicKey.Version;
    public byte[] Fingerprint => PublicKey.Fingerprint;
    public long KeyId => PublicKey.KeyId;
    public DateTime CreationTime => PublicKey.CreationTime;
    public PgpPublicKey PublicKey { get; }
    public PgpPrivateKey PrivateKey { get; }
}

/// <summary>
/// Represents a key in a transportable form, either as a public key packet or
/// a secret key packet.
///
/// It allows materializing the public key into one that can be used to perform
/// cryptographic operations.
///
/// If secret key material is present it allows extracting the private key from
/// it with a provided password and materializing it into one that can be used
/// to perform cryptographic operations.
///
/// Additionally, if the key has any attached certifications (subkey binding,
/// revocation) they are available through this object.
/// </summary>
public class PgpTransportableKey
{
    public PgpTransportableKey(PgpPublicKey publicKey);
    public PgpTransportableKey(PgpKeyPair keyPair, ReadOnlySpan<char> password, S2kParameters s2kParamaters);
    public PgpTransportableKey(PgpKeyPair keyPair, ReadOnlySpan<byte> rawPassword, S2kParameters s2kParamaters);

    public PgpPublicKey GetPublicKey();

    public bool HasPrivateKey { get; }
    // public PgpPrivateKey GetPrivateKey(ReadOnlySpan<char> password);
    // public PgpPrivateKey GetPrivateKey(ReadOnlySpan<byte> rawPassword);
    public PgpKeyPair GetKeyPair(ReadOnlySpan<char> password);
    public PgpKeyPair GetKeyPair(ReadOnlySpan<byte> rawPassword);
    public PgpTransportableKey CopyWithPassword(ReadOnlySpan<char> oldPassword, ReadOnlySpan<char> newPassword, S2kParameters s2kParamaters = null);
    public PgpTransportableKey CopyWithPassword(ReadOnlySpan<byte> rawOldPassword, ReadOnlySpan<byte> rawNewPassword, S2kParameters s2kParamaters = null);

    // TODO: Certfications, Revocations
}

// TODO: Maybe split PgpTransportableKey into PgpTransportableSecretKey

public class PgpUser
{
    // Self-signature attributes 
    public PgpSignatureAttributes Attributes { get; }
    public PgpCompressionAlgorithm[]? PreferredCompressionAlgorithms => Attributes.PreferredCompressionAlgorithms;
    public PgpHashAlgorithm[]? PreferredHashAlgorithms => Attributes.PreferredHashAlgorithms;
    public PgpSymmetricKeyAlgorithm[]? PreferredEncryptionAlgorithms => Attributes.PreferredEncryptionAlgorithms;
    // TODO: Shorthands for other attributes like Key flags, Features 
    // TODO: Counter-signatures, revocations
}

public class PgpUserId : PgpUser
{
     public string Id { get; }
     // TODO: Name, Email for convenience?
}

public class PgpUserAttributes : PgpUser
{
    // TODO: ...
}

public class PgpCertificate
{
    public PgpCertificate(byte[] data, bool armored);
    public PgpCertificate(Stream data, bool armored);

    public PgpUserId? GetPrimaryUserId(); // -- always returns verified user
    public IEnumerable<PgpUserId> GetUserIds();
    public IEnumerable<PgpUserAttributes> GetUserAttributes();
    public IList<PgpUser> UnverifiedUsers { get; }

    public PgpTransportableKey PrimaryKey { get; }
    public IList<PgpTransportableKey> Subkeys { get; }
    public PgpTransportableKey GetKey(long keyId);
    public PgpTransportableKey GetKey(byte[] fingerprint);

    // TODO: certifications, revocations
    // TODO: add/remove subkey, revoke

    public void Write(Stream outputStream, bool armored, bool includeSecretKey = false);
}

Still not liking it though. There are two usable decrypted/decoded classes - PgpPublicKey and PgpKeyPair (yes, key pair, not PgpPrivateKey, since PgpPrivateKey is not usable by itself; it only exists so the implementation can be replaced with an external one like key card) but only one encoded one.

filipnavara commented 3 years ago

The more I think about it the more I like the idea of separating the decoded key implementations from the transportable encoded ones, whatever the names are. The problem is that the decoded public key still needs to carry all the fingerprint information which is dependent on the packet version and encoding.

filipnavara commented 3 years ago

I keep going back and forth on the design of this and how to separate the internal implementations from the actual public facing classes. For the majority of cases one will deal with transferrable keys or ones that were generated as new key pair. The new key pair's public key necessary has to be converted to transferrable key format before usage because of how its key id and fingerprints are generated. The only reason to expose some of the internal implementation is to allow plugging-in implementations of new/legacy algorithms (a non-goal but nice to have) and to allow external private keys (smart cards).

The only thing that I am nearly certain about is that the current PgpKeyRing should become PgpCertificate and it should be a holder for the keys and user ids. The user ids should not be part of the key because they logically are only present along with the primary key. It violates law of demeter to pass them along. Encryption subkeys would not have them but they are necessary to make the correct decision about what symmetric algorithm to use.

I decided to summarise down what are the actual requirements for different operations and which have to be provided by the key API:

Overall I am slowly reaching the conclusion that the public key representation and public key algorithm implementation are basically inseparable. Places where private key is used also require the knowledge of key id which in effect means an implied link to a public key representations (but strictly speaking only KeyId is necessary).