Skip to content

Commit d541639

Browse files
committed
Merge remote-tracking branch 'upstream/main' into ec-key-infinity-rejection
2 parents 9be02bb + a9e26fe commit d541639

File tree

5 files changed

+45
-36
lines changed

5 files changed

+45
-36
lines changed

crypto/ocsp/ocsp_verify.c

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ static X509 *ocsp_find_signer_sk(STACK_OF(X509) *certs, OCSP_RESPID *id) {
3535
for (size_t i = 0; i < sk_X509_num(certs); i++) {
3636
cert = sk_X509_value(certs, i);
3737
if (X509_pubkey_digest(cert, EVP_sha1(), tmphash, NULL)) {
38-
if (memcmp(keyhash, tmphash, SHA_DIGEST_LENGTH) == 0) {
38+
if (OPENSSL_memcmp(keyhash, tmphash, SHA_DIGEST_LENGTH) == 0) {
3939
return cert;
4040
}
4141
}
@@ -201,6 +201,8 @@ static int ocsp_check_ids(STACK_OF(OCSP_SINGLERESP) *sresp, OCSP_CERTID **ret) {
201201
return 1;
202202
}
203203

204+
// Returns -1 on fatal error, 0 if there is no match and 1 if there is a
205+
// match.
204206
static int ocsp_match_issuerid(X509 *cert, OCSP_CERTID *cid,
205207
STACK_OF(OCSP_SINGLERESP) *sresp) {
206208
if (cert == NULL) {
@@ -232,13 +234,14 @@ static int ocsp_match_issuerid(X509 *cert, OCSP_CERTID *cid,
232234
return 0;
233235
}
234236
}
235-
if (memcmp(md, cid->issuerNameHash->data, mdlen) != 0) {
237+
if (0 != OPENSSL_memcmp(md, cid->issuerNameHash->data, mdlen)) {
236238
return 0;
237239
}
238-
if (0 <= X509_pubkey_digest(cert, dgst, md, NULL)) {
239-
if (memcmp(md, cid->issuerKeyHash->data, mdlen) != 0) {
240-
return 0;
241-
}
240+
if (1 != X509_pubkey_digest(cert, dgst, md, NULL)) {
241+
return -1;
242+
}
243+
if (0 != OPENSSL_memcmp(md, cid->issuerKeyHash->data, mdlen)) {
244+
return 0;
242245
}
243246
return 1;
244247

@@ -473,8 +476,8 @@ int OCSP_request_verify(OCSP_REQUEST *req, STACK_OF(X509) *certs,
473476
int ret = 0;
474477
if (!IS_OCSP_FLAG_SET(flags, OCSP_NOVERIFY)) {
475478
// Initialize and set purpose of |ctx| for verification.
476-
if (!X509_STORE_CTX_init(ctx, store, signer, NULL) &&
477-
!X509_STORE_CTX_set_purpose(ctx, X509_PURPOSE_OCSP_HELPER)) {
479+
if (1 != X509_STORE_CTX_init(ctx, store, signer, NULL) ||
480+
1 != X509_STORE_CTX_set_purpose(ctx, X509_PURPOSE_OCSP_HELPER)) {
478481
OPENSSL_PUT_ERROR(OCSP, ERR_R_X509_LIB);
479482
goto end;
480483
}

crypto/x509/x509_vfy.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -676,7 +676,7 @@ static int check_chain_extensions(X509_STORE_CTX *ctx) {
676676
}
677677
// Check pathlen if not self issued
678678
if (i > 1 && !(x->ex_flags & EXFLAG_SI) && x->ex_pathlen != -1 &&
679-
plen > x->ex_pathlen + 1) {
679+
plen - 1 > x->ex_pathlen) {
680680
ctx->error = X509_V_ERR_PATH_LENGTH_EXCEEDED;
681681
ctx->error_depth = i;
682682
ctx->current_cert = x;

ssl/tls13_both.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ bool tls13_process_finished(SSL_HANDSHAKE *hs, const SSLMessage &msg,
374374
if (verify_data.size() > sizeof(ssl->s3->previous_client_finished) ||
375375
verify_data.size() > sizeof(ssl->s3->previous_server_finished)) {
376376
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
377-
return ssl_hs_error;
377+
return false;
378378
}
379379

380380
if (ssl->server) {
@@ -614,7 +614,7 @@ bool tls13_add_finished(SSL_HANDSHAKE *hs) {
614614
if (verify_data_len > sizeof(ssl->s3->previous_client_finished) ||
615615
verify_data_len > sizeof(ssl->s3->previous_server_finished)) {
616616
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
617-
return ssl_hs_error;
617+
return false;
618618
}
619619

620620
if (ssl->server) {

tool-openssl/pass_util.cc

Lines changed: 20 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
#include <openssl/err.h>
66
#include <openssl/evp.h>
77
#include <openssl/pem.h>
8+
#include <cerrno>
9+
#include <climits>
810
#include <cstring>
911
#include <string>
1012
#include "internal.h"
@@ -39,6 +41,11 @@ static Source DetectSource(const Password &source) {
3941
// Helper function to validate password sources and detect same-file case
4042
static bool ValidateSource(Password &passin, Password *passout = nullptr,
4143
bool *same_file = nullptr) {
44+
// Always initialize same_file
45+
if (same_file) {
46+
*same_file = false;
47+
}
48+
4249
// Validate passin format (if not empty)
4350
if (!passin.empty()) {
4451
Source passin_type = DetectSource(passin);
@@ -68,26 +75,14 @@ static bool ValidateSource(Password &passin, Password *passout = nullptr,
6875
}
6976
}
7077

71-
// Initialize same_file to false if not detected
72-
if (same_file && (!passout || passout->empty())) {
73-
*same_file = false;
74-
}
75-
7678
return true;
7779
}
7880

7981
static bool ExtractDirectPassword(Password &source) {
80-
// Check for additional colons in password portion after prefix
81-
if (source.get().find(':', 5) != std::string::npos) {
82-
fprintf(stderr,
83-
"Invalid password format (use pass:, file:, env:, or stdin)\n");
84-
return false;
85-
}
86-
8782
// Check length before modification
88-
if (source.get().length() - 5 > PEM_BUFSIZE) {
83+
if (source.get().length() >= 5 + PEM_BUFSIZE) {
8984
fprintf(stderr, "Password exceeds maximum allowed length (%d bytes)\n",
90-
PEM_BUFSIZE);
85+
PEM_BUFSIZE - 1);
9186
return false;
9287
}
9388

@@ -121,12 +116,13 @@ static bool ExtractPasswordFromStream(Password &source, Source source_type,
121116
fprintf(stderr, "Invalid file descriptor: %s\n", source.get().c_str());
122117
return false;
123118
}
124-
125-
int fd = atoi(source.get().c_str());
126-
if (fd < 0) {
119+
errno = 0;
120+
unsigned long fd_val = strtoul(source.get().c_str(), nullptr, 10);
121+
if (errno != 0 || fd_val > INT_MAX) {
127122
fprintf(stderr, "Invalid file descriptor: %s\n", source.get().c_str());
128123
return false;
129124
}
125+
int fd = static_cast<int>(fd_val);
130126
bio.reset(BIO_new_fd(fd, BIO_NOCLOSE));
131127
#endif
132128
} else {
@@ -189,6 +185,7 @@ static bool ExtractPasswordFromStream(Password &source, Source source_type,
189185
}
190186

191187
target.assign(buf, len);
188+
OPENSSL_cleanse(buf, sizeof(buf));
192189
return true;
193190
};
194191

@@ -201,8 +198,8 @@ static bool ExtractPasswordFromStream(Password &source, Source source_type,
201198
} else {
202199
// Handle skip_first_line if needed
203200
if (skip_first_line) {
204-
std::string dummy;
205-
if (!read_password_line(dummy)) {
201+
Password dummy;
202+
if (!read_password_line(dummy.get())) {
206203
return false;
207204
}
208205
}
@@ -213,7 +210,6 @@ static bool ExtractPasswordFromStream(Password &source, Source source_type,
213210
}
214211
}
215212

216-
OPENSSL_cleanse(buf, sizeof(buf));
217213
return true;
218214
}
219215

@@ -239,14 +235,14 @@ static bool ExtractPasswordFromEnv(Password &source) {
239235
source.get().c_str());
240236
return false;
241237
}
242-
if (env_val_len > PEM_BUFSIZE) {
238+
if (env_val_len >= PEM_BUFSIZE) {
243239
fprintf(stderr, "Environment variable value too long (maximum %d bytes)\n",
244-
PEM_BUFSIZE);
240+
PEM_BUFSIZE - 1);
245241
return false;
246242
}
247243

248244
// Replace source content with environment value
249-
source.get() = std::string(env_val);
245+
source.get().assign(env_val, env_val_len);
250246
return true;
251247
}
252248

@@ -305,7 +301,7 @@ bool ExtractPasswords(Password &passin, Password &passout) {
305301
// Handle same_file case with single extraction call
306302
if (same_file && !passin.empty() && !passout.empty()) {
307303
Source source_type = DetectSource(passin);
308-
return ExtractPasswordFromSource(passin, source_type, same_file, &passout);
304+
return ExtractPasswordFromSource(passin, source_type, false, &passout);
309305
}
310306

311307
// Extract passin (always from first line)

tool-openssl/pass_util_test.cc

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,10 +236,20 @@ TEST_F(PassUtilTest, DirectPasswordEdgeCases) {
236236
EXPECT_TRUE(result) << "Should succeed with empty direct password";
237237
EXPECT_TRUE(source.empty()) << "Password should be empty";
238238

239+
// Test password containing colons
240+
source = Password("pass:test:123");
241+
result = pass_util::ExtractPassword(source);
242+
EXPECT_TRUE(result) << "Should succeed with colons in password";
243+
EXPECT_EQ(source.get(), "test:123") << "Password with colons should be preserved";
244+
245+
source = Password("pass:a:b:c:d");
246+
result = pass_util::ExtractPassword(source);
247+
EXPECT_TRUE(result) << "Should succeed with multiple colons in password";
248+
EXPECT_EQ(source.get(), "a:b:c:d") << "All colons in password should be preserved";
249+
239250
// Test invalid format strings
240251
const char *invalid_formats[] = {
241252
"pass", // Missing colon
242-
"pass:test:123", // Multiple colons
243253
":password", // Missing prefix
244254
"invalid:pass", // Invalid prefix
245255
"file:", // Empty file path

0 commit comments

Comments
 (0)