[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Splitting lib/krb5/pkinit.c
Yes it is complex, and may be overkill. But rather then using the engine, I would like to
see PKCS11 used instead. The main use of PKINIT will be with smart cards, and smart cards
come with a PKCS11. Browsers and other applications already know how to use PKCS11,
and smartcard vendors provide PKCS11 libs. If you look close at the examples the engine code
is loading and calling PKCS11.
PKCS11 is a much cleaner well developed interface then the engine code. (The engine code
in turn can call a pkcs11, like the OpenSC which is what I was using. This is what makes
it look so complicated.)
(More comments below.)
Geoff Elgey wrote:
> 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
Not really the private key, but access to the signing function that can use the key.
With a smartcard, the key never leaves the card.
> * the user's X.509 certificate(s) for sending to the server
> * the user's trusted certificates for verifying 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).
>
Please do not hard code, put in a seperate file maybe.
> (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?
>
This is the standard OpenSSL way of handling trust certificates. Since they need to be
trusted by the local system there may be security risks in fetching them from a URL.
> The user information required by PKINIT could be abstracted out by using an instance of the following type and functions:
>
PKCS11 abstractions would be an alternative choice here.
> 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]
>
The verify certificate chain, is what OpenSSL can do for you, aqnd it uses the trusted
certificates directory.
> 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 ... }
> ...
>
Keep in mind that the one of the main uses of the engine was to
allow the private key to be used from a smartcard, or other crypto
device, so it may not be as simple as this.
>
> 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).
>
No don't put this burden on the applicaiton. Using PKCS11 makes a lot more sense. It depends on
what is in your krb5_pki_identity. If its a pointer to the pkcs11 lib to use, then OK.
Note that the principal may also be derived from the cert, or mapling of the cert to
a principal.
If the engine to pkcs11 code was removed, and replaced with pkcs11 calls from PKINIT then
that would simplify and standardize the interface.
> 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
>
--
Douglas E. Engert <DEEngert@anl.gov>
Argonne National Laboratory
9700 South Cass Avenue
Argonne, Illinois 60439
(630) 252-5444