Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Qt Nightly Jenkins failure #8428

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

miyazakh
Copy link
Contributor

@miyazakh miyazakh commented Feb 8, 2025

Description

Fix Qt Jenkins failure. The failure starts after PR#8345 merged.

Testing

Run Qt Jenkins test

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@miyazakh miyazakh self-assigned this Feb 8, 2025
@miyazakh
Copy link
Contributor Author

Retest this please

1 similar comment
@miyazakh
Copy link
Contributor Author

Retest this please

@miyazakh miyazakh assigned wolfSSL-Bot and unassigned miyazakh Feb 11, 2025
Comment on lines 14460 to 14469
}
#if defined(WOLFSSL_QT)
/* Qt handles a peer cert pushing to chain. */
else if (ssl->options.side == WOLFSSL_SERVER_END) {
/* to be compliant with openssl
first element is kept as peer cert on server side.*/
wolfSSL_sk_X509_pop(sk);
}
#endif
return sk;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is QT behaving differently than other OSP's? We are moving away from OSP specific changes in the compatibility layer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure the reason why Qt 5 does so. But, the following code in Qt5 handles it.

if (!configuration.peerCertificate.isNull() && mode == QSslSocket::SslServerMode)
            configuration.peerCertificateChain.prepend(configuration.peerCertificate);

There are two same cert in the chain if reverted code doesn't exist.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird but it checks out.

    // tls_process_client_certificate
    X509_free(s->session->peer);
    s->session->peer = sk_X509_shift(sk);
    s->session->verify_result = s->verify_result;

    sk_X509_pop_free(s->session->peer_chain, X509_free);
    s->session->peer_chain = sk;
    sk = NULL;

vs

    // tls_process_server_certificate
    s->session->peer_chain = sk;

Please write some tests asserting this behaviour.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And remove the WOLFSSL_QT guard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants