G'day,
One thing that I do not like about lib/krb5/pkinit.c is that this file contains both the PKINIT logic and openssl engine configuration logic.
I propose that the openssl engine configuration should be separated from pkinit.c, and that the PKINIT logic should use an abstraction for obtaining the user's PKI data. This abstraction may use openssl engine ogic, or other user-supplied logic.
Examination of the PKINIT code reveals that the following information is required from the user:
* the user's private key
* the user's X.509 certificate(s) for sending to the server
* the user's trusted certificates for verifyig the server's certificates
* the user's CRL's for verifying certificates (currently unused)
Currently this information is obtained as follows:
(1) By supplying the "user_id" that identifies the private key and certificate of the user. In turn, this "user_id" is supplied to a custom openssl engine that is used to load a private key and a certificate. The configuration data for the openssl engine is located in krb5.conf
(2) By supplying the location of an 'anchors' directory for trusted certificates. The certificates are PEM-encoded files whose names must conform to an 8-character hash part ending in ".0"
I have the following problems with this approach:
(1) Complexity: For my application, I didn't want to add openssl configuration to the user's krb5.conf. It's overly complex, and there's enough steps that can go wrong without worrying about incorrect openssl configuration files. Better to hide this information where possible -- possibly by hardcoding the engine configuration data (such as the engine name and "LOAD" command).
(2) Inflexibility: The requirement of locating trusted certificates as PEM-encoded files in a single directory seems to be an unncessary restriction to me. There's no reason that the trusted certificates could not be fetched from a URL, or be DER-encoded, etc. In addition, the requirement of using an openssl engine is also overly restrictive. If I can create the appropriate EVP_PKEY and an X509 values, why do I need to introduce the complexity of configuring and loading an openssl engine?
The user information required by PKINIT could be abstracted out by using an instance of the following type and functions:
typedef struct krb5_pki_identity krb5_pki_identity;
typedef struct {
int (*get_private_key)(krb5_pki_identity *id,
EVP_PKEY **p_key);
int (*get_user_certs)(krb5_pki_identity *id,
STACK_OF(X509) **p_certs);
int (*get_trusted_certs)(krb5_pki_identity *id,
STACK_OF(X509) **p_certs);
int (*get_crls)(krb5_pki_identity *id,
STACK_OF(X509_CRL) **p_crls);
void (*destroy)(krb5_pki_identity *id);
} krb5_pki_identity_methods;
struct krb5_pki_identity {
const krb5_pki_identity_methods *methods;
void *private_data;
};
/* Get the user's private key */
int krb5_pki_identity_get_private_key(krb5_pki_identity *id,
EVP_PKEY **p_key);
/* Get the user's certificate chain */
int krb5_pki_identity_get_user_certs(krb5_pki_identity *id,
STACK_OF(X509) **p_certs);
/* Get the user's trusted certificates */
int krb5_pki_identity_get_trusted_certs(krb5_pki_identity *id,
STACK_OF(X509) **p_certs);
/* Get the CRLs for certificate verification */
int krb5_pki_identity_get_crls(krb5_pki_identity *id,
STACK_OF(X509_CRL) **p_crls);
int krb5_pki_identity_destroy(krb5_pki_identity *id);
[In fact, we could abstract further, by replacing the 'get_trusted_certs' and 'get_crls' functions with a single 'verify_certificate_chain' function, if desired]
In the PKINIT code, references such as :
EVP_PKEY *private_key = id->private_key;
...
would then be replaced with calls such as:
EVP_PKEY *private_key = NULL;
ret = krb5_pki_identity_get_private_key(id, &private_key);
if (ret != 0) { ... error ... }
...
It is up to the application to provide an appropriately populated krb5_pki_identity value, which is responsible for loading the private key (possibly prompting for a passphrase or PIN), the user's certificate chain, the trusted certs, etc. A default implementation may be provided that loads an openssl engine and reads trusted certs from an 'anchors' directory, as is currently the case. Other applications (such as mine) may return this information via other mechanisms, if appropriate (I use PKCS#11, and do need to create an openssl engine).
With this new type, the krb5_get_init_creds_opt_set_pkinit function can then be simplified from:
krb5_error_code KRB5_LIB_FUNCTION
krb5_get_init_creds_opt_set_pkinit(krb5_context context,
krb5_get_init_creds_opt *opt,
krb5_principal principal,
const char *user_id,
const char *x509_anchors,
int flags,
krb5_prompter_fct prompter,
void *prompter_data,
char *password)
to:
krb5_error_code KRB5_LIB_FUNCTION
krb5_get_init_creds_opt_set_pkinit(krb5_context context,
krb5_get_init_creds_opt *opt,
krb5_principal principal,
krb5_pki_identity *id,
int flags)
This would be called as follows:
krb5_pki_identity *id = NULL;
/* Create a PKI identity that uses an openssl engine to obtain
the private key and certificate, loads anchors from the given
directory, and prompts for the passphrase/PIN using the given
prompter. */
if (create_engine_id(&id, user_id, x509_anchors,
prompter, prompter_data) != KRB5_SUCCESS)
{
... error ...
}
assert(id != NULL);
/* Set PKINIT so that it uses the PKI id */
krb5_get_init_creds_opt_set_pkinit(krb5_ctx, opts, id, flags);
... perform PKINIT authentication ...
/* Destroy the PKI identity */
krb5_pki_identity_destroy(id);
Of course, this would break compatibility, but offers significant flexibilty.
Thoughts?
-- Geoff