httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jan Pazdziora <jpazdzi...@redhat.com>
Subject Re: Using UPN from subjectAltName with SSLUserName
Date Mon, 29 Jun 2015 11:47:45 GMT
On Sun, Jun 28, 2015 at 05:11:57PM +0200, Kaspar Brand wrote:
> On 22.06.2015 10:37, Jan Pazdziora wrote:
> > Please find a new patch attached which I hope covers all the
> > parts you've outlined, for SSL_CLIENT_SAN_OTHER_msUPN_*.
> 
> Thanks. Your implementation assumes that only a single otherName form
> (msUPN) needs to be supported, but I would prefer to code it in a
> somewhat more extensible way.
> 
> Does the attached patch work for you? As a practical way of

Yes it does.

My only question (and comments bellow) is about passing the oid rather
than nid through the functions. I can see the string "id-on-dnsSRV"
used twice in the patch in call OBJ_txt2nid and twice in call OBJ_txt2obj
as well when ideally all which should be needed one
OBJ_txt2nid("id-on-dnsSRV") and one OBJ_create if not found -- the rest
could be done by comparing integers (nid). Unless I'm missing something
about the oid/nid interaction.

> demonstrating generic support of otherName forms, I have
> added the case of the SRVName otherName form (RFC 4985, for things like
> _carddavs._tcp.example.com, exposed via SSL_SERVER_SAN_OTHER_dnsSRV_n).
> 
> Kaspar

> Index: modules/ssl/ssl_engine_init.c
> ===================================================================
> --- modules/ssl/ssl_engine_init.c	(revision 1687983)
> +++ modules/ssl/ssl_engine_init.c	(working copy)
> @@ -352,6 +352,11 @@ apr_status_t ssl_init_Module(apr_pool_t *p, apr_po
>  
>      init_dh_params();
>  
> +    if (OBJ_txt2nid("id-on-dnsSRV") == NID_undef) {
> +        (void)OBJ_create("1.3.6.1.5.5.7.8.7", "id-on-dnsSRV",
> +                         "SRVName otherName form");
> +    }

Why do the OBJ_create call here and not in
ssl_var_lookup_ssl_cert_san, once it it clear it will be needed at
all?

> Index: modules/ssl/ssl_util_ssl.c
> ===================================================================
> --- modules/ssl/ssl_util_ssl.c	(revision 1687983)
> +++ modules/ssl/ssl_util_ssl.c	(working copy)
> @@ -252,19 +252,46 @@ char *modssl_X509_NAME_to_string(apr_pool_t *p, X5
>      return result;
>  }
>  
> +static void parse_otherName_value(apr_pool_t *p, ASN1_TYPE *value,
> +                                  ASN1_OBJECT *oid,
> +                                  apr_array_header_t **entries)
> +{
> +    const char *str;
> +
> +    if (!value || !oid || !*entries)
> +       return;
> +
> +    /* 
> +     * Currently supported otherName forms:
> +     * - "msUPN" (1.3.6.1.4.1.311.20.2.3): Microsoft User Principal Name
> +     * - "dnsSRV" (1.3.6.1.5.5.7.8.7): SRVName, as specified in RFC 4985
> +     */
> +    if ((OBJ_obj2nid(oid) == NID_ms_upn) &&

Since the OBJ_obj2nid is made always before the comparison, could it be
simpler to just convert to nid in ssl_var_lookup_ssl_cert_san and pass
int, instead passingof ASN1_OBJECT * to modssl_X509_getSAN and
parse_otherName_value?

> +        (value->type == V_ASN1_UTF8STRING) &&
> +        (str = asn1_string_to_utf8(p, value->value.utf8string))) {
> +        APR_ARRAY_PUSH(*entries, const char *) = str;
> +    } else if ((OBJ_obj2nid(oid) == OBJ_txt2nid("id-on-dnsSRV")) &&
> +               (value->type == V_ASN1_IA5STRING) &&
> +               (str = asn1_string_to_utf8(p, value->value.ia5string))) {
> +        APR_ARRAY_PUSH(*entries, const char *) = str;
> +    }
> +}
> +
>  /* 
>   * Return an array of subjectAltName entries of type "type". If idx is -1,
>   * return all entries of the given type, otherwise return an array consisting
>   * of the n-th occurrence of that type only. Currently supported types:
>   * GEN_EMAIL (rfc822Name)
>   * GEN_DNS (dNSName)
> + * GEN_OTHERNAME (see parse_otherName_value for currently supported forms)
>   */
> -BOOL modssl_X509_getSAN(apr_pool_t *p, X509 *x509, int type, int idx,
> -                        apr_array_header_t **entries)
> +BOOL modssl_X509_getSAN(apr_pool_t *p, X509 *x509, int type, ASN1_OBJECT *oid,
> +                        int idx, apr_array_header_t **entries)
>  {
>      STACK_OF(GENERAL_NAME) *names;
>  
> -    if (!x509 || (type < GEN_OTHERNAME) || (type > GEN_RID) || (idx < -1) ||
> +    if (!x509 || (type < GEN_OTHERNAME) || (type == GEN_OTHERNAME && !oid)
||
> +        (type > GEN_RID) || (idx < -1) ||
>          !(*entries = apr_array_make(p, 0, sizeof(char *)))) {
>          *entries = NULL;
>          return FALSE;
> @@ -277,33 +304,43 @@ char *modssl_X509_NAME_to_string(apr_pool_t *p, X5
>  
>          for (i = 0; i < sk_GENERAL_NAME_num(names); i++) {
>              name = sk_GENERAL_NAME_value(names, i);
> -            if (name->type == type) {
> -                if ((idx == -1) || (n == idx)) {
> -                    switch (type) {
> -                    case GEN_EMAIL:
> -                    case GEN_DNS:
> -                        utf8str = asn1_string_to_utf8(p, name->d.ia5);
> -                        if (utf8str) {
> -                            APR_ARRAY_PUSH(*entries, const char *) = utf8str;
> -                        }
> -                        break;
> -                    default:
> -                        /*
> -                         * Not implemented right now:
> -                         * GEN_OTHERNAME (otherName)
> -                         * GEN_X400 (x400Address)
> -                         * GEN_DIRNAME (directoryName)
> -                         * GEN_EDIPARTY (ediPartyName)
> -                         * GEN_URI (uniformResourceIdentifier)
> -                         * GEN_IPADD (iPAddress)
> -                         * GEN_RID (registeredID)
> -                         */
> -                        break;
> +
> +            if (name->type != type)
> +                continue;
> +
> +            switch (type) {
> +            case GEN_EMAIL:
> +            case GEN_DNS:
> +                if (((idx == -1) || (n == idx)) &&
> +                    (utf8str = asn1_string_to_utf8(p, name->d.ia5))) {
> +                    APR_ARRAY_PUSH(*entries, const char *) = utf8str;
> +                }
> +                n++;
> +                break;
> +            case GEN_OTHERNAME:
> +                if (!OBJ_cmp(name->d.otherName->type_id, oid)) {

If we used nid throughout the call stack, we could just do integer
comparison here.

-- 
Jan Pazdziora
Senior Principal Software Engineer, Identity Management Engineering, Red Hat

Mime
View raw message