Re: [openssl-users] Failed to access LDAP server when a valid certificate is at <hash>.1+

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [openssl-users] Failed to access LDAP server when a valid certificate is at <hash>.1+

Misaki Miyashita
(switching the alias to [hidden email])

I would like to suggest the following fix so that a valid certificate at
<hash>.x can be recognized during the cert validation even when <hash>.0
is linking to a bad/expired certificate.  This may not be the most
elegant solution, but it is a minimal change with low impact to the rest
of the code.

Could I possibly get a review on the change? and possibly be considered
to be integrated to the upstream?
(This is for the 1.0.1 branch)

Thanks in advance.

-- misaki


--- a/crypto/x509/x509_vfy.c    2017-11-02 07:32:58.000000000 -0700
+++ b/crypto/x509/x509_vfy.c    2017-12-11 12:37:55.591835780 -0800
@@ -185,6 +185,39 @@
      return xtmp;
  }

+/*
+ * Look through the trust store setup by get_issuer() and
+ * return the certificate which matches the server cert 'x'
+ * via 'xtmp'.
+ */
+static int X509_get_cert(X509 **xtmp, X509_STORE_CTX *ctx, X509 *x)
+{
+    X509_OBJECT    *tmp;
+    int            i;
+    int            ret = 0;
+
+    CRYPTO_w_lock(CRYPTO_LOCK_X509_STORE);
+    for (i = 0; i < sk_X509_OBJECT_num(ctx->ctx->objs); i++) {
+        tmp = sk_X509_OBJECT_value(ctx->ctx->objs, i);
+        if (tmp == NULL) {
+            goto exit;
+        }
+        if (X509_cmp(tmp->data.x509, x) == 0) {
+            /*
+             * Found the cert in the trust store which matches the
+             * server cert.  Increment the ref count and return.
+             */
+            X509_OBJECT_up_ref_count(tmp);
+            *xtmp = tmp->data.x509;
+            ret = 1;
+            goto exit;
+        }
+    }
+exit:
+    CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE);
+    return ret;
+}
+
  int X509_verify_cert(X509_STORE_CTX *ctx)
  {
      X509 *x, *xtmp, *xtmp2, *chain_ss = NULL;
@@ -316,9 +350,13 @@
                   * We have a single self signed certificate: see if we can
                   * find it in the store. We must have an exact match
to avoid
                   * possible impersonation.
+                 * get_issuer() sets up the trust store for the subject and
+                 * returns the first cert via 'xtmp'. The first cert in the
+                 * trust store may not be the certificate that we are
interested
+                 * in. Look through the trust store to see there is an
exact match.
                   */
                  ok = ctx->get_issuer(&xtmp, ctx, x);
-                if ((ok <= 0) || X509_cmp(x, xtmp)) {
+                if ((ok <= 0) || (X509_get_cert(&xtmp, ctx, x) != 1)) {
                      ctx->error = X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT;
                      ctx->current_cert = x;
                      ctx->error_depth = i - 1;


On 10/21/17 03:21 PM, Viktor Dukhovni wrote:

>
> On Oct 21, 2017, at 11:20 AM, Misaki Miyashita <[hidden email]> wrote:
>
>> We encountered a problem using OpenLDAP with OpenSSL when there were more than one certificate with the same subject.
>>
>> Does OpenSSL stop searching for a valid certificate when it finds a certificate with matching DN?
> Yes, when a matching issuer is found in the trust store, but is expired
> no alternative certificates will be tested.  You need to remove outdated
> issuer certificates from your trust store before they expire.
>

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [openssl-users] Failed to access LDAP server when a valid certificate is at <hash>.1+

Misaki Miyashita


On 01/ 8/18 04:46 PM, Misaki Miyashita wrote:

> (switching the alias to [hidden email])
>
> I would like to suggest the following fix so that a valid certificate
> at <hash>.x can be recognized during the cert validation even when
> <hash>.0 is linking to a bad/expired certificate.  This may not be the
> most elegant solution, but it is a minimal change with low impact to
> the rest of the code.
>
> Could I possibly get a review on the change? and possibly be
> considered to be integrated to the upstream?
> (This is for the 1.0.1 branch)

Sorry, I meant to say it is for the 1.0.2 branch.

>
> Thanks in advance.
>
> -- misaki
>
>
> --- a/crypto/x509/x509_vfy.c    2017-11-02 07:32:58.000000000 -0700
> +++ b/crypto/x509/x509_vfy.c    2017-12-11 12:37:55.591835780 -0800
> @@ -185,6 +185,39 @@
>      return xtmp;
>  }
>
> +/*
> + * Look through the trust store setup by get_issuer() and
> + * return the certificate which matches the server cert 'x'
> + * via 'xtmp'.
> + */
> +static int X509_get_cert(X509 **xtmp, X509_STORE_CTX *ctx, X509 *x)
> +{
> +    X509_OBJECT    *tmp;
> +    int            i;
> +    int            ret = 0;
> +
> +    CRYPTO_w_lock(CRYPTO_LOCK_X509_STORE);
> +    for (i = 0; i < sk_X509_OBJECT_num(ctx->ctx->objs); i++) {
> +        tmp = sk_X509_OBJECT_value(ctx->ctx->objs, i);
> +        if (tmp == NULL) {
> +            goto exit;
> +        }
> +        if (X509_cmp(tmp->data.x509, x) == 0) {
> +            /*
> +             * Found the cert in the trust store which matches the
> +             * server cert.  Increment the ref count and return.
> +             */
> +            X509_OBJECT_up_ref_count(tmp);
> +            *xtmp = tmp->data.x509;
> +            ret = 1;
> +            goto exit;
> +        }
> +    }
> +exit:
> +    CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE);
> +    return ret;
> +}
> +
>  int X509_verify_cert(X509_STORE_CTX *ctx)
>  {
>      X509 *x, *xtmp, *xtmp2, *chain_ss = NULL;
> @@ -316,9 +350,13 @@
>                   * We have a single self signed certificate: see if
> we can
>                   * find it in the store. We must have an exact match
> to avoid
>                   * possible impersonation.
> +                 * get_issuer() sets up the trust store for the
> subject and
> +                 * returns the first cert via 'xtmp'. The first cert
> in the
> +                 * trust store may not be the certificate that we are
> interested
> +                 * in. Look through the trust store to see there is
> an exact match.
>                   */
>                  ok = ctx->get_issuer(&xtmp, ctx, x);
> -                if ((ok <= 0) || X509_cmp(x, xtmp)) {
> +                if ((ok <= 0) || (X509_get_cert(&xtmp, ctx, x) != 1)) {
>                      ctx->error = X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT;
>                      ctx->current_cert = x;
>                      ctx->error_depth = i - 1;
>
>
> On 10/21/17 03:21 PM, Viktor Dukhovni wrote:
>>
>> On Oct 21, 2017, at 11:20 AM, Misaki Miyashita
>> <[hidden email]> wrote:
>>
>>> We encountered a problem using OpenLDAP with OpenSSL when there were
>>> more than one certificate with the same subject.
>>>
>>> Does OpenSSL stop searching for a valid certificate when it finds a
>>> certificate with matching DN?
>> Yes, when a matching issuer is found in the trust store, but is expired
>> no alternative certificates will be tested.  You need to remove outdated
>> issuer certificates from your trust store before they expire.
>>
>

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [openssl-users] Failed to access LDAP server when a valid certificate is at <hash>.1+

OpenSSL - Dev mailing list
On 01/09/2018 12:53 AM, Misaki Miyashita wrote:

>
>
> On 01/ 8/18 04:46 PM, Misaki Miyashita wrote:
>> (switching the alias to [hidden email])
>>
>> I would like to suggest the following fix so that a valid certificate
>> at <hash>.x can be recognized during the cert validation even when
>> <hash>.0 is linking to a bad/expired certificate.  This may not be
>> the most elegant solution, but it is a minimal change with low impact
>> to the rest of the code.
>>
>> Could I possibly get a review on the change? and possibly be
>> considered to be integrated to the upstream?
>> (This is for the 1.0.1 branch)
>
> Sorry, I meant to say it is for the 1.0.2 branch.
>

Except in exceptional circumstances, code only ends up in the 1.0.2
branch after having first gotten into the master branch and then the
1.1.0 branch.  The current release policy only allows bug fixes to be
backported to the stable branches, not new features. To me, this code
seems more like a new feature than a bugfix, though I do not claim to
speak authoritatively on the matter.

The preferred mechanism for submitting patches is as github pull
requests (against the master branch, with a note in the pull request
message if the backport is desired).

-Ben
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [openssl-users] Failed to access LDAP server when a valid certificate is at <hash>.1+

Misaki Miyashita

>> Sorry, I meant to say it is for the 1.0.2 branch.
>>
> Except in exceptional circumstances, code only ends up in the 1.0.2
> branch after having first gotten into the master branch and then the
> 1.1.0 branch.  The current release policy only allows bug fixes to be
> backported to the stable branches, not new features. To me, this code
> seems more like a new feature than a bugfix, though I do not claim to
> speak authoritatively on the matter.
>
> The preferred mechanism for submitting patches is as github pull
> requests (against the master branch, with a note in the pull request
> message if the backport is desired).

Thank so much for your comment, Ben.

We are planing to upgrade to the 1.1.0 branch as soon as we can which is
not so easy to do at this moment as we need the FIPS capability.
Thus, we are still focusing on the 1.0.2 release, and haven't had a
chance to work on the 1.1.0 branch.  Thus, I won't be able to submit a
PR against the master branch at this moment.

Thus, I was hoping to get a review on the suggested fix for the 1.0.2 to
see it is viable by the upstream first.

Would it be possible to get a review on the [hidden email]
alias? or filing an issue via github is the right course of action?

Thanks again for your comment.

Regards,

-- misaki
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [openssl-users] Failed to access LDAP server when a valid certificate is at <hash>.1+

OpenSSL - Dev mailing list
On 01/09/2018 01:47 PM, Misaki Miyashita wrote:

>
>>> Sorry, I meant to say it is for the 1.0.2 branch.
>>>
>> Except in exceptional circumstances, code only ends up in the 1.0.2
>> branch after having first gotten into the master branch and then the
>> 1.1.0 branch.  The current release policy only allows bug fixes to be
>> backported to the stable branches, not new features. To me, this code
>> seems more like a new feature than a bugfix, though I do not claim to
>> speak authoritatively on the matter.
>>
>> The preferred mechanism for submitting patches is as github pull
>> requests (against the master branch, with a note in the pull request
>> message if the backport is desired).
>
> Thank so much for your comment, Ben.
>
> We are planing to upgrade to the 1.1.0 branch as soon as we can which
> is not so easy to do at this moment as we need the FIPS capability.
> Thus, we are still focusing on the 1.0.2 release, and haven't had a
> chance to work on the 1.1.0 branch.  Thus, I won't be able to submit a
> PR against the master branch at this moment.
>
> Thus, I was hoping to get a review on the suggested fix for the 1.0.2
> to see it is viable by the upstream first.
>
> Would it be possible to get a review on the [hidden email]
> alias? or filing an issue via github is the right course of action?
>

You already got a review, from Viktor.  I don't think there's much
reason to file an issue in github without a patch (and if there's a
patch, it should just go straight to a pull request with no separate
issue).  If you want the feature to get upstreamed, the onus is on you
to forward-port the patch to master and adapt it to review comments; I
don't think we've seen sufficient interest to cause a team member to
spontaneously take that work upon themselves.

-Ben

> Thanks again for your comment.
>
> Regards,
>
> -- misaki

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [openssl-users] Failed to access LDAP server when a valid certificate is at <hash>.1+

Misaki Miyashita

>> Thank so much for your comment, Ben.
>>
>> We are planing to upgrade to the 1.1.0 branch as soon as we can which
>> is not so easy to do at this moment as we need the FIPS capability.
>> Thus, we are still focusing on the 1.0.2 release, and haven't had a
>> chance to work on the 1.1.0 branch.  Thus, I won't be able to submit a
>> PR against the master branch at this moment.
>>
>> Thus, I was hoping to get a review on the suggested fix for the 1.0.2
>> to see it is viable by the upstream first.
>>
>> Would it be possible to get a review on the [hidden email]
>> alias? or filing an issue via github is the right course of action?
>>
> You already got a review, from Viktor.

I totally missed the review from Viktor.  I'll follow up on the thread.
Sorry about that.

>    I don't think there's much
> reason to file an issue in github without a patch (and if there's a
> patch, it should just go straight to a pull request with no separate
> issue).  If you want the feature to get upstreamed, the onus is on you
> to forward-port the patch to master and adapt it to review comments; I
> don't think we've seen sufficient interest to cause a team member to
> spontaneously take that work upon themselves.

Understood.  Thanks for your input.
Once we move to the 1.1 release train, we will look into the issue and
submit a PR.

Thank you,

-- misaki
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev