Skip to content

Commit

Permalink
crypto: have fixed NodeBIOs return EOF.
Browse files Browse the repository at this point in the history
Prior to this change, the NodeBIO objects used to wrap fixed data had
`num` equal to -1. This caused them to return -1 and set the retry flags
when they ran out of data. Since the data is fixed, that's incorrect.
Instead they should return zero to signal EOF.

This change adds a new, static function, NodeBIO::NewFixed to create a
BIO that wraps fixed data and which returns zero when exhausted.

The practical impact of this is limited since most (all?) the parsing
functions that these BIOs get passed to consider any return value less
than one to be EOF and ignore the retry flags anyway.
  • Loading branch information
agl committed Feb 11, 2016
1 parent 1124de2 commit 26f2daf
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 20 deletions.
26 changes: 6 additions & 20 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -416,29 +416,18 @@ void SecureContext::Init(const FunctionCallbackInfo<Value>& args) {
// Takes a string or buffer and loads it into a BIO.
// Caller responsible for BIO_free_all-ing the returned object.
static BIO* LoadBIO(Environment* env, Local<Value> v) {
BIO* bio = NodeBIO::New();
if (!bio)
return nullptr;

HandleScope scope(env->isolate());

int r = -1;

if (v->IsString()) {
const node::Utf8Value s(env->isolate(), v);
r = BIO_write(bio, *s, s.length());
} else if (Buffer::HasInstance(v)) {
char* buffer_data = Buffer::Data(v);
size_t buffer_length = Buffer::Length(v);
r = BIO_write(bio, buffer_data, buffer_length);
return NodeBIO::NewFixed(*s, s.length());
}

if (r <= 0) {
BIO_free_all(bio);
return nullptr;
if (Buffer::HasInstance(v)) {
return NodeBIO::NewFixed(Buffer::Data(v), Buffer::Length(v));
}

return bio;
return nullptr;
}


Expand Down Expand Up @@ -761,15 +750,12 @@ void SecureContext::AddRootCerts(const FunctionCallbackInfo<Value>& args) {
root_cert_store = X509_STORE_new();

for (size_t i = 0; i < ARRAY_SIZE(root_certs); i++) {
BIO* bp = NodeBIO::New();

if (!BIO_write(bp, root_certs[i], strlen(root_certs[i]))) {
BIO_free_all(bp);
BIO* bp = NodeBIO::NewFixed(root_certs[i], strlen(root_certs[i]));
if (bp == nullptr) {
return;
}

X509 *x509 = PEM_read_bio_X509(bp, nullptr, CryptoPemCallback, nullptr);

if (x509 == nullptr) {
BIO_free_all(bp);
return;
Expand Down
16 changes: 16 additions & 0 deletions src/node_crypto_bio.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include "openssl/bio.h"
#include "util.h"
#include "util-inl.h"
#include <limits.h>
#include <string.h>

namespace node {
Expand All @@ -27,6 +28,21 @@ BIO* NodeBIO::New() {
}


BIO* NodeBIO::NewFixed(const char* data, size_t len) {
BIO* bio = New();

if (bio == nullptr ||
len > INT_MAX ||
BIO_write(bio, data, len) != static_cast<int>(len) ||
BIO_set_mem_eof_return(bio, 0) != 1) {
BIO_free(bio);
return nullptr;
}

return bio;
}


void NodeBIO::AssignEnvironment(Environment* env) {
env_ = env;
}
Expand Down
4 changes: 4 additions & 0 deletions src/node_crypto_bio.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ class NodeBIO {

static BIO* New();

// NewFixed takes a copy of `len` bytes from `data` and returns a BIO that,
// when read from, returns those bytes followed by EOF.
static BIO* NewFixed(const char* data, size_t len);

void AssignEnvironment(Environment* env);

// Move read head to next buffer if needed
Expand Down

0 comments on commit 26f2daf

Please sign in to comment.