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.

Sign up for GitHub

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

X509 client authentication #787 #1842

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

X509 client authentication #787 #1842

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
226567b
X509 client authentication #787
opentissandy Oct 7, 2024
33aca76
Resolve code by CendioOssman reviewed.
opentissandy Oct 8, 2024
e1b3151
Fixed X509Cert in man.
opentissandy Oct 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions common/rfb/CSecurityTLS.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
opentissandy marked this conversation as resolved.
Show resolved Hide resolved
configdirfn("x509_cert.pem"),
opentissandy marked this conversation as resolved.
Show resolved Hide resolved
ConfViewer);
StringParameter CSecurityTLS::X509KEY("X509KEY", "X509 client private key",
configdirfn("x509_key.pem"),
ConfViewer);

static LogWriter vlog("TLS");

Expand Down Expand Up @@ -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");
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we need to have a bit more explicit error handling. What is the user experience if there is some issue with the certificate or key file?


ret = gnutls_credentials_set(session, GNUTLS_CRD_CERTIFICATE, cert_cred);
if (ret != GNUTLS_E_SUCCESS)
throw rdr::TLSException("gnutls_credentials_set()", ret);
Expand Down
2 changes: 2 additions & 0 deletions common/rfb/CSecurityTLS.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ namespace rfb {

static StringParameter X509CA;
static StringParameter X509CRL;
static StringParameter X509CERT;
static StringParameter X509KEY;

protected:
void shutdown();
Expand Down
24 changes: 23 additions & 1 deletion vncviewer/OptionsDialog.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ std::map<OptionsCallback*, void*> OptionsDialog::callbacks;
static std::set<OptionsDialog *> instances;

OptionsDialog::OptionsDialog()
: Fl_Window(580, 420, _("TigerVNC Options"))
: Fl_Window(580, 450, _("TigerVNC Options"))
{
int x, y;
Fl_Navigation *navigation;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Comment on lines +735 to +748
Copy link
Member

Choose a reason for hiding this comment

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

These are authentication settings, not encryption. So I think they fit better in the other section.

#endif
#ifdef HAVE_NETTLE
encRSAAESCheckbox = new Fl_Check_Button(LBLRIGHT(tx, ty,
Expand Down Expand Up @@ -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();
}
}

Expand Down
2 changes: 2 additions & 0 deletions vncviewer/OptionsDialog.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions vncviewer/parameters.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,8 @@ static VoidParameter* parameterArray[] = {
#ifdef HAVE_GNUTLS
&CSecurityTLS::X509CA,
&CSecurityTLS::X509CRL,
&CSecurityTLS::X509CERT,
&CSecurityTLS::X509KEY,
#endif // HAVE_GNUTLS
&SecurityClient::secTypes,
/* Misc. */
Expand Down
14 changes: 14 additions & 0 deletions vncviewer/vncviewer.man
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
opentissandy marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
Loading