From c179bb28feb8957da35b66d90612c00b80a6e167 Mon Sep 17 00:00:00 2001 From: Ben Olden-Cooligan Date: Mon, 18 Mar 2019 15:22:44 -0400 Subject: [PATCH] Use TLS for the worker channel --- .../EntryPoints/WorkerEntryPoint.cs | 21 +++++++++++++----- NAPS2.Sdk.Tests/Worker/WorkerChannelTests.cs | 20 ++++++++++++++--- NAPS2.Sdk/Util/SslHelper.cs | 4 ++-- NAPS2.Sdk/Worker/GrpcHelper.cs | 10 +++++++++ NAPS2.Sdk/Worker/GrpcWorkerServiceAdapter.cs | 5 +++-- NAPS2.Sdk/Worker/WorkerManager.cs | 22 +++++++++++++++---- NAPS2.sln.DotSettings | 1 + 7 files changed, 67 insertions(+), 16 deletions(-) diff --git a/NAPS2.Lib.Common/EntryPoints/WorkerEntryPoint.cs b/NAPS2.Lib.Common/EntryPoints/WorkerEntryPoint.cs index eafc8e488..761d35e61 100644 --- a/NAPS2.Lib.Common/EntryPoints/WorkerEntryPoint.cs +++ b/NAPS2.Lib.Common/EntryPoints/WorkerEntryPoint.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; using System.ServiceModel; +using System.Text; using System.Threading; using System.Threading.Tasks; using System.Windows.Forms; @@ -51,16 +52,20 @@ namespace NAPS2.DI.EntryPoints var form = new BackgroundForm(); Invoker.Current = form; - var (rootCert, rootPrivate) = SslHelper.GenerateRootCertificate(); - //var (cert, privateKey) = SslHelper.GenerateCertificateChain(rootCert, rootPrivate); - var creds = new SslServerCredentials(new[] {new KeyCertificatePair(rootCert, rootPrivate)}, rootCert, - SslClientCertificateRequestType.RequestAndRequireAndVerify); + // We share the TLS public and private key between worker and parent; + // this is equivalent to using a symmetric key. The only point is to + // prevent other applications sniffing the TCP traffic, since there's + // already full trust between worker and parent, and stdin/stdout are + // secure channels for key sharing. + var cert = ReadEncodedString(); + var privateKey = ReadEncodedString(); + var creds = GrpcHelper.GetServerCreds(cert, privateKey); // Connect to the main NAPS2 process and listen for assigned work Server server = new Server { Services = { GrpcWorkerService.BindService(kernel.Get()) }, - Ports = { new ServerPort("localhost", 0, ServerCredentials.Insecure) } + Ports = { new ServerPort("localhost", 0, creds) } }; server.Start(); try @@ -82,6 +87,12 @@ namespace NAPS2.DI.EntryPoints } } + private static string ReadEncodedString() + { + string input = Console.ReadLine() ?? throw new InvalidOperationException("No input"); + return Encoding.UTF8.GetString(Convert.FromBase64String(input)); + } + private static bool IsProcessRunning(int procId) { try diff --git a/NAPS2.Sdk.Tests/Worker/WorkerChannelTests.cs b/NAPS2.Sdk.Tests/Worker/WorkerChannelTests.cs index 35ab33457..879980b1c 100644 --- a/NAPS2.Sdk.Tests/Worker/WorkerChannelTests.cs +++ b/NAPS2.Sdk.Tests/Worker/WorkerChannelTests.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Drawing; using System.Linq; +using System.Security.Cryptography.X509Certificates; using System.Threading; using System.Threading.Tasks; using Grpc.Core; @@ -12,6 +13,7 @@ using NAPS2.ImportExport.Email.Mapi; using NAPS2.Scan; using NAPS2.Scan.Exceptions; using NAPS2.Scan.Twain; +using NAPS2.Util; using NAPS2.Worker; using Xunit; @@ -19,15 +21,15 @@ namespace NAPS2.Sdk.Tests.Worker { public class WorkerChannelTests { - private Channel Start(ITwainWrapper twainWrapper = null, ThumbnailRenderer thumbnailRenderer = null, IMapiWrapper mapiWrapper = null) + private Channel Start(ITwainWrapper twainWrapper = null, ThumbnailRenderer thumbnailRenderer = null, IMapiWrapper mapiWrapper = null, ServerCredentials serverCreds = null, ChannelCredentials clientCreds = null) { Server server = new Server { Services = { GrpcWorkerService.BindService(new GrpcWorkerServiceImpl(twainWrapper, thumbnailRenderer, mapiWrapper)) }, - Ports = { new ServerPort("localhost", 0, ServerCredentials.Insecure) } + Ports = { new ServerPort("localhost", 0, serverCreds ?? ServerCredentials.Insecure) } }; server.Start(); - var client = new GrpcWorkerServiceAdapter(server.Ports.First().BoundPort); + var client = new GrpcWorkerServiceAdapter(server.Ports.First().BoundPort, clientCreds ?? ChannelCredentials.Insecure); return new Channel { Server = server, @@ -35,6 +37,18 @@ namespace NAPS2.Sdk.Tests.Worker }; } + [Fact] + public void SslCreds() + { + var (cert, privateKey) = SslHelper.GenerateRootCertificate(); + var serverCreds = GrpcHelper.GetServerCreds(cert, privateKey); + var clientCreds = GrpcHelper.GetClientCreds(cert, privateKey); + using (var channel = Start(serverCreds: serverCreds, clientCreds: clientCreds)) + { + channel.Client.Init(null); + } + } + [Fact] public void Init() { diff --git a/NAPS2.Sdk/Util/SslHelper.cs b/NAPS2.Sdk/Util/SslHelper.cs index 4f69ea65b..29eee1de6 100644 --- a/NAPS2.Sdk/Util/SslHelper.cs +++ b/NAPS2.Sdk/Util/SslHelper.cs @@ -42,8 +42,8 @@ namespace NAPS2.Util { var gen = new X509V3CertificateGenerator(); gen.SetPublicKey(keyPair.Public); - gen.SetIssuerDN(new X509Name("CN=naps2")); - gen.SetSubjectDN(new X509Name("CN=naps2")); + gen.SetIssuerDN(new X509Name("CN=localhost")); + gen.SetSubjectDN(new X509Name("CN=localhost")); gen.SetNotBefore(DateTime.Now - TimeSpan.FromDays(2)); gen.SetNotAfter(DateTime.Now + TimeSpan.FromDays(365)); gen.SetSerialNumber(new BigInteger(128, Random)); diff --git a/NAPS2.Sdk/Worker/GrpcHelper.cs b/NAPS2.Sdk/Worker/GrpcHelper.cs index 95db1e513..2849f5c89 100644 --- a/NAPS2.Sdk/Worker/GrpcHelper.cs +++ b/NAPS2.Sdk/Worker/GrpcHelper.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; using System.Reflection; using System.Threading.Tasks; +using Grpc.Core; using NAPS2.Scan.Exceptions; using NAPS2.Util; @@ -77,5 +78,14 @@ namespace NAPS2.Worker error(ToError(e)); } }); + + public static ChannelCredentials GetClientCreds(string cert, string privateKey) => + new SslCredentials(cert, new KeyCertificatePair(cert, privateKey)); + + public static ServerCredentials GetServerCreds(string cert, string privateKey) => + new SslServerCredentials( + new[] { new KeyCertificatePair(cert, privateKey) }, + cert, + SslClientCertificateRequestType.RequestAndRequireAndVerify); } } diff --git a/NAPS2.Sdk/Worker/GrpcWorkerServiceAdapter.cs b/NAPS2.Sdk/Worker/GrpcWorkerServiceAdapter.cs index 3e49cae5c..89b469309 100644 --- a/NAPS2.Sdk/Worker/GrpcWorkerServiceAdapter.cs +++ b/NAPS2.Sdk/Worker/GrpcWorkerServiceAdapter.cs @@ -19,10 +19,11 @@ namespace NAPS2.Worker { private readonly GrpcWorkerService.GrpcWorkerServiceClient client; - public GrpcWorkerServiceAdapter(int port) + public GrpcWorkerServiceAdapter(int port, ChannelCredentials creds) { - client = new GrpcWorkerService.GrpcWorkerServiceClient(new Channel("localhost", port, ChannelCredentials.Insecure)); + client = new GrpcWorkerService.GrpcWorkerServiceClient(new Channel("localhost", port, creds)); } + public void Init(string recoveryFolderPath) { var req = new InitRequest { RecoveryFolderPath = recoveryFolderPath ?? "" }; diff --git a/NAPS2.Sdk/Worker/WorkerManager.cs b/NAPS2.Sdk/Worker/WorkerManager.cs index f423e072e..a5a8b836c 100644 --- a/NAPS2.Sdk/Worker/WorkerManager.cs +++ b/NAPS2.Sdk/Worker/WorkerManager.cs @@ -5,8 +5,12 @@ using System.Diagnostics; using System.IO; using System.Linq; using System.Reflection; +using System.Security.Cryptography.X509Certificates; +using System.Text; using System.Threading.Tasks; +using Grpc.Core; using NAPS2.Platform; +using NAPS2.Util; namespace NAPS2.Worker { @@ -45,13 +49,14 @@ namespace NAPS2.Worker } } - private static (Process, int) StartWorkerProcess() + private static (Process, int, string, string) StartWorkerProcess() { var parentId = Process.GetCurrentProcess().Id; var proc = Process.Start(new ProcessStartInfo { FileName = PlatformCompat.Runtime.ExeRunner ?? WorkerExePath, Arguments = PlatformCompat.Runtime.ExeRunner != null ? $"{WorkerExePath} {parentId}" : $"{parentId}", + RedirectStandardInput = true, RedirectStandardOutput = true, UseShellExecute = false }); @@ -74,17 +79,26 @@ namespace NAPS2.Worker } } + var (cert, privateKey) = SslHelper.GenerateRootCertificate(); + WriteEncodedString(proc.StandardInput, cert); + WriteEncodedString(proc.StandardInput, privateKey); int port = int.Parse(proc.StandardOutput.ReadLine() ?? throw new Exception("Could not read worker port")); - return (proc, port); + return (proc, port, cert, privateKey); + } + + private static void WriteEncodedString(StreamWriter streamWriter, string value) + { + streamWriter.WriteLine(Convert.ToBase64String(Encoding.UTF8.GetBytes(value))); } private static void StartWorkerService() { Task.Factory.StartNew(() => { - var (proc, port) = StartWorkerProcess(); - _workerQueue.Add(new WorkerContext { Service = new GrpcWorkerServiceAdapter(port), Process = proc }); + var (proc, port, cert, privateKey) = StartWorkerProcess(); + var creds = GrpcHelper.GetClientCreds(cert, privateKey); + _workerQueue.Add(new WorkerContext { Service = new GrpcWorkerServiceAdapter(port, creds), Process = proc }); }); } diff --git a/NAPS2.sln.DotSettings b/NAPS2.sln.DotSettings index c4637d0a5..25c881467 100644 --- a/NAPS2.sln.DotSettings +++ b/NAPS2.sln.DotSettings @@ -6,6 +6,7 @@ <Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /> <data><IncludeFilters /><ExcludeFilters /></data> <data /> + True True True True