From 226567ba5dd9877efa02933d60cf38a913979ea6 Mon Sep 17 00:00:00 2001 From: opentissandy Date: Mon, 7 Oct 2024 16:16:38 +0000 Subject: [PATCH 1/3] X509 client authentication #787 --- common/rfb/CSecurityTLS.cxx | 9 +++++++++ common/rfb/CSecurityTLS.h | 2 ++ vncviewer/OptionsDialog.cxx | 24 +++++++++++++++++++++++- vncviewer/OptionsDialog.h | 2 ++ vncviewer/parameters.cxx | 2 ++ vncviewer/vncviewer.man | 14 ++++++++++++++ 6 files changed, 52 insertions(+), 1 deletion(-) diff --git a/common/rfb/CSecurityTLS.cxx b/common/rfb/CSecurityTLS.cxx index 6eeb6a84d3..96c0ad14ec 100644 --- a/common/rfb/CSecurityTLS.cxx +++ b/common/rfb/CSecurityTLS.cxx @@ -56,6 +56,12 @@ StringParameter CSecurityTLS::X509CA("X509CA", "X509 CA certificate", StringParameter CSecurityTLS::X509CRL("X509CRL", "X509 CRL file", configdirfn("x509_crl.pem"), ConfViewer); +StringParameter CSecurityTLS::X509CERT("X509CERT", "X509 client certificate", + configdirfn("x509_cert.pem"), + ConfViewer); +StringParameter CSecurityTLS::X509KEY("X509KEY", "X509 client private key", + configdirfn("x509_key.pem"), + ConfViewer); static LogWriter vlog("TLS"); @@ -281,6 +287,9 @@ void CSecurityTLS::setParam() if (gnutls_certificate_set_x509_crl_file(cert_cred, X509CRL, GNUTLS_X509_FMT_PEM) < 0) vlog.error("Could not load user specified certificate revocation list"); + if (gnutls_certificate_set_x509_key_file (cert_cred, X509CERT, X509KEY, GNUTLS_X509_FMT_PEM) < 0) + vlog.error("Could not load user specified client certificate"); + ret = gnutls_credentials_set(session, GNUTLS_CRD_CERTIFICATE, cert_cred); if (ret != GNUTLS_E_SUCCESS) throw rdr::TLSException("gnutls_credentials_set()", ret); diff --git a/common/rfb/CSecurityTLS.h b/common/rfb/CSecurityTLS.h index 848ef9bb8c..d2634b6e50 100644 --- a/common/rfb/CSecurityTLS.h +++ b/common/rfb/CSecurityTLS.h @@ -43,6 +43,8 @@ namespace rfb { static StringParameter X509CA; static StringParameter X509CRL; + static StringParameter X509CERT; + static StringParameter X509KEY; protected: void shutdown(); diff --git a/vncviewer/OptionsDialog.cxx b/vncviewer/OptionsDialog.cxx index e04065eced..e4b1e789fb 100644 --- a/vncviewer/OptionsDialog.cxx +++ b/vncviewer/OptionsDialog.cxx @@ -61,7 +61,7 @@ std::map OptionsDialog::callbacks; static std::set instances; OptionsDialog::OptionsDialog() - : Fl_Window(580, 420, _("TigerVNC Options")) + : Fl_Window(580, 450, _("TigerVNC Options")) { int x, y; Fl_Navigation *navigation; @@ -305,6 +305,8 @@ void OptionsDialog::loadOptions(void) #ifdef HAVE_GNUTLS caInput->value(CSecurityTLS::X509CA); crlInput->value(CSecurityTLS::X509CRL); + certInput->value(CSecurityTLS::X509CERT); + keyInput->value(CSecurityTLS::X509KEY); handleX509(encX509Checkbox, this); #endif @@ -436,6 +438,8 @@ void OptionsDialog::storeOptions(void) CSecurityTLS::X509CA.setParam(caInput->value()); CSecurityTLS::X509CRL.setParam(crlInput->value()); + CSecurityTLS::X509CERT.setParam(certInput->value()); + CSecurityTLS::X509KEY.setParam(keyInput->value()); #endif #ifdef HAVE_NETTLE @@ -728,6 +732,20 @@ void OptionsDialog::createSecurityPage(int tx, int ty, int tw, int th) _("Path to X509 CRL file")); crlInput->align(FL_ALIGN_LEFT | FL_ALIGN_TOP); ty += INPUT_HEIGHT + TIGHT_MARGIN; + + ty += INPUT_LABEL_OFFSET; + certInput = new Fl_Input(tx + INDENT, ty, + width - INDENT * 2, INPUT_HEIGHT, + _("Path to X509 client certificate")); + certInput->align(FL_ALIGN_LEFT | FL_ALIGN_TOP); + ty += INPUT_HEIGHT + TIGHT_MARGIN; + + ty += INPUT_LABEL_OFFSET; + keyInput = new Fl_Input(tx + INDENT, ty, + width - INDENT * 2, INPUT_HEIGHT, + _("Path to X509 client private key")); + keyInput->align(FL_ALIGN_LEFT | FL_ALIGN_TOP); + ty += INPUT_HEIGHT + TIGHT_MARGIN; #endif #ifdef HAVE_NETTLE encRSAAESCheckbox = new Fl_Check_Button(LBLRIGHT(tx, ty, @@ -1094,9 +1112,13 @@ void OptionsDialog::handleX509(Fl_Widget* /*widget*/, void *data) if (dialog->encX509Checkbox->value()) { dialog->caInput->activate(); dialog->crlInput->activate(); + dialog->certInput->activate(); + dialog->keyInput->activate(); } else { dialog->caInput->deactivate(); dialog->crlInput->deactivate(); + dialog->certInput->deactivate(); + dialog->keyInput->deactivate(); } } diff --git a/vncviewer/OptionsDialog.h b/vncviewer/OptionsDialog.h index f6ca89b1ba..b19e055988 100644 --- a/vncviewer/OptionsDialog.h +++ b/vncviewer/OptionsDialog.h @@ -105,6 +105,8 @@ class OptionsDialog : public Fl_Window { Fl_Check_Button *encRSAAESCheckbox; Fl_Input *caInput; Fl_Input *crlInput; + Fl_Input *certInput; + Fl_Input *keyInput; Fl_Group *authenticationGroup; Fl_Check_Button *authNoneCheckbox; diff --git a/vncviewer/parameters.cxx b/vncviewer/parameters.cxx index 4bbf7a7f20..d76c47d06c 100644 --- a/vncviewer/parameters.cxx +++ b/vncviewer/parameters.cxx @@ -178,6 +178,8 @@ static VoidParameter* parameterArray[] = { #ifdef HAVE_GNUTLS &CSecurityTLS::X509CA, &CSecurityTLS::X509CRL, + &CSecurityTLS::X509CERT, + &CSecurityTLS::X509KEY, #endif // HAVE_GNUTLS &SecurityClient::secTypes, /* Misc. */ diff --git a/vncviewer/vncviewer.man b/vncviewer/vncviewer.man index 79c410ae3a..80508d56f6 100644 --- a/vncviewer/vncviewer.man +++ b/vncviewer/vncviewer.man @@ -165,6 +165,20 @@ Path to certificate revocation list to use in conjunction with \fI~/.config/tigervnc/x509_crl.pem\fP. . .TP +.B \-X509CERT \fIpath\fP +Path to client certificate to use when authenticating client using any +of the X509 security schemes (X509None, X509Vnc, etc.). Must be in PEM +format. Default is \fI$XDG_CONFIG_HOME/tigervnc/x509_cert.pem\fP, or +\fI~/.config/tigervnc/x509_cert.pem\fP. +. +.TP +.B \-X509KEY \fIpath\fP +Path to client private key to use in conjunction with +\fB-X509CERT\fP. Must also be in PEM format. Default is +\fI$XDG_CONFIG_HOME/tigervnc/x509_key.pem\fP, or +\fI~/.config/tigervnc/x509_key.pem\fP. +. +.TP .B \-Shared When you make a connection to a VNC server, all other existing connections are normally closed. This option requests that they be left open, allowing you to From 33aca76cfe9cef2119e81b37168f98a772e80b82 Mon Sep 17 00:00:00 2001 From: opentissandy Date: Tue, 8 Oct 2024 12:59:07 +0000 Subject: [PATCH 2/3] Resolve code by CendioOssman reviewed. --- common/rfb/CSecurityTLS.cxx | 10 +++++----- common/rfb/CSecurityTLS.h | 4 ++-- vncviewer/OptionsDialog.cxx | 8 ++++---- vncviewer/parameters.cxx | 4 ++-- vncviewer/vncviewer.man | 12 ++++++------ 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/common/rfb/CSecurityTLS.cxx b/common/rfb/CSecurityTLS.cxx index 96c0ad14ec..0acefda9dc 100644 --- a/common/rfb/CSecurityTLS.cxx +++ b/common/rfb/CSecurityTLS.cxx @@ -56,11 +56,11 @@ StringParameter CSecurityTLS::X509CA("X509CA", "X509 CA certificate", StringParameter CSecurityTLS::X509CRL("X509CRL", "X509 CRL file", configdirfn("x509_crl.pem"), ConfViewer); -StringParameter CSecurityTLS::X509CERT("X509CERT", "X509 client certificate", - configdirfn("x509_cert.pem"), +StringParameter CSecurityTLS::X509Cert("X509Cert", "X509 client certificate", + configdirfn("x509_client_cert.pem"), ConfViewer); -StringParameter CSecurityTLS::X509KEY("X509KEY", "X509 client private key", - configdirfn("x509_key.pem"), +StringParameter CSecurityTLS::X509Key("X509Key", "X509 client private key", + configdirfn("x509_client_key.pem"), ConfViewer); static LogWriter vlog("TLS"); @@ -287,7 +287,7 @@ void CSecurityTLS::setParam() if (gnutls_certificate_set_x509_crl_file(cert_cred, X509CRL, GNUTLS_X509_FMT_PEM) < 0) vlog.error("Could not load user specified certificate revocation list"); - if (gnutls_certificate_set_x509_key_file (cert_cred, X509CERT, X509KEY, GNUTLS_X509_FMT_PEM) < 0) + if (gnutls_certificate_set_x509_key_file (cert_cred, X509Cert, X509Key, GNUTLS_X509_FMT_PEM) < 0) vlog.error("Could not load user specified client certificate"); ret = gnutls_credentials_set(session, GNUTLS_CRD_CERTIFICATE, cert_cred); diff --git a/common/rfb/CSecurityTLS.h b/common/rfb/CSecurityTLS.h index d2634b6e50..f47a6d438a 100644 --- a/common/rfb/CSecurityTLS.h +++ b/common/rfb/CSecurityTLS.h @@ -43,8 +43,8 @@ namespace rfb { static StringParameter X509CA; static StringParameter X509CRL; - static StringParameter X509CERT; - static StringParameter X509KEY; + static StringParameter X509Cert; + static StringParameter X509Key; protected: void shutdown(); diff --git a/vncviewer/OptionsDialog.cxx b/vncviewer/OptionsDialog.cxx index e4b1e789fb..2708bca6dc 100644 --- a/vncviewer/OptionsDialog.cxx +++ b/vncviewer/OptionsDialog.cxx @@ -305,8 +305,8 @@ void OptionsDialog::loadOptions(void) #ifdef HAVE_GNUTLS caInput->value(CSecurityTLS::X509CA); crlInput->value(CSecurityTLS::X509CRL); - certInput->value(CSecurityTLS::X509CERT); - keyInput->value(CSecurityTLS::X509KEY); + certInput->value(CSecurityTLS::X509Cert); + keyInput->value(CSecurityTLS::X509Key); handleX509(encX509Checkbox, this); #endif @@ -438,8 +438,8 @@ void OptionsDialog::storeOptions(void) CSecurityTLS::X509CA.setParam(caInput->value()); CSecurityTLS::X509CRL.setParam(crlInput->value()); - CSecurityTLS::X509CERT.setParam(certInput->value()); - CSecurityTLS::X509KEY.setParam(keyInput->value()); + CSecurityTLS::X509Cert.setParam(certInput->value()); + CSecurityTLS::X509Key.setParam(keyInput->value()); #endif #ifdef HAVE_NETTLE diff --git a/vncviewer/parameters.cxx b/vncviewer/parameters.cxx index d76c47d06c..085286b8d2 100644 --- a/vncviewer/parameters.cxx +++ b/vncviewer/parameters.cxx @@ -178,8 +178,8 @@ static VoidParameter* parameterArray[] = { #ifdef HAVE_GNUTLS &CSecurityTLS::X509CA, &CSecurityTLS::X509CRL, - &CSecurityTLS::X509CERT, - &CSecurityTLS::X509KEY, + &CSecurityTLS::X509Cert, + &CSecurityTLS::X509Key, #endif // HAVE_GNUTLS &SecurityClient::secTypes, /* Misc. */ diff --git a/vncviewer/vncviewer.man b/vncviewer/vncviewer.man index 80508d56f6..50b156ce37 100644 --- a/vncviewer/vncviewer.man +++ b/vncviewer/vncviewer.man @@ -165,18 +165,18 @@ Path to certificate revocation list to use in conjunction with \fI~/.config/tigervnc/x509_crl.pem\fP. . .TP -.B \-X509CERT \fIpath\fP +.B \-X509Cert \fIpath\fP Path to client certificate to use when authenticating client using any of the X509 security schemes (X509None, X509Vnc, etc.). Must be in PEM -format. Default is \fI$XDG_CONFIG_HOME/tigervnc/x509_cert.pem\fP, or -\fI~/.config/tigervnc/x509_cert.pem\fP. +format. Default is \fI$XDG_CONFIG_HOME/tigervnc/x509_client_cert.pem\fP, or +\fI~/.config/tigervnc/x509_client_cert.pem\fP. . .TP -.B \-X509KEY \fIpath\fP +.B \-X509Key \fIpath\fP Path to client private key to use in conjunction with \fB-X509CERT\fP. Must also be in PEM format. Default is -\fI$XDG_CONFIG_HOME/tigervnc/x509_key.pem\fP, or -\fI~/.config/tigervnc/x509_key.pem\fP. +\fI$XDG_CONFIG_HOME/tigervnc/x509_client_key.pem\fP, or +\fI~/.config/tigervnc/x509_client_key.pem\fP. . .TP .B \-Shared From e1b3151764b0be46e86344a3411764388d9f3f4b Mon Sep 17 00:00:00 2001 From: opentissandy Date: Tue, 8 Oct 2024 13:06:30 +0000 Subject: [PATCH 3/3] Fixed X509Cert in man. --- vncviewer/vncviewer.man | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vncviewer/vncviewer.man b/vncviewer/vncviewer.man index 50b156ce37..538d175e29 100644 --- a/vncviewer/vncviewer.man +++ b/vncviewer/vncviewer.man @@ -174,7 +174,7 @@ format. Default is \fI$XDG_CONFIG_HOME/tigervnc/x509_client_cert.pem\fP, or .TP .B \-X509Key \fIpath\fP Path to client private key to use in conjunction with -\fB-X509CERT\fP. Must also be in PEM format. Default is +\fB-X509Cert\fP. Must also be in PEM format. Default is \fI$XDG_CONFIG_HOME/tigervnc/x509_client_key.pem\fP, or \fI~/.config/tigervnc/x509_client_key.pem\fP. .