[Swan-dev] Pluto crash with expired certificates
Wolfgang Nothdurft
wolfgang at linogate.de
Fri Feb 6 10:02:07 EET 2015
Am 06.02.2015 um 02:55 schrieb Paul Wouters:
> On Thu, 5 Feb 2015, Wolfgang Nothdurft wrote:
>
>> With commit aac20299b27be6c401cb5d45262a559994e52431 a bug was
>> introduced that causes pluto to crash if an end user certificate is
>> expired.
>
>> The attached patch added the missing return false statement to fix
>> this problem.
>
> I don't understand the patch:
>
> diff --git a/programs/pluto/connections.c b/programs/pluto/connections.c
> index 82f6bac..32defad 100644
> --- a/programs/pluto/connections.c
> +++ b/programs/pluto/connections.c
> @@ -823,7 +823,7 @@ static bool load_end_certificate(const char *name,
> struct end *dst)
> if (dst->ca.ptr == NULL)
> dst->ca = dst->cert.u.x509->issuer;
> }
> - break;
> + return FALSE;
> default:
> bad_case(cert.ty);
> }
>
> If I look at the code:
>
> valid_until = cert.u.x509->notAfter;
> ugh = check_validity(cert.u.x509, &valid_until /* IN/OUT
> */);
> if (ugh != NULL) {
> loglog(RC_LOG_SERIOUS," %s", ugh);
> free_x509cert(cert.u.x509);
> } else {
> DBG(DBG_CONTROL,
> DBG_log("certificate is valid"));
> add_x509_public_key(&dst->id, cert.u.x509,
> valid_until,
> DAL_LOCAL);
> dst->cert.ty = cert.ty;
> dst->cert.u.x509 = add_x509cert(cert.u.x509);
>
> /* if no CA is defined, use issuer as default */
> if (dst->ca.ptr == NULL)
> dst->ca = dst->cert.u.x509->issuer;
> }
> break;
> default:
> bad_case(cert.ty);
> }
>
> return TRUE;
> }
>
> if ugh is non-null we have a serious error. So I can see that the return
> FALSE goes in there and we currently return TRUE. But with your patch,
> regardless of whether ugh returns NULL or not, you return FALSE. The
> break seems fine for the "good case" because it will leave the switch,
> skipping over the default bad_case() and return TRUE? So I think the
> patch should be:
>
> diff --git a/programs/pluto/connections.c b/programs/pluto/connections.c
> index 1446849..72600b9 100644
> --- a/programs/pluto/connections.c
> +++ b/programs/pluto/connections.c
> @@ -811,6 +811,7 @@ static bool load_end_certificate(const char *name,
> struct end *dst)
> if (ugh != NULL) {
> loglog(RC_LOG_SERIOUS," %s", ugh);
> free_x509cert(cert.u.x509);
> + return FALSE;
> } else {
> DBG(DBG_CONTROL,
> DBG_log("certificate is valid"));
>
> Paul
Your right, sorry.
That was a hasty reaction right before leaving the office :(
Wolfgang
More information about the Swan-dev
mailing list