[9front] [PATCH] libsec, pushtls, tlssrv: add support for Server Name Indication (SNI) extension

[9front] [PATCH] libsec, pushtls, tlssrv: add support for Server Name Indication (SNI) extension

[igor]

[-- Attachment #1: Type: text/plain, Size: 4923 bytes --]

This patch extends libsec to support the Server Name Indication (SNI)
extension.

Server Name Indication (SNI) is an extension to the Transport Layer
Security (TLS) computer networking protocol by which a client
indicates which hostname it is attempting to connect to at the start
of the handshaking process.  The extension allows a server to present
one of multiple possible certificates on the same IP address and TCP
port number and hence allows multiple secure (HTTPS) websites (or any
other service over TLS) to be served by the same IP address without
requiring all those sites to use the same certificate.

A real world example is outlined here:
https://9lab.org/plan9/tlssrv8-with-server-name-indication-sni-support/

Please let me know what you think and whether you would like additional
input on why this extension is useful after having glanced at the
example above.

One open item that is not addressed by the attached patch is where
and how to document this feature. The best place seems to be the
pushtls(2) man page with additional references from tlssrv(8) and
friends. I plan to address this once the core implementation of SNI
gets past the approval stage.

Cheers,
Igor

---
diff c2a290b8fe17f06370bc748552f2af1d58f50a71 eff99a21434474625fbdd60e68fb1b4cab389238
--- a/sys/src/libsec/port/tlshand.c
+++ b/sys/src/libsec/port/tlshand.c
@@ -98,6 +98,8 @@
 	char *digest;	// name of digest algorithm to use
 	char *enc;	// name of encryption algorithm to use
 
+	char *serverName;	// server name indication; extension
+
 	// for finished messages
 	HandshakeHash	handhash;
 	Finished	finished;
@@ -355,7 +357,7 @@
 };
 
 static TlsConnection *tlsServer2(int ctl, int hand,
-	uchar *cert, int certlen,
+	uchar **cert, int certlen,
 	char *pskid, uchar *psk, int psklen,
 	int (*trace)(char*fmt, ...), PEMChain *chain);
 static TlsConnection *tlsClient2(int ctl, int hand,
@@ -456,7 +458,7 @@
 	data = -1;
 	fprint(ctl, "fd %d 0x%x", fd, ProtocolVersion);
 	tls = tlsServer2(ctl, hand,
-		conn->cert, conn->certlen,
+		&(conn->cert), conn->certlen,
 		conn->pskID, conn->psk, conn->psklen,
 		conn->trace, conn->chain);
 	if(tls != nil){
@@ -659,9 +661,23 @@
 		if(e-p < 4)
 			goto Short;
 		p += 4;
-		if(e-p < (n = get16(p-2)))
+		if(e-p < (n = get16(p-2)))	/* Length */
 			goto Short;
-		switch(get16(p-4)){
+		switch(get16(p-4)){			/* Type */
+		case Extsni:
+			if(n < 4 || get16(p) != (n -= 2))
+				goto Short;
+			if(*(p+2) != 0)			/* Server Name Type: host_name */
+				break;
+			p += 2+1+2;
+			if(e-p < (n = get16(p-2)))
+				goto Short;
+			if(n > 255)				/* DNS name can not exceed 255 bytes RFC1035 */
+				break;
+			c->serverName = emalloc(n+1);
+			memmove(c->serverName, p, n);
+			c->serverName[n] = 0;
+			break;
 		case Extec:
 			if(n < 4 || n % 2 || get16(p) != (n -= 2))
 				goto Short;
@@ -717,7 +733,7 @@
 
 static TlsConnection *
 tlsServer2(int ctl, int hand,
-	uchar *cert, int certlen,
+	uchar **cert, int certlen,
 	char *pskid, uchar *psk, int psklen,
 	int (*trace)(char*fmt, ...), PEMChain *chp)
 {
@@ -765,9 +781,26 @@
 		c->sec->psk = psk;
 		c->sec->psklen = psklen;
 	}
+	if(checkClientExtensions(c, m.u.clientHello.extensions) < 0)
+		goto Err;
 	if(certlen > 0){
+		/* override default certificate using Server Name Indication (SNI) extension */
+		if(c->serverName){
+			char path[512];
+			PEMChain *chain;
+
+			snprint(path, sizeof(path), "/sys/lib/tls/acmed/%s.crt", c->serverName);
+			if(trace)
+				trace("ClientHello extension server name identifier selects %s\n", path);
+			chain = readcertchain(path);
+			if (chain){
+				free(*cert);
+				*cert = chain->pem;
+				certlen = chain->pemlen;
+			}
+		}
 		/* server certificate */
-		c->sec->rsapub = X509toRSApub(cert, certlen, nil, 0);
+		c->sec->rsapub = X509toRSApub(*cert, certlen, nil, 0);
 		if(c->sec->rsapub == nil){
 			tlsError(c, EHandshakeFailure, "invalid X509/rsa certificate");
 			goto Err;
@@ -780,8 +813,6 @@
 	}
 	if(lookupid(m.u.clientHello.ciphers, TLS_EMPTY_RENEGOTIATION_INFO_SCSV) >= 0)
 		c->sec->reneg = 1;
-	if(checkClientExtensions(c, m.u.clientHello.extensions) < 0)
-		goto Err;
 	cipher = okCipher(m.u.clientHello.ciphers, psklen > 0, c->sec->nc != nil);
 	if(cipher < 0 || !setAlgs(c, cipher)) {
 		tlsError(c, EHandshakeFailure, "no matching cipher suite");
@@ -813,7 +844,7 @@
 		numcerts = countchain(chp);
 		m.u.certificate.ncert = 1 + numcerts;
 		m.u.certificate.certs = emalloc(m.u.certificate.ncert * sizeof(Bytes*));
-		m.u.certificate.certs[0] = makebytes(cert, certlen);
+		m.u.certificate.certs[0] = makebytes(*cert, certlen);
 		for (i = 0; i < numcerts && chp; i++, chp = chp->next)
 			m.u.certificate.certs[i+1] = makebytes(chp->pem, chp->pemlen);
 		if(!msgSend(c, &m, AQueue))
@@ -2113,6 +2144,8 @@
 	factotum_rsa_close(c->sec->rpc);
 	rsapubfree(c->sec->rsapub);
 	freebytes(c->cert);
+
+	free(c->serverName);
 
 	memset(c, 0, sizeof(*c));
 	free(c);

[-- Attachment #2: tlssrv.sni.patch --]
[-- Type: text/plain, Size: 4544 bytes --]

From: Igor Böhm <igor@9lab.org>
Date: Thu, 25 Jan 2024 11:20:51 +0000
Subject: [PATCH] libsec, pushtls, tlssrv: add support for Server Name Indication (SNI) extension


Server Name Indication (SNI) is an extension to the Transport Layer
Security (TLS) computer networking protocol by which a client
indicates which hostname it is attempting to connect to at the start
of the handshaking process.  The extension allows a server to present
one of multiple possible certificates on the same IP address and TCP
port number and hence allows multiple secure (HTTPS) websites (or any
other service over TLS) to be served by the same IP address without
requiring all those sites to use the same certificate.

A real world example is outlined here:
https://9lab.org/plan9/tlssrv8-with-server-name-indication-sni-support/
---
diff c2a290b8fe17f06370bc748552f2af1d58f50a71 eff99a21434474625fbdd60e68fb1b4cab389238
--- a/sys/src/libsec/port/tlshand.c
+++ b/sys/src/libsec/port/tlshand.c
@@ -98,6 +98,8 @@
 	char *digest;	// name of digest algorithm to use
 	char *enc;	// name of encryption algorithm to use
 
+	char *serverName;	// server name indication; extension
+
 	// for finished messages
 	HandshakeHash	handhash;
 	Finished	finished;
@@ -355,7 +357,7 @@
 };
 
 static TlsConnection *tlsServer2(int ctl, int hand,
-	uchar *cert, int certlen,
+	uchar **cert, int certlen,
 	char *pskid, uchar *psk, int psklen,
 	int (*trace)(char*fmt, ...), PEMChain *chain);
 static TlsConnection *tlsClient2(int ctl, int hand,
@@ -456,7 +458,7 @@
 	data = -1;
 	fprint(ctl, "fd %d 0x%x", fd, ProtocolVersion);
 	tls = tlsServer2(ctl, hand,
-		conn->cert, conn->certlen,
+		&(conn->cert), conn->certlen,
 		conn->pskID, conn->psk, conn->psklen,
 		conn->trace, conn->chain);
 	if(tls != nil){
@@ -659,9 +661,23 @@
 		if(e-p < 4)
 			goto Short;
 		p += 4;
-		if(e-p < (n = get16(p-2)))
+		if(e-p < (n = get16(p-2)))	/* Length */
 			goto Short;
-		switch(get16(p-4)){
+		switch(get16(p-4)){			/* Type */
+		case Extsni:
+			if(n < 4 || get16(p) != (n -= 2))
+				goto Short;
+			if(*(p+2) != 0)			/* Server Name Type: host_name */
+				break;
+			p += 2+1+2;
+			if(e-p < (n = get16(p-2)))
+				goto Short;
+			if(n > 255)				/* DNS name can not exceed 255 bytes RFC1035 */
+				break;
+			c->serverName = emalloc(n+1);
+			memmove(c->serverName, p, n);
+			c->serverName[n] = 0;
+			break;
 		case Extec:
 			if(n < 4 || n % 2 || get16(p) != (n -= 2))
 				goto Short;
@@ -717,7 +733,7 @@
 
 static TlsConnection *
 tlsServer2(int ctl, int hand,
-	uchar *cert, int certlen,
+	uchar **cert, int certlen,
 	char *pskid, uchar *psk, int psklen,
 	int (*trace)(char*fmt, ...), PEMChain *chp)
 {
@@ -765,9 +781,26 @@
 		c->sec->psk = psk;
 		c->sec->psklen = psklen;
 	}
+	if(checkClientExtensions(c, m.u.clientHello.extensions) < 0)
+		goto Err;
 	if(certlen > 0){
+		/* override default certificate using Server Name Indication (SNI) extension */
+		if(c->serverName){
+			char path[512];
+			PEMChain *chain;
+
+			snprint(path, sizeof(path), "/sys/lib/tls/acmed/%s.crt", c->serverName);
+			if(trace)
+				trace("ClientHello extension server name identifier selects %s\n", path);
+			chain = readcertchain(path);
+			if (chain){
+				free(*cert);
+				*cert = chain->pem;
+				certlen = chain->pemlen;
+			}
+		}
 		/* server certificate */
-		c->sec->rsapub = X509toRSApub(cert, certlen, nil, 0);
+		c->sec->rsapub = X509toRSApub(*cert, certlen, nil, 0);
 		if(c->sec->rsapub == nil){
 			tlsError(c, EHandshakeFailure, "invalid X509/rsa certificate");
 			goto Err;
@@ -780,8 +813,6 @@
 	}
 	if(lookupid(m.u.clientHello.ciphers, TLS_EMPTY_RENEGOTIATION_INFO_SCSV) >= 0)
 		c->sec->reneg = 1;
-	if(checkClientExtensions(c, m.u.clientHello.extensions) < 0)
-		goto Err;
 	cipher = okCipher(m.u.clientHello.ciphers, psklen > 0, c->sec->nc != nil);
 	if(cipher < 0 || !setAlgs(c, cipher)) {
 		tlsError(c, EHandshakeFailure, "no matching cipher suite");
@@ -813,7 +844,7 @@
 		numcerts = countchain(chp);
 		m.u.certificate.ncert = 1 + numcerts;
 		m.u.certificate.certs = emalloc(m.u.certificate.ncert * sizeof(Bytes*));
-		m.u.certificate.certs[0] = makebytes(cert, certlen);
+		m.u.certificate.certs[0] = makebytes(*cert, certlen);
 		for (i = 0; i < numcerts && chp; i++, chp = chp->next)
 			m.u.certificate.certs[i+1] = makebytes(chp->pem, chp->pemlen);
 		if(!msgSend(c, &m, AQueue))
@@ -2113,6 +2144,8 @@
 	factotum_rsa_close(c->sec->rpc);
 	rsapubfree(c->sec->rsapub);
 	freebytes(c->cert);
+
+	free(c->serverName);
 
 	memset(c, 0, sizeof(*c));
 	free(c);

Re: [9front] [PATCH] libsec, pushtls, tlssrv: add support for Server Name Indication (SNI) extension

[cinap_lenrek]

I think you'd really want to have a new tlsServerX() function for this
with a callback to provide the certificate giving the server name.

Or alternatively we pass a whole array of certificates in,
and the server matches the certificate subject(s). For that
programs could maybe accept multiple -c arguments.

Hardcoding some path with achmed in there feels very wrong.

The whole SNI stuff is a gigant layer-violation in my opinion,
and if you use it, you are going to want to expose the effective
server name (like putenv() in tlssrv).

--
cinap

Re: [9front] [PATCH] libsec, pushtls, tlssrv: add support for Server Name Indication (SNI) extension

[igor]

Quoth cinap_lenrek@felloff.net:
> I think you'd really want to have a new tlsServerX() function for this
> with a callback to provide the certificate giving the server name.
> 
> Or alternatively we pass a whole array of certificates in,
> and the server matches the certificate subject(s). For that
> programs could maybe accept multiple -c arguments.
> 
> Hardcoding some path with achmed in there feels very wrong.

Yes.  The current patch more or less just serves the function to get
the conversation started using a working implementation that aimed at
keeping the amount of differences to a bare minimum.

So the proposal is to introduce a (1) new tlsServerX() function using
a (2) lookup interface that provides a certificate given a server
name. The 'database' of lookup results (i.e.  the certificates)
is indexed by certificate subjects extracted from certificates that
are passed via multiple (3) '-c arguments' to tlssrv to be matched
against whatever is provided via the SNI extension.

That seems reasonable.  The presence of multiple '-c' arguments would
then be an indicator to call 'tlsServerX()' with the SNI extension; a
single '-c' argument would go down the well trodden 'tlsServer2()'
path.

The only worry would be some level of code duplication but this might
just be a bullshit argument.  Let me try it and present that solution
for further discussion.

> The whole SNI stuff is a gigant layer-violation in my opinion,
> and if you use it, you are going to want to expose the effective
> server name (like putenv() in tlssrv).

Ok. I have to admit I am not entirely sure what you mean by 'expose
the effective server name (like putenv() in tlssrv)'.  Do you mean
there is an env variable we ought to set before calling say tcp80 or
rc-httpd that provides the effective server name tlssrv parsed via
SNI?

Cheers,
Igor

Re: [9front] [PATCH] libsec, pushtls, tlssrv: add support for Server Name Indication (SNI) extension

[cinap_lenrek]

> Yes.  The current patch more or less just serves the function to get
> the conversation started using a working implementation that aimed at
> keeping the amount of differences to a bare minimum.

yeah, thats totally reasonable. thank you btw :)

> So the proposal is to introduce a (1) new tlsServerX() function using
> a (2) lookup interface that provides a certificate given a server
> name. The 'database' of lookup results (i.e.  the certificates)
> is indexed by certificate subjects extracted from certificates that
> are passed via multiple (3) '-c arguments' to tlssrv to be matched
> against whatever is provided via the SNI extension.

i see the following options:

1) new tlsServerX() function with a callback that returns the certificate
   for a given server-name.
2) new tlsServerY() function that lets you pass multiple certificates.
3) we keep tlsServer() as it is and require the user to get a multi-domain
   certificate (just has multiple subjects).

for cases 1 and 2 we need to think how that would be handled by the
server in question (what i said about passing multiple -c arguments).

> Ok. I have to admit I am not entirely sure what you mean by 'expose
> the effective server name (like putenv() in tlssrv)'.  Do you mean
> there is an env variable we ought to set before calling say tcp80 or
> rc-httpd that provides the effective server name tlssrv parsed via
> SNI?

yes, exactly. the service should get the effective domain negotiated
from the handshake. the whole reason this SNI complication exist is that
people ran out of ipv4 addresses and start sharing a single service
port with dozens of different domains. pushing all that muxing into
the application layer that the ip layer would normally handle. for
that muxing to be effective, the service then needs to know what
domain/serverName it is.

--
cinap

Re: [9front] [PATCH] libsec, pushtls, tlssrv: add support for Server Name Indication (SNI) extension

[igor]

Quoth cinap_lenrek@felloff.net:
[…]
> i see the following options:
> 
> 1) new tlsServerX() function with a callback that returns the certificate
>    for a given server-name.
> 2) new tlsServerY() function that lets you pass multiple certificates.
> 3) we keep tlsServer() as it is and require the user to get a multi-domain
>    certificate (just has multiple subjects).
> 
> for cases 1 and 2 we need to think how that would be handled by the
> server in question (what i said about passing multiple -c arguments).

This reiteration was indeed helpful in clearing up the proposal!

> > Ok. I have to admit I am not entirely sure what you mean by 'expose
> > the effective server name (like putenv() in tlssrv)'.  Do you mean
> > there is an env variable we ought to set before calling say tcp80 or
> > rc-httpd that provides the effective server name tlssrv parsed via
> > SNI?
> 
> yes, exactly. the service should get the effective domain negotiated
> from the handshake. the whole reason this SNI complication exist is that
> people ran out of ipv4 addresses and start sharing a single service
> port with dozens of different domains. pushing all that muxing into
> the application layer that the ip layer would normally handle. for
> that muxing to be effective, the service then needs to know what
> domain/serverName it is.

Thanks for elucidating this point; the penny finally dropped on my
end ☺

Cheers,
Igor