Skip to content

Commit

Permalink
Remove OpenSSL-based signing
Browse files Browse the repository at this point in the history
  • Loading branch information
edolstra committed May 4, 2016
1 parent dfebfc8 commit f435f82
Show file tree
Hide file tree
Showing 16 changed files with 52 additions and 142 deletions.
4 changes: 0 additions & 4 deletions configure.ac
Expand Up @@ -168,10 +168,6 @@ AC_SUBST(storedir)


# Look for OpenSSL, a required dependency.
AC_PATH_PROG(openssl, openssl, openssl) # if not found, call openssl in $PATH
AC_SUBST(openssl)
AC_DEFINE_UNQUOTED(OPENSSL_PATH, ["$openssl"], [Path of the OpenSSL binary])

PKG_CHECK_MODULES([OPENSSL], [libcrypto], [CXXFLAGS="$OPENSSL_CFLAGS $CXXFLAGS"])


Expand Down
1 change: 0 additions & 1 deletion perl/lib/Nix/Config.pm.in
Expand Up @@ -14,7 +14,6 @@ $storeDir = $ENV{"NIX_STORE_DIR"} || "@storedir@";
$bzip2 = "@bzip2@";
$xz = "@xz@";
$curl = "@curl@";
$openssl = "@openssl@";

$useBindings = "@perlbindings@" eq "yes";

Expand Down
12 changes: 6 additions & 6 deletions perl/lib/Nix/CopyClosure.pm
Expand Up @@ -10,7 +10,7 @@ use IPC::Open2;


sub copyToOpen {
my ($from, $to, $sshHost, $storePaths, $includeOutputs, $dryRun, $sign, $useSubstitutes) = @_;
my ($from, $to, $sshHost, $storePaths, $includeOutputs, $dryRun, $useSubstitutes) = @_;

$useSubstitutes = 0 if $dryRun || !defined $useSubstitutes;

Expand Down Expand Up @@ -41,13 +41,13 @@ sub copyToOpen {

# Send the "import paths" command.
syswrite($to, pack("L<x4", 4)) or die;
exportPaths(fileno($to), $sign, @missing);
exportPaths(fileno($to), @missing);
readInt($from) == 1 or die "remote machine ‘$sshHost’ failed to import closure\n";
}


sub copyTo {
my ($sshHost, $storePaths, $includeOutputs, $dryRun, $sign, $useSubstitutes) = @_;
my ($sshHost, $storePaths, $includeOutputs, $dryRun, $useSubstitutes) = @_;

# Connect to the remote host.
my ($from, $to);
Expand All @@ -61,7 +61,7 @@ sub copyTo {
return oldCopyTo(@_);
}

copyToOpen($from, $to, $sshHost, $storePaths, $includeOutputs, $dryRun, $sign, $useSubstitutes);
copyToOpen($from, $to, $sshHost, $storePaths, $includeOutputs, $dryRun, $useSubstitutes);

close $to;
}
Expand All @@ -70,7 +70,7 @@ sub copyTo {
# For backwards compatibility with Nix <= 1.7. Will be removed
# eventually.
sub oldCopyTo {
my ($sshHost, $storePaths, $includeOutputs, $dryRun, $sign, $useSubstitutes) = @_;
my ($sshHost, $storePaths, $includeOutputs, $dryRun, $useSubstitutes) = @_;

# Get the closure of this path.
my @closure = reverse(topoSortPaths(computeFSClosure(0, $includeOutputs,
Expand Down Expand Up @@ -105,7 +105,7 @@ sub oldCopyTo {
print STDERR "copying ", scalar @missing, " missing paths to ‘$sshHost’...\n";
unless ($dryRun) {
open SSH, "| ssh $sshHost @globalSshOpts 'nix-store --import' > /dev/null" or die;
exportPaths(fileno(SSH), $sign, @missing);
exportPaths(fileno(SSH), @missing);
close SSH or die "copying store paths to remote machine ‘$sshHost’ failed: $?";
}
}
Expand Down
6 changes: 3 additions & 3 deletions perl/lib/Nix/Store.xs
Expand Up @@ -169,13 +169,13 @@ SV * followLinksToStorePath(char * path)
RETVAL


void exportPaths(int fd, int sign, ...)
void exportPaths(int fd, ...)
PPCODE:
try {
Paths paths;
for (int n = 2; n < items; ++n) paths.push_back(SvPV_nolen(ST(n)));
FdSink sink(fd);
store()->exportPaths(paths, sign, sink);
store()->exportPaths(paths, sink);
} catch (Error & e) {
croak("%s", e.what());
}
Expand All @@ -185,7 +185,7 @@ void importPaths(int fd)
PPCODE:
try {
FdSource source(fd);
store()->importPaths(false, source, 0);
store()->importPaths(source, 0);
} catch (Error & e) {
croak("%s", e.what());
}
Expand Down
6 changes: 1 addition & 5 deletions scripts/build-remote.pl.in
Expand Up @@ -223,10 +223,6 @@ my @inputs = split /\s/, readline(STDIN);
my @outputs = split /\s/, readline(STDIN);


my $maybeSign = "";
$maybeSign = "--sign" if -e "$Nix::Config::confDir/signing-key.sec";


# Copy the derivation and its dependencies to the build machine. This
# is guarded by an exclusive lock per machine to prevent multiple
# build-remote instances from copying to a machine simultaneously.
Expand All @@ -250,7 +246,7 @@ if ($@) {
print STDERR "somebody is hogging $uploadLock, continuing...\n";
unlink $uploadLock;
}
Nix::CopyClosure::copyToOpen($from, $to, $hostName, [ $drvPath, @inputs ], 0, 0, $maybeSign ne "");
Nix::CopyClosure::copyToOpen($from, $to, $hostName, [ $drvPath, @inputs ], 0, 0);
close UPLOADLOCK;


Expand Down
10 changes: 3 additions & 7 deletions scripts/nix-copy-closure.in
Expand Up @@ -12,7 +12,7 @@ binmode STDERR, ":encoding(utf8)";

if (scalar @ARGV < 1) {
print STDERR <<EOF
Usage: nix-copy-closure [--from | --to] HOSTNAME [--sign] [--gzip] [--bzip2] [--xz] PATHS...
Usage: nix-copy-closure [--from | --to] HOSTNAME [--gzip] [--bzip2] [--xz] PATHS...
EOF
;
exit 1;
Expand All @@ -21,7 +21,6 @@ EOF

# Get the target host.
my $sshHost;
my $sign = 0;
my $toMode = 1;
my $includeOutputs = 0;
my $dryRun = 0;
Expand All @@ -38,9 +37,6 @@ while (@ARGV) {
if ($arg eq "--help") {
exec "man nix-copy-closure" or die;
}
elsif ($arg eq "--sign") {
$sign = 1;
}
elsif ($arg eq "--gzip" || $arg eq "--bzip2" || $arg eq "--xz") {
warn "$0: ‘$arg’ is not implemented\n" if $arg ne "--gzip";
push @globalSshOpts, "-C";
Expand Down Expand Up @@ -81,7 +77,7 @@ die "$0: you did not specify a host name\n" unless defined $sshHost;
if ($toMode) { # Copy TO the remote machine.
Nix::CopyClosure::copyTo(
$sshHost, [ @storePaths ],
$includeOutputs, $dryRun, $sign, $useSubstitutes);
$includeOutputs, $dryRun, $useSubstitutes);
}

else { # Copy FROM the remote machine.
Expand All @@ -99,7 +95,7 @@ else { # Copy FROM the remote machine.
if (scalar @missing > 0) {
print STDERR "copying ", scalar @missing, " missing paths from ‘$sshHost’...\n";
writeInt(5, $to); # == cmdExportPaths
writeInt($sign ? 1 : 0, $to);
writeInt(0, $to); # obsolete
writeStrings(\@missing, $to);
importPaths(fileno($from));
}
Expand Down
9 changes: 3 additions & 6 deletions src/libstore/binary-cache-store.cc
Expand Up @@ -156,10 +156,8 @@ void BinaryCacheStore::narFromPath(const Path & storePath, Sink & sink)
sink((unsigned char *) nar->c_str(), nar->size());
}

void BinaryCacheStore::exportPath(const Path & storePath, bool sign, Sink & sink)
void BinaryCacheStore::exportPath(const Path & storePath, Sink & sink)
{
assert(!sign);

auto res = queryPathInfo(storePath);

narFromPath(storePath, sink);
Expand All @@ -169,10 +167,9 @@ void BinaryCacheStore::exportPath(const Path & storePath, bool sign, Sink & sink
sink << exportMagic << storePath << res->references << res->deriver << 0;
}

Paths BinaryCacheStore::importPaths(bool requireSignature, Source & source,
Paths BinaryCacheStore::importPaths(Source & source,
std::shared_ptr<FSAccessor> accessor)
{
assert(!requireSignature);
Paths res;
while (true) {
unsigned long long n = readLongLong(source);
Expand Down Expand Up @@ -346,7 +343,7 @@ struct BinaryCacheStoreAccessor : public FSAccessor
if (i != nars.end()) return {i->second, restPath};

StringSink sink;
store->exportPath(storePath, false, sink);
store->exportPath(storePath, sink);

auto accessor = makeNarAccessor(sink.s);
nars.emplace(storePath, accessor);
Expand Down
4 changes: 2 additions & 2 deletions src/libstore/binary-cache-store.hh
Expand Up @@ -91,9 +91,9 @@ public:

void narFromPath(const Path & path, Sink & sink) override;

void exportPath(const Path & path, bool sign, Sink & sink) override;
void exportPath(const Path & path, Sink & sink) override;

Paths importPaths(bool requireSignature, Source & source,
Paths importPaths(Source & source,
std::shared_ptr<FSAccessor> accessor) override;

Path importPath(Source & source, std::shared_ptr<FSAccessor> accessor);
Expand Down
77 changes: 8 additions & 69 deletions src/libstore/local-store.cc
Expand Up @@ -1035,18 +1035,7 @@ struct HashAndWriteSink : Sink
};


static void checkSecrecy(const Path & path)
{
struct stat st;
if (stat(path.c_str(), &st))
throw SysError(format("getting status of ‘%1%’") % path);
if ((st.st_mode & (S_IRWXG | S_IRWXO)) != 0)
throw Error(format("file ‘%1%’ should be secret (inaccessible to everybody else)!") % path);
}


void LocalStore::exportPath(const Path & path, bool sign,
Sink & sink)
void LocalStore::exportPath(const Path & path, Sink & sink)
{
assertStorePath(path);

Expand All @@ -1068,30 +1057,7 @@ void LocalStore::exportPath(const Path & path, bool sign,

hashAndWriteSink << exportMagic << path << info->references << info->deriver;

if (sign) {
Hash hash = hashAndWriteSink.currentHash();

Path tmpDir = createTempDir();
AutoDelete delTmp(tmpDir);
Path hashFile = tmpDir + "/hash";
writeFile(hashFile, printHash(hash));

Path secretKey = settings.nixConfDir + "/signing-key.sec";
checkSecrecy(secretKey);

Strings args;
args.push_back("rsautl");
args.push_back("-sign");
args.push_back("-inkey");
args.push_back(secretKey);
args.push_back("-in");
args.push_back(hashFile);
string signature = runProgram(OPENSSL_PATH, true, args);

hashAndWriteSink << 1 << signature;

} else
hashAndWriteSink << 0;
hashAndWriteSink << 0; // backwards compatibility
}


Expand Down Expand Up @@ -1129,7 +1095,7 @@ Path LocalStore::createTempDirInStore()
}


Path LocalStore::importPath(bool requireSignature, Source & source)
Path LocalStore::importPath(Source & source)
{
HashAndReadSource hashAndReadSource(source);

Expand Down Expand Up @@ -1160,36 +1126,9 @@ Path LocalStore::importPath(bool requireSignature, Source & source)

bool haveSignature = readInt(hashAndReadSource) == 1;

if (requireSignature && !haveSignature)
throw Error(format("imported archive of ‘%1%’ lacks a signature") % dstPath);

if (haveSignature) {
string signature = readString(hashAndReadSource);

if (requireSignature) {
Path sigFile = tmpDir + "/sig";
writeFile(sigFile, signature);

Strings args;
args.push_back("rsautl");
args.push_back("-verify");
args.push_back("-inkey");
args.push_back(settings.nixConfDir + "/signing-key.pub");
args.push_back("-pubin");
args.push_back("-in");
args.push_back(sigFile);
string hash2 = runProgram(OPENSSL_PATH, true, args);

/* Note: runProgram() throws an exception if the signature
is invalid. */

if (printHash(hash) != hash2)
throw Error(
"signed hash doesn't match actual contents of imported "
"archive; archive could be corrupt, or someone is trying "
"to import a Trojan horse");
}
}
if (haveSignature)
// Ignore legacy signature.
readString(hashAndReadSource);

/* Do the actual import. */

Expand Down Expand Up @@ -1239,15 +1178,15 @@ Path LocalStore::importPath(bool requireSignature, Source & source)
}


Paths LocalStore::importPaths(bool requireSignature, Source & source,
Paths LocalStore::importPaths(Source & source,
std::shared_ptr<FSAccessor> accessor)
{
Paths res;
while (true) {
unsigned long long n = readLongLong(source);
if (n == 0) break;
if (n != 1) throw Error("input doesn't look like something created by ‘nix-store --export’");
res.push_back(importPath(requireSignature, source));
res.push_back(importPath(source));
}
return res;
}
Expand Down
7 changes: 3 additions & 4 deletions src/libstore/local-store.hh
Expand Up @@ -126,10 +126,9 @@ public:
Path addTextToStore(const string & name, const string & s,
const PathSet & references, bool repair = false) override;

void exportPath(const Path & path, bool sign,
Sink & sink) override;
void exportPath(const Path & path, Sink & sink) override;

Paths importPaths(bool requireSignature, Source & source,
Paths importPaths(Source & source,
std::shared_ptr<FSAccessor> accessor) override;

void buildPaths(const PathSet & paths, BuildMode buildMode) override;
Expand Down Expand Up @@ -230,7 +229,7 @@ private:

Path createTempDirInStore();

Path importPath(bool requireSignature, Source & source);
Path importPath(Source & source);

void checkDerivationOutputs(const Path & drvPath, const Derivation & drv);

Expand Down
9 changes: 3 additions & 6 deletions src/libstore/remote-store.cc
Expand Up @@ -373,23 +373,20 @@ Path RemoteStore::addTextToStore(const string & name, const string & s,
}


void RemoteStore::exportPath(const Path & path, bool sign,
Sink & sink)
void RemoteStore::exportPath(const Path & path, Sink & sink)
{
auto conn(connections->get());
conn->to << wopExportPath << path << (sign ? 1 : 0);
conn->to << wopExportPath << path << 0;
conn->processStderr(&sink); /* sink receives the actual data */
readInt(conn->from);
}


Paths RemoteStore::importPaths(bool requireSignature, Source & source,
Paths RemoteStore::importPaths(Source & source,
std::shared_ptr<FSAccessor> accessor)
{
auto conn(connections->get());
conn->to << wopImportPaths;
/* We ignore requireSignature, since the worker forces it to true
anyway. */
conn->processStderr(0, &source);
return readStorePaths<Paths>(conn->from);
}
Expand Down
5 changes: 2 additions & 3 deletions src/libstore/remote-store.hh
Expand Up @@ -58,10 +58,9 @@ public:
Path addTextToStore(const string & name, const string & s,
const PathSet & references, bool repair = false) override;

void exportPath(const Path & path, bool sign,
Sink & sink) override;
void exportPath(const Path & path, Sink & sink) override;

Paths importPaths(bool requireSignature, Source & source,
Paths importPaths(Source & source,
std::shared_ptr<FSAccessor> accessor) override;

void buildPaths(const PathSet & paths, BuildMode buildMode) override;
Expand Down

0 comments on commit f435f82

Please sign in to comment.