| Message ID | 20210616162225.2517463-6-philmd@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | crypto: Make QCryptoTLSCreds* structures private | expand |
On 6/16/21 9:22 AM, Philippe Mathieu-Daudé wrote: > Introduce the qcrypto_tls_creds_check_endpoint() helper > to avoid accessing QCryptoTLSCreds internal 'endpoint' field > directly. I don't understand this one. Comment ... > +bool qcrypto_tls_session_check_role(QCryptoTLSCreds *creds, > + QCryptoTLSCredsEndpoint endpoint, > + Error **errp) > +{ > + return qcrypto_tls_creds_check_endpoint(creds, endpoint, errp); > +} ... doesn't match the function. The new function is a pure forwarder, and begs the question of why the caller isn't using qcrypto_tls_creds_check_endpoint directly. r~
On 6/16/21 9:12 PM, Richard Henderson wrote: > On 6/16/21 9:22 AM, Philippe Mathieu-Daudé wrote: >> Introduce the qcrypto_tls_creds_check_endpoint() helper >> to avoid accessing QCryptoTLSCreds internal 'endpoint' field >> directly. > > I don't understand this one. Comment ... > >> +bool qcrypto_tls_session_check_role(QCryptoTLSCreds *creds, >> + QCryptoTLSCredsEndpoint endpoint, >> + Error **errp) >> +{ >> + return qcrypto_tls_creds_check_endpoint(creds, endpoint, errp); >> +} > > ... doesn't match the function. > > The new function is a pure forwarder, and begs the question of why the > caller isn't using qcrypto_tls_creds_check_endpoint directly. I tried to follow the maintainer/subsystem style (I was also tempted to use qcrypto_tls_creds_check_endpoint directly). ui/vnc uses the TLS "session" API and not the "creds" one. Daniel, what is your preference?
On Wed, Jun 16, 2021 at 09:21:45PM +0200, Philippe Mathieu-Daudé wrote: > On 6/16/21 9:12 PM, Richard Henderson wrote: > > On 6/16/21 9:22 AM, Philippe Mathieu-Daudé wrote: > >> Introduce the qcrypto_tls_creds_check_endpoint() helper > >> to avoid accessing QCryptoTLSCreds internal 'endpoint' field > >> directly. > > > > I don't understand this one. Comment ... > > > >> +bool qcrypto_tls_session_check_role(QCryptoTLSCreds *creds, > >> + QCryptoTLSCredsEndpoint endpoint, > >> + Error **errp) > >> +{ > >> + return qcrypto_tls_creds_check_endpoint(creds, endpoint, errp); > >> +} > > > > ... doesn't match the function. > > > > The new function is a pure forwarder, and begs the question of why the > > caller isn't using qcrypto_tls_creds_check_endpoint directly. > > I tried to follow the maintainer/subsystem style (I was also tempted to > use qcrypto_tls_creds_check_endpoint directly). ui/vnc uses the TLS > "session" API and not the "creds" one. Daniel, what is your preference? I think we don't need this extra function - just use the function from earlier directly. Regards, Daniel
On 6/17/21 11:33 AM, Daniel P. Berrangé wrote: > On Wed, Jun 16, 2021 at 09:21:45PM +0200, Philippe Mathieu-Daudé wrote: >> On 6/16/21 9:12 PM, Richard Henderson wrote: >>> On 6/16/21 9:22 AM, Philippe Mathieu-Daudé wrote: >>>> Introduce the qcrypto_tls_creds_check_endpoint() helper >>>> to avoid accessing QCryptoTLSCreds internal 'endpoint' field >>>> directly. >>> >>> I don't understand this one. Comment ... >>> >>>> +bool qcrypto_tls_session_check_role(QCryptoTLSCreds *creds, >>>> + QCryptoTLSCredsEndpoint endpoint, >>>> + Error **errp) >>>> +{ >>>> + return qcrypto_tls_creds_check_endpoint(creds, endpoint, errp); >>>> +} >>> >>> ... doesn't match the function. >>> >>> The new function is a pure forwarder, and begs the question of why the >>> caller isn't using qcrypto_tls_creds_check_endpoint directly. >> >> I tried to follow the maintainer/subsystem style (I was also tempted to >> use qcrypto_tls_creds_check_endpoint directly). ui/vnc uses the TLS >> "session" API and not the "creds" one. Daniel, what is your preference? > > I think we don't need this extra function - just use the function from > earlier directly. Great, simpler :)
diff --git a/include/crypto/tlssession.h b/include/crypto/tlssession.h index 15b9cef086c..2fb0bb02d9f 100644 --- a/include/crypto/tlssession.h +++ b/include/crypto/tlssession.h @@ -162,6 +162,21 @@ void qcrypto_tls_session_free(QCryptoTLSSession *sess); G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoTLSSession, qcrypto_tls_session_free) +/** + * qcrypto_tls_session_check_role: + * @creds: pointer to a TLS credentials object + * @endpoint: role of the TLS session, client or server + * @errp: pointer to a NULL-initialized error object + * + * Check whether the session object operates according to + * the role of the @endpoint argument. + * + * Returns true if the session is setup for the endpoint role, false otherwise + */ +bool qcrypto_tls_session_check_role(QCryptoTLSCreds *creds, + QCryptoTLSCredsEndpoint endpoint, + Error **errp); + /** * qcrypto_tls_session_check_credentials: * @sess: the TLS session object diff --git a/crypto/tlssession.c b/crypto/tlssession.c index 33203e8ca71..4e614b73a28 100644 --- a/crypto/tlssession.c +++ b/crypto/tlssession.c @@ -640,3 +640,10 @@ qcrypto_tls_session_get_peer_name(QCryptoTLSSession *sess) } #endif + +bool qcrypto_tls_session_check_role(QCryptoTLSCreds *creds, + QCryptoTLSCredsEndpoint endpoint, + Error **errp) +{ + return qcrypto_tls_creds_check_endpoint(creds, endpoint, errp); +}
Introduce the qcrypto_tls_creds_check_endpoint() helper to avoid accessing QCryptoTLSCreds internal 'endpoint' field directly. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/crypto/tlssession.h | 15 +++++++++++++++ crypto/tlssession.c | 7 +++++++ 2 files changed, 22 insertions(+)