Check sk_push return value in openssl(1) pkcs12
This adds return value check for sk_X509_push.
In error case, allocated memories are freed at 'export_end:'.
After this diff, I would like to add more another return value check.
ok?
Index: pkcs12.c
===
RCS file: /cvs/src/usr.bin/openssl/pkcs12.c,v
retrieving revision 1.20
diff -u -p -u -p -r1.20 pkcs12.c
--- pkcs12.c28 Apr 2022 15:42:10 - 1.20
+++ pkcs12.c28 Apr 2022 18:59:39 -
@@ -613,6 +613,9 @@ pkcs12_main(int argc, char **argv)
EVP_PKEY *key = NULL;
X509 *ucert = NULL, *x = NULL;
STACK_OF(X509) *certs = NULL;
+ STACK_OF(X509) *morecerts = NULL;
+ STACK_OF(X509) *chain2 = NULL;
+ X509_STORE *store = NULL;
const EVP_MD *macmd = NULL;
unsigned char *catmp = NULL;
int i;
@@ -664,23 +667,24 @@ pkcs12_main(int argc, char **argv)
/* Add any more certificates asked for */
if (pkcs12_config.certfile != NULL) {
- STACK_OF(X509) *morecerts = NULL;
if ((morecerts = load_certs(bio_err,
pkcs12_config.certfile, FORMAT_PEM, NULL,
"certificates from certfile")) == NULL)
goto export_end;
- while (sk_X509_num(morecerts) > 0)
- sk_X509_push(certs, sk_X509_shift(morecerts));
+ while (sk_X509_num(morecerts) > 0) {
+ if (!sk_X509_push(certs,
sk_X509_shift(morecerts)))
+ goto export_end;
+ }
sk_X509_free(morecerts);
+ morecerts = NULL;
}
/* If chaining get chain from user cert */
if (pkcs12_config.chain) {
int vret;
- STACK_OF(X509) *chain2;
- X509_STORE *store = X509_STORE_new();
- if (store == NULL) {
+
+ if ((store = X509_STORE_new()) == NULL) {
BIO_printf(bio_err,
"Memory allocation error\n");
goto export_end;
@@ -691,15 +695,19 @@ pkcs12_main(int argc, char **argv)
vret = get_cert_chain(ucert, store, );
X509_STORE_free(store);
+ store = NULL;
if (vret == X509_V_OK) {
/* Exclude verified certificate */
- for (i = 1; i < sk_X509_num(chain2); i++)
- sk_X509_push(certs, sk_X509_value(
- chain2, i));
+ for (i = 1; i < sk_X509_num(chain2); i++) {
+ if (!sk_X509_push(certs, sk_X509_value(
+ chain2, i)))
+ goto export_end;
+ }
/* Free first certificate */
X509_free(sk_X509_value(chain2, 0));
sk_X509_free(chain2);
+ chain2 = NULL;
} else {
if (vret != X509_V_ERR_UNSPECIFIED)
BIO_printf(bio_err,
@@ -765,8 +773,11 @@ pkcs12_main(int argc, char **argv)
export_end:
EVP_PKEY_free(key);
- sk_X509_pop_free(certs, X509_free);
X509_free(ucert);
+ sk_X509_pop_free(certs, X509_free);
+ sk_X509_pop_free(morecerts, X509_free);
+ sk_X509_pop_free(chain2, X509_free);
+ X509_STORE_free(store);
goto end;
Re: Compare pointer value with NULL in openssl(1) pkcs12
On Sat, Apr 23, 2022 at 08:31:50AM +0200, Theo Buehler wrote:
> On Sat, Apr 23, 2022 at 01:45:12PM +0900, Kinichiro Inoguchi wrote:
>
> > I would like to do some clean up for openssl(1) pkcs12.
> > This diff changes pointer value checking to explicit comparison with NULL,
> > and no functional changes here.
> > This works fine for me on my local pc.
> >
> > ok?
>
> ok.
Thanks for checking this.
>
> Two comments for a next diff below.
>
> >
> > Index: pkcs12.c
> > ===
> > RCS file: /cvs/src/usr.bin/openssl/pkcs12.c,v
> > retrieving revision 1.18
> > diff -u -p -u -p -r1.18 pkcs12.c
> > --- pkcs12.c28 Mar 2022 11:02:49 - 1.18
> > +++ pkcs12.c23 Apr 2022 04:08:55 -
> > @@ -556,7 +556,7 @@ pkcs12_main(int argc, char **argv)
> > goto end;
> > }
> >
> > - if (pkcs12_config.passarg) {
> > + if (pkcs12_config.passarg != NULL) {
> > if (pkcs12_config.export_cert)
> > pkcs12_config.passargout = pkcs12_config.passarg;
> > else
> > @@ -567,13 +567,13 @@ pkcs12_main(int argc, char **argv)
> > BIO_printf(bio_err, "Error getting passwords\n");
> > goto end;
> > }
> > - if (!cpass) {
> > + if (cpass == NULL) {
> > if (pkcs12_config.export_cert)
> > cpass = passout;
> > else
> > cpass = passin;
> > }
> > - if (cpass) {
> > + if (cpass != NULL) {
> > mpass = cpass;
> > pkcs12_config.noprompt = 1;
> > } else {
> > @@ -581,22 +581,22 @@ pkcs12_main(int argc, char **argv)
> > mpass = macpass;
> > }
> >
> > - if (!pkcs12_config.infile)
> > + if (pkcs12_config.infile == NULL)
> > in = BIO_new_fp(stdin, BIO_NOCLOSE);
> > else
> > in = BIO_new_file(pkcs12_config.infile, "rb");
> > - if (!in) {
> > + if (in == NULL) {
> > BIO_printf(bio_err, "Error opening input file %s\n",
> > pkcs12_config.infile ? pkcs12_config.infile : "");
> > perror(pkcs12_config.infile);
> > goto end;
> > }
> >
> > - if (!pkcs12_config.outfile) {
> > + if (pkcs12_config.outfile == NULL) {
> > out = BIO_new_fp(stdout, BIO_NOCLOSE);
> > } else
> > out = BIO_new_file(pkcs12_config.outfile, "wb");
> > - if (!out) {
> > + if (out == NULL) {
> > BIO_printf(bio_err, "Error opening output file %s\n",
> > pkcs12_config.outfile ? pkcs12_config.outfile : "");
> > perror(pkcs12_config.outfile);
> > @@ -637,10 +637,10 @@ pkcs12_main(int argc, char **argv)
> > if (!(pkcs12_config.options & NOCERTS)) {
> > certs = load_certs(bio_err, pkcs12_config.infile,
> > FORMAT_PEM, NULL, "certificates");
> > - if (!certs)
> > + if (certs == NULL)
> > goto export_end;
> >
> > - if (key) {
> > + if (key != NULL) {
> > /* Look for matching private key */
> > for (i = 0; i < sk_X509_num(certs); i++) {
> > x = sk_X509_value(certs, i);
> > @@ -654,7 +654,7 @@ pkcs12_main(int argc, char **argv)
> > break;
> > }
> > }
> > - if (!ucert) {
> > + if (ucert == NULL) {
> > BIO_printf(bio_err,
> > "No certificate matches private
> > key\n");
> > goto export_end;
> > @@ -663,11 +663,11 @@ pkcs12_main(int argc, char **argv)
> > }
> >
> > /* Add any more certificates asked for */
> > - if (pkcs12_config.certfile) {
> > + if (pkcs12_config.certfile != NULL) {
> > STACK_OF(X509) *morecerts = NULL;
> > - if (!(morecerts = load_certs(bio_err,
> > + if ((morecerts = load_certs(bio_err,
> >
Compare pointer value with NULL in openssl(1) pkcs12
I would like to do some clean up for openssl(1) pkcs12.
This diff changes pointer value checking to explicit comparison with NULL,
and no functional changes here.
This works fine for me on my local pc.
ok?
Index: pkcs12.c
===
RCS file: /cvs/src/usr.bin/openssl/pkcs12.c,v
retrieving revision 1.18
diff -u -p -u -p -r1.18 pkcs12.c
--- pkcs12.c28 Mar 2022 11:02:49 - 1.18
+++ pkcs12.c23 Apr 2022 04:08:55 -
@@ -556,7 +556,7 @@ pkcs12_main(int argc, char **argv)
goto end;
}
- if (pkcs12_config.passarg) {
+ if (pkcs12_config.passarg != NULL) {
if (pkcs12_config.export_cert)
pkcs12_config.passargout = pkcs12_config.passarg;
else
@@ -567,13 +567,13 @@ pkcs12_main(int argc, char **argv)
BIO_printf(bio_err, "Error getting passwords\n");
goto end;
}
- if (!cpass) {
+ if (cpass == NULL) {
if (pkcs12_config.export_cert)
cpass = passout;
else
cpass = passin;
}
- if (cpass) {
+ if (cpass != NULL) {
mpass = cpass;
pkcs12_config.noprompt = 1;
} else {
@@ -581,22 +581,22 @@ pkcs12_main(int argc, char **argv)
mpass = macpass;
}
- if (!pkcs12_config.infile)
+ if (pkcs12_config.infile == NULL)
in = BIO_new_fp(stdin, BIO_NOCLOSE);
else
in = BIO_new_file(pkcs12_config.infile, "rb");
- if (!in) {
+ if (in == NULL) {
BIO_printf(bio_err, "Error opening input file %s\n",
pkcs12_config.infile ? pkcs12_config.infile : "");
perror(pkcs12_config.infile);
goto end;
}
- if (!pkcs12_config.outfile) {
+ if (pkcs12_config.outfile == NULL) {
out = BIO_new_fp(stdout, BIO_NOCLOSE);
} else
out = BIO_new_file(pkcs12_config.outfile, "wb");
- if (!out) {
+ if (out == NULL) {
BIO_printf(bio_err, "Error opening output file %s\n",
pkcs12_config.outfile ? pkcs12_config.outfile : "");
perror(pkcs12_config.outfile);
@@ -637,10 +637,10 @@ pkcs12_main(int argc, char **argv)
if (!(pkcs12_config.options & NOCERTS)) {
certs = load_certs(bio_err, pkcs12_config.infile,
FORMAT_PEM, NULL, "certificates");
- if (!certs)
+ if (certs == NULL)
goto export_end;
- if (key) {
+ if (key != NULL) {
/* Look for matching private key */
for (i = 0; i < sk_X509_num(certs); i++) {
x = sk_X509_value(certs, i);
@@ -654,7 +654,7 @@ pkcs12_main(int argc, char **argv)
break;
}
}
- if (!ucert) {
+ if (ucert == NULL) {
BIO_printf(bio_err,
"No certificate matches private
key\n");
goto export_end;
@@ -663,11 +663,11 @@ pkcs12_main(int argc, char **argv)
}
/* Add any more certificates asked for */
- if (pkcs12_config.certfile) {
+ if (pkcs12_config.certfile != NULL) {
STACK_OF(X509) *morecerts = NULL;
- if (!(morecerts = load_certs(bio_err,
+ if ((morecerts = load_certs(bio_err,
pkcs12_config.certfile, FORMAT_PEM, NULL,
- "certificates from certfile")))
+ "certificates from certfile")) == NULL)
goto export_end;
while (sk_X509_num(morecerts) > 0)
sk_X509_push(certs, sk_X509_shift(morecerts));
@@ -680,7 +680,7 @@ pkcs12_main(int argc, char **argv)
int vret;
STACK_OF(X509) *chain2;
X509_STORE *store = X509_STORE_new();
- if (!store) {
+ if (store == NULL) {
BIO_printf(bio_err,
"Memory allocation error\n");
goto export_end;
@@ -720,12 +720,12 @@ pkcs12_main(int argc, char **argv)
X509_alias_set1(sk_X509_value(certs, i), catmp, -1);
}
- if (pkcs12_config.csp_name && key)
+ if
Re: openssl(1): implement naccept
This builds fine and works good.
ok inoguchi@
I have one comment additionally what jmc@ mentioned.
On Sun, Aug 29, 2021 at 01:10:56PM +0100, Jason McIntyre wrote:
> On Sun, Aug 29, 2021 at 02:00:44PM +0200, Theo Buehler wrote:
> > Terminate the s_server after n clients connected to it. This is
> > occasionally useful, matches OpenSSL's behavior and should help
> > simplifying regress/usr.bin/openssl/x509.
> >
>
> hi.
>
> > Index: openssl.1
> > ===
> > RCS file: /cvs/src/usr.bin/openssl/openssl.1,v
> > retrieving revision 1.129
> > diff -u -p -r1.129 openssl.1
> > --- openssl.1 17 Mar 2021 18:08:32 - 1.129
> > +++ openssl.1 28 Aug 2021 17:12:59 -
> > @@ -4607,6 +4607,7 @@ will be used.
> > .Op Fl keymatexportlen Ar len
> > .Op Fl msg
> > .Op Fl mtu Ar mtu
> > +.Op Fl naccept Ar arg
>
> i guess "arg" should be "num".
> also i think it needs to be added to sv_usage in s_server.c.
>
> otherwise doc parts ok.
>
> jmc
>
> > .Op Fl named_curve Ar arg
> > .Op Fl nbio
> > .Op Fl nbio_test
> > @@ -4807,6 +4808,10 @@ Export len bytes of keying material (def
> > Show all protocol messages with hex dump.
> > .It Fl mtu Ar mtu
> > Set the link layer MTU.
> > +.It Fl naccept Ar num
> > +Terminate server after
> > +.Ar num
> > +connections.
> > .It Fl named_curve Ar arg
> > Specify the elliptic curve name to use for ephemeral ECDH keys.
> > This option is deprecated; use
> > Index: s_apps.h
> > ===
> > RCS file: /cvs/src/usr.bin/openssl/s_apps.h,v
> > retrieving revision 1.5
> > diff -u -p -r1.5 s_apps.h
> > --- s_apps.h25 Apr 2018 07:12:33 - 1.5
> > +++ s_apps.h28 Aug 2021 17:12:59 -
> > @@ -120,7 +120,7 @@ extern int verify_return_error;
> >
> > int do_server(int port, int type, int *ret,
> > int (*cb)(char *hostname, int s, unsigned char *context),
> > -unsigned char *context);
> > +unsigned char *context, int naccept);
> > #ifdef HEADER_X509_H
> > int verify_callback(int ok, X509_STORE_CTX *ctx);
> > #endif
> > Index: s_server.c
> > ===
> > RCS file: /cvs/src/usr.bin/openssl/s_server.c,v
> > retrieving revision 1.47
> > diff -u -p -r1.47 s_server.c
> > --- s_server.c 17 Mar 2021 18:11:01 - 1.47
> > +++ s_server.c 28 Aug 2021 17:17:38 -
> > @@ -267,6 +267,7 @@ static struct {
> > uint16_t min_version;
> > const SSL_METHOD *meth;
> > int msg;
> > + int naccept;
> > char *named_curve;
> > int nbio;
> > int nbio_test;
> > @@ -741,6 +742,13 @@ static const struct option s_server_opti
> > },
> > #endif
> > {
> > + .name = "naccept",
> > + .argname = "num",
> > + .desc = "terminate after num connections",
Other .desc begins with capital letter, so "Terminate" would be better.
> > + .type = OPTION_ARG_INT,
> > + .opt.value = _server_config.naccept
> > + },
> > + {
> > .name = "named_curve",
> > .argname = "arg",
> > .type = OPTION_ARG,
> > @@ -1084,6 +1092,7 @@ s_server_main(int argc, char *argv[])
> > memset(_server_config, 0, sizeof(s_server_config));
> > s_server_config.keymatexportlen = 20;
> > s_server_config.meth = TLS_server_method();
> > + s_server_config.naccept = -1;
> > s_server_config.port = PORT;
> > s_server_config.cert_file = TEST_CERT;
> > s_server_config.cert_file2 = TEST_CERT2;
> > @@ -1465,10 +1474,12 @@ s_server_main(int argc, char *argv[])
> > (void) BIO_flush(bio_s_out);
> > if (s_server_config.www)
> > do_server(s_server_config.port, s_server_config.socket_type,
> > - _socket, www_body, s_server_config.context);
> > + _socket, www_body, s_server_config.context,
> > + s_server_config.naccept);
> > else
> > do_server(s_server_config.port, s_server_config.socket_type,
> > - _socket, sv_body, s_server_config.context);
> > + _socket, sv_body, s_server_config.context,
> > + s_server_config.naccept);
> > print_stats(bio_s_out, ctx);
> > ret = 0;
> > end:
> > Index: s_socket.c
> > ===
> > RCS file: /cvs/src/usr.bin/openssl/s_socket.c,v
> > retrieving revision 1.11
> > diff -u -p -r1.11 s_socket.c
> > --- s_socket.c 28 Jun 2019 13:35:02 - 1.11
> > +++ s_socket.c 28 Aug 2021 17:12:59 -
> > @@ -132,7 +132,7 @@ init_client(int *sock, char *host, char
> > int
> > do_server(int port, int type, int *ret,
> > int (*cb) (char *hostname, int s, unsigned char *context),
> > -unsigned char *context)
> > +unsigned char *context, int naccept)
> > {
> > int sock;
> > char *name = NULL;
> > @@ -161,7 +161,9 @@ do_server(int port, int
Re: Update Windows getentropy implementation
On Mon, Nov 09, 2020 at 01:10:51PM -0600, Brent Cook wrote:
>
> This updates the getentropy implementation for Windows to use the newer
> "Cryptography Next Generation APIs", replacing CryptGenRandom, which
> already has been removed from applications built for the Windows Store.
>
> Tested with libressl-portable, it passes all regression tests. Details
> of the API are in the comment link below. Noted by Stephan Vedder
> (feliwir on github) and others.
>
> Any objections to gettin this in?
>
> diff --git a/src/lib/libcrypto/arc4random/getentropy_win.c
> b/src/lib/libcrypto/arc4random/getentropy_win.c
> index 2abeb27bc..0a014f3b0 100644
> --- a/src/lib/libcrypto/arc4random/getentropy_win.c
> +++ b/src/lib/libcrypto/arc4random/getentropy_win.c
> @@ -21,39 +21,30 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> -#include
> -#include
>
> int getentropy(void *buf, size_t len);
>
> /*
> - * On Windows, CryptGenRandom is supposed to be a well-seeded
> - * cryptographically strong random number generator.
> + * On Windows, BCryptGenRandom with BCRYPT_USE_SYSTEM_PREFERRED_RNG is
> supposed
> + * to be a well-seeded, cryptographically strong random number generator.
> + *
> https://docs.microsoft.com/en-us/windows/win32/api/bcrypt/nf-bcrypt-bcryptgenrandom
> */
> int
> getentropy(void *buf, size_t len)
> {
> - HCRYPTPROV provider;
> -
> if (len > 256) {
> errno = EIO;
> return (-1);
> }
>
> - if (CryptAcquireContext(, NULL, NULL, PROV_RSA_FULL,
> - CRYPT_VERIFYCONTEXT) == 0)
> - goto fail;
> - if (CryptGenRandom(provider, len, buf) == 0) {
> - CryptReleaseContext(provider, 0);
> - goto fail;
> + if (FAILED(BCryptGenRandom(NULL, buf, len,
> BCRYPT_USE_SYSTEM_PREFERRED_RNG))) {
> + errno = EIO;
> + return (-1);
> }
> - CryptReleaseContext(provider, 0);
> +
> return (0);
> -
> -fail:
> - errno = EIO;
> - return (-1);
> }
>
I had tested this diff on Windows10 with Visual Studio 2019.
It built both shared and static successfully with adding bcrypt
to PLATFORM_LIBS, and I confirmed that all regress has passed.
No objection and ok inoguchi@
Re: openssl(1) x509, change in serial number output between 6.5 and 6.6
On Thu, Apr 09, 2020 at 07:44:51PM +0100, Stuart Henderson wrote:
> On 2020/04/09 20:13, Theo Buehler wrote:
> > On Thu, Apr 09, 2020 at 05:56:55PM +0100, Stuart Henderson wrote:
> > > Not new - this happened somewhere between 6.5 and 6.6 - but some
> > > certificates are now showing up with bad serial numbers in "openssl x509".
> > > Example below, a few other certs are affected.
> > >
> > > From the current /etc/ssl/cert.pem, these are the ones showing the same:
> > >
> > > -Serial Number: 11806822484801597146 (0xa3da427ea4b1aeda)
> > > -Serial Number: 14541511773111788494 (0xc9cdd3e9d57d23ce)
> > > -Serial Number: 18364802974209362175 (0xfedce3010fc948ff)
> > > -Serial Number: 10572350602393338211 (0x92b888dbb08ac163)
> > > -Serial Number: 14014712776195784473 (0xc27e43044e473f19)
> > > -Serial Number: 13492815561806991280 (0xbb401c43f55e4fb0)
> > > -Serial Number: 9548242946988625984 (0x84822c5f1c62d040)
> > > -Serial Number: 15752444095811006489 (0xda9bec71f303b019)
> >
> > This is due to r1.34 of src/lib/libcrypto/asn1/a_int.c which changed
> > ASN1_INTEGER_get() to avoid undefined behavior. It now returns -1 more
> > often. Note that your examples are all larger than LONG_MAX.
> >
> > Minimal fix for this issue is to use the fallback to colon separated hex
> > digits in X509_print_ex() in case ASN1_INTEGER_get() returns an error so
> > that your example cert yields:
> >
> > Serial Number:
> > da:9b:ec:71:f3:03:b0:19
>
> Aha - this matches what OpenSSL does. I'm OK with this.
>
> > Index: asn1/t_x509.c
> > ===
> > RCS file: /var/cvs/src/lib/libcrypto/asn1/t_x509.c,v
> > retrieving revision 1.31
> > diff -u -p -r1.31 t_x509.c
> > --- asn1/t_x509.c 18 May 2018 18:23:24 - 1.31
> > +++ asn1/t_x509.c 9 Apr 2020 17:53:51 -
> > @@ -145,8 +145,10 @@ X509_print_ex(BIO *bp, X509 *x, unsigned
> > goto err;
> >
> > bs = X509_get_serialNumber(x);
> > - if (bs->length <= (int)sizeof(long)) {
> > + l = -1;
> > + if (bs->length <= (int)sizeof(long))
> > l = ASN1_INTEGER_get(bs);
> > + if (l != -1) {
> > if (bs->type == V_ASN1_NEG_INTEGER) {
> > l = -l;
> > neg = "-";
> >
I'm fine with this fix, but 1 comment.
How about doing below rather than assign -1 to 'l' for readability ?
Index: t_x509.c
===
RCS file: /cvs/src/lib/libcrypto/asn1/t_x509.c,v
retrieving revision 1.31
diff -u -p -u -p -r1.31 t_x509.c
--- t_x509.c18 May 2018 18:23:24 - 1.31
+++ t_x509.c10 Apr 2020 07:08:43 -
@@ -145,8 +145,8 @@ X509_print_ex(BIO *bp, X509 *x, unsigned
goto err;
bs = X509_get_serialNumber(x);
- if (bs->length <= (int)sizeof(long)) {
- l = ASN1_INTEGER_get(bs);
+ if (bs->length <= (int)sizeof(long) &&
+ (l = ASN1_INTEGER_get(bs)) != -1) {
if (bs->type == V_ASN1_NEG_INTEGER) {
l = -l;
neg = "-";
Re: [PATCH] gostr341001: support unwrapped private keys support
> There is no English specification for GOST PKCS8 files yet,
> unfortunately. You can find similar pieces of code in OpenSSL's GOST
> engine (https://github.com/gost-engine/engine/blob/master/gost_ameth.c#L347)
> and in GnuTLS
> (https://gitlab.com/gnutls/gnutls/-/blob/master/lib/x509/privkey_pkcs8.c#L1159).
I checked GOST engine one and I saw the similar implementation was there.
This is just a question and not request, though, don't you need
"V_ASN1_SEQUENCE | V_ASN1_CONSTRUCTED" case for now, since GOST engine has it.
I would like to suggest two return value checks, for BN_mod_mul and
unmask_priv_key, details are below.
---
src/lib/libcrypto/gost/gostr341001_ameth.c | 75 --
1 file changed, 70 insertions(+), 5 deletions(-)
diff --git a/src/lib/libcrypto/gost/gostr341001_ameth.c
b/src/lib/libcrypto/gost/gostr341001_ameth.c
index 0f816377dde1..70bd3357f184 100644
--- a/src/lib/libcrypto/gost/gostr341001_ameth.c
+++ b/src/lib/libcrypto/gost/gostr341001_ameth.c
@@ -437,6 +437,56 @@ priv_print_gost01(BIO *out, const EVP_PKEY *pkey, int
indent, ASN1_PCTX *pctx)
return pub_print_gost01(out, pkey, indent, pctx);
}
+static BIGNUM *unmask_priv_key(EVP_PKEY *pk,
+ const unsigned char *buf, int len, int num_masks)
+{
+ BIGNUM *pknum_masked = NULL, *q = NULL;
+ const GOST_KEY *key_ptr = pk->pkey.gost;
+ const EC_GROUP *group = GOST_KEY_get0_group(key_ptr);
+
+ pknum_masked = GOST_le2bn(buf, len, NULL);
+ if (!pknum_masked) {
+ GOSTerror(ERR_R_MALLOC_FAILURE);
+ return NULL;
+ }
+
+ if (num_masks > 0) {
+ /*
+* XXX Remove sign by gost94
+*/
+ const unsigned char *p = buf + num_masks * len;
+
+ q = BN_new();
+ if (!q) {
+ GOSTerror(ERR_R_MALLOC_FAILURE);
+ BN_free(pknum_masked);
+ pknum_masked = NULL;
+ goto end;
+ }
+ if (EC_GROUP_get_order(group, q, NULL) <= 0) {
+ GOSTerror(ERR_R_EC_LIB);
+ BN_free(pknum_masked);
+ pknum_masked = NULL;
+ goto end;
+ }
+
+ for (; p != buf; p -= len) {
+ BIGNUM *mask = GOST_le2bn(p, len, NULL);
+ BN_CTX *ctx = BN_CTX_new();
+
+ BN_mod_mul(pknum_masked, pknum_masked, mask, q, ctx);
BN_mod_mul might fail and return 0 on error.
I would like to suggest checking this.
+
+ BN_CTX_free(ctx);
+ BN_free(mask);
+ }
+ }
+
+end:
+ if (q)
+ BN_free(q);
+ return pknum_masked;
+}
+
static int
priv_decode_gost01(EVP_PKEY *pk, const PKCS8_PRIV_KEY_INFO *p8inf)
{
@@ -450,6 +500,7 @@ priv_decode_gost01(EVP_PKEY *pk, const PKCS8_PRIV_KEY_INFO
*p8inf)
GOST_KEY *ec;
int ptype = V_ASN1_UNDEF;
ASN1_STRING *pval = NULL;
+ int expected_key_len;
if (PKCS8_pkey_get0(_obj, _buf, _len, , p8inf) ==
0) {
GOSTerror(GOST_R_BAD_KEY_PARAMETERS_FORMAT);
@@ -467,29 +518,43 @@ priv_decode_gost01(EVP_PKEY *pk, const
PKCS8_PRIV_KEY_INFO *p8inf)
return 0;
}
p = pkey_buf;
- if (V_ASN1_OCTET_STRING == *p) {
+
+ expected_key_len = (pkey_bits_gost01(pk) + 7) / 8;
+ if (expected_key_len == 0) {
+ EVPerror(EVP_R_DECODE_ERROR);
+ return 0;
+ } else if (priv_len % expected_key_len == 0) {
+ /* Key is not wrapped but masked */
+ pk_num = unmask_priv_key(pk, pkey_buf, expected_key_len,
unmask_priv_key returns NULL on error.
I would like to suggest checking this.
+ priv_len / expected_key_len - 1);
+ } else if (V_ASN1_OCTET_STRING == *p) {
/* New format - Little endian octet string */
ASN1_OCTET_STRING *s =
d2i_ASN1_OCTET_STRING(NULL, , priv_len);
if (s == NULL) {
- GOSTerror(EVP_R_DECODE_ERROR);
+ EVPerror(EVP_R_DECODE_ERROR);
ASN1_STRING_free(s);
return 0;
}
pk_num = GOST_le2bn(s->data, s->length, NULL);
ASN1_STRING_free(s);
- } else {
+ } else if (V_ASN1_INTEGER == *p) {
priv_key = d2i_ASN1_INTEGER(NULL, , priv_len);
- if (priv_key == NULL)
+ if (priv_key == NULL) {
+ EVPerror(EVP_R_DECODE_ERROR);
return 0;
+ }
ret = ((pk_num = ASN1_INTEGER_to_BN(priv_key, NULL)) != NULL);
ASN1_INTEGER_free(priv_key);
if (ret == 0) {
-
Re: [PATCH] gostr341001: support unwrapped private keys support
Hi, Where can we see the specifcation for these 3 different format, wrapped in OCTET STRING, INTEGER and unwrapped but masked ? I tried to find but couldn't.
Re: [PATCH v3 2/2] gost: populate params tables with new curves
Confirmed that portable build and regresses succeeded.
I'm ok with this patch.
On Sun, Mar 29, 2020 at 02:48:05PM +0300, Dmitry Baryshkov wrote:
> Allow users to specify new curves via strings.
>
> Sponsored by ROSA Linux
>
> Signed-off-by: Dmitry Baryshkov
> ---
> src/lib/libcrypto/gost/gostr341001_params.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/src/lib/libcrypto/gost/gostr341001_params.c
> b/src/lib/libcrypto/gost/gostr341001_params.c
> index 13054cd0fc26..138860dee56e 100644
> --- a/src/lib/libcrypto/gost/gostr341001_params.c
> +++ b/src/lib/libcrypto/gost/gostr341001_params.c
> @@ -94,12 +94,22 @@ static const GostR3410_params GostR3410_256_params[] = {
> { "0", NID_id_GostR3410_2001_TestParamSet },
> { "XA", NID_id_GostR3410_2001_CryptoPro_XchA_ParamSet },
> { "XB", NID_id_GostR3410_2001_CryptoPro_XchB_ParamSet },
> + { "TCA", NID_id_tc26_gost_3410_12_256_paramSetA },
> + { "TCB", NID_id_tc26_gost_3410_12_256_paramSetB },
> + { "TCC", NID_id_tc26_gost_3410_12_256_paramSetC },
> + { "TCD", NID_id_tc26_gost_3410_12_256_paramSetD },
> { NULL, NID_undef },
> };
>
> static const GostR3410_params GostR3410_512_params[] = {
> { "A", NID_id_tc26_gost_3410_12_512_paramSetA },
> { "B", NID_id_tc26_gost_3410_12_512_paramSetB },
> + { "C", NID_id_tc26_gost_3410_12_512_paramSetC },
> + { "0", NID_id_tc26_gost_3410_12_512_paramSetTest},
> + /* Duplicates for compatibility with OpenSSL */
> + { "TCA", NID_id_tc26_gost_3410_12_512_paramSetA },
> + { "TCB", NID_id_tc26_gost_3410_12_512_paramSetB },
> + { "TCC", NID_id_tc26_gost_3410_12_512_paramSetC },
> { NULL, NID_undef },
> };
>
> --
> 2.25.1
>
Re: [PATCH v2 2/2] gost: populate params tables with new curves
Hi,
I have 2 questions.
In GostR3410_512_params[], "A" and "TCA" have the same NID, "B" and "TCB" too.
I thought these were redundant, but are there any reasons for this ?
In GostR3410_512_params[], don't you need the record for
NID_id_tc26_gost_3410_12_512_paramSetTest ?
Best regards,
On Sat, Mar 28, 2020 at 07:16:14PM +0300, Dmitry Baryshkov wrote:
> Allow users to specify new curves via strings.
>
> Sponsored by ROSA Linux
>
> Signed-off-by: Dmitry Baryshkov
> ---
> src/lib/libcrypto/gost/gostr341001_params.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/src/lib/libcrypto/gost/gostr341001_params.c
> b/src/lib/libcrypto/gost/gostr341001_params.c
> index 13054cd0fc26..0f068d97eb0a 100644
> --- a/src/lib/libcrypto/gost/gostr341001_params.c
> +++ b/src/lib/libcrypto/gost/gostr341001_params.c
> @@ -94,12 +94,19 @@ static const GostR3410_params GostR3410_256_params[] = {
> { "0", NID_id_GostR3410_2001_TestParamSet },
> { "XA", NID_id_GostR3410_2001_CryptoPro_XchA_ParamSet },
> { "XB", NID_id_GostR3410_2001_CryptoPro_XchB_ParamSet },
> + { "TCA", NID_id_tc26_gost_3410_12_256_paramSetA },
> + { "TCB", NID_id_tc26_gost_3410_12_256_paramSetB },
> + { "TCC", NID_id_tc26_gost_3410_12_256_paramSetC },
> + { "TCD", NID_id_tc26_gost_3410_12_256_paramSetD },
> { NULL, NID_undef },
> };
>
> static const GostR3410_params GostR3410_512_params[] = {
> { "A", NID_id_tc26_gost_3410_12_512_paramSetA },
> { "B", NID_id_tc26_gost_3410_12_512_paramSetB },
> + { "TCA", NID_id_tc26_gost_3410_12_512_paramSetA },
> + { "TCB", NID_id_tc26_gost_3410_12_512_paramSetB },
> + { "TCC", NID_id_tc26_gost_3410_12_512_paramSetC },
> { NULL, NID_undef },
> };
>
> --
> 2.25.1
>
Re: [PATCH v2 1/2] ec: add support for several more GOST curves
Verified added curve parameters _EC_GOST_2012_256_TC26_A and
_EC_GOST_2012_512_TC26_C are equivalent to the definition of
https://tools.ietf.org/html/rfc7836#appendix-A.2 .
Verified added curve parameter _EC_GOST_2012_512_Test is equivalent to
https://tools.ietf.org/html/draft-deremin-rfc4491-bis-04#appendix-D .
I had confirmed that portable build and regresses succeeded.
I'm ok with this diff.
On Sat, Mar 28, 2020 at 07:16:13PM +0300, Dmitry Baryshkov wrote:
> Add support for GOST curves defined by RFC 7836 and
> draft-deremin-rfc4491-bis. Add aliases for 256-bit GOST curves (see
> draft-smyshlyaev-tls12-gost-suites). 512-bit curve ids were renamed to
> follow names defined in tc26 OID registry.
> (https://tc26.ru/about/protsedury-i-reglamenty/identifikatory-obektov-oid-tekhnicheskogo-komiteta-po-standartizatsii-kriptograficheskaya-zashchita-1.html)
>
> Sponsored by ROSA Linux.
>
> Signed-off-by: Dmitry Baryshkov
> ---
> src/lib/libcrypto/ec/ec_curve.c | 162 +++-
> src/lib/libcrypto/gost/gostr341001_params.c | 4 +-
> src/lib/libcrypto/objects/obj_mac.num | 10 +-
> src/lib/libcrypto/objects/objects.txt | 10 +-
> 4 files changed, 176 insertions(+), 10 deletions(-)
>
> diff --git a/src/lib/libcrypto/ec/ec_curve.c b/src/lib/libcrypto/ec/ec_curve.c
> index e075b1ed3ea5..830bb47b3d0b 100644
> --- a/src/lib/libcrypto/ec/ec_curve.c
> +++ b/src/lib/libcrypto/ec/ec_curve.c
> @@ -2900,11 +2900,103 @@ static const struct {
> }
> };
>
> +/* This curve is defined in two birationally equal forms: canonical and
> Twisted
> + * Edwards. We do calculations in canonical (Weierstrass) form. */
> +static const struct {
> + EC_CURVE_DATA h;
> + unsigned char data[0 + 32 * 6];
> +}
> + _EC_GOST_2012_256_TC26_A = {
> + {
> + NID_X9_62_prime_field, 0, 32, 4
> + },
> + { /* no seed */
> + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
> /* p */
> + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
> + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
> + 0xFD, 0x97,
> + 0xc2, 0x17, 0x3f, 0x15, 0x13, 0x98, 0x16, 0x73, 0xaf, 0x48,
> /* a */
> + 0x92, 0xc2, 0x30, 0x35, 0xa2, 0x7c, 0xe2, 0x5e, 0x20, 0x13,
> + 0xbf, 0x95, 0xaa, 0x33, 0xb2, 0x2c, 0x65, 0x6f, 0x27, 0x7e,
> + 0x73, 0x35,
> + 0x29, 0x5f, 0x9b, 0xae, 0x74, 0x28, 0xed, 0x9c, 0xcc, 0x20,
> /* b */
> + 0xe7, 0xc3, 0x59, 0xa9, 0xd4, 0x1a, 0x22, 0xfc, 0xcd, 0x91,
> + 0x08, 0xe1, 0x7b, 0xf7, 0xba, 0x93, 0x37, 0xa6, 0xf8, 0xae,
> + 0x95, 0x13,
> + 0x91, 0xe3, 0x84, 0x43, 0xa5, 0xe8, 0x2c, 0x0d, 0x88, 0x09,
> /* x */
> + 0x23, 0x42, 0x57, 0x12, 0xb2, 0xbb, 0x65, 0x8b, 0x91, 0x96,
> + 0x93, 0x2e, 0x02, 0xc7, 0x8b, 0x25, 0x82, 0xfe, 0x74, 0x2d,
> + 0xaa, 0x28,
> + 0x32, 0x87, 0x94, 0x23, 0xab, 0x1a, 0x03, 0x75, 0x89, 0x57,
> /* y */
> + 0x86, 0xc4, 0xbb, 0x46, 0xe9, 0x56, 0x5f, 0xde, 0x0b, 0x53,
> + 0x44, 0x76, 0x67, 0x40, 0xaf, 0x26, 0x8a, 0xdb, 0x32, 0x32,
> + 0x2e, 0x5c,
> + 0x40, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> /* order */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0f, 0xd8, 0xcd, 0xdf,
> + 0xc8, 0x7b, 0x66, 0x35, 0xc1, 0x15, 0xaf, 0x55, 0x6c, 0x36,
> + 0x0c, 0x67,
> + }
> +};
> +
> static const struct {
> EC_CURVE_DATA h;
> unsigned char data[0 + 64 * 6];
> }
> - _EC_GOST_2012_TC26_A = {
> + _EC_GOST_2012_512_Test = {
> + {
> + NID_X9_62_prime_field, 0, 64, 1
> + },
> + { /* no seed */
> + 0x45, 0x31, 0xac, 0xd1, 0xfe, 0x00, 0x23, 0xc7, 0x55, 0x0d,
> /* p */
> + 0x26, 0x7b, 0x6b, 0x2f, 0xee, 0x80, 0x92, 0x2b, 0x14, 0xb2,
> + 0xff, 0xb9, 0x0f, 0x04, 0xd4, 0xeb, 0x7c, 0x09, 0xb5, 0xd2,
> + 0xd1, 0x5d, 0xf1, 0xd8, 0x52, 0x74, 0x1a, 0xf4, 0x70, 0x4a,
> + 0x04, 0x58, 0x04, 0x7e, 0x80, 0xe4, 0x54, 0x6d, 0x35, 0xb8,
> + 0x33, 0x6f, 0xac, 0x22, 0x4d, 0xd8, 0x16, 0x64, 0xbb, 0xf5,
> + 0x28, 0xbe, 0x63, 0x73,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> /* a */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x07,
> + 0x1c, 0xff, 0x08, 0x06, 0xa3, 0x11, 0x16, 0xda, 0x29, 0xd8,
> /* b */
> + 0xcf, 0xa5, 0x4e, 0x57, 0xeb, 0x74,
Re: [PATCH 2/2] gost: use ECerror to report EC errors
I had checked this by portable build and all regresses passed.
I'm ok with this diff.
On Thu, Mar 26, 2020 at 09:28:02PM +0300, dbarysh...@gmail.com wrote:
> From: Dmitry Baryshkov
>
> GOST code uses GOSTerror(EC_R_foo) to report several errors. Use
> ECerror(EC_R_foo) instead to make error messages match error code.
>
> Sponsored by ROSA Linux.
>
> Signed-off-by: Dmitry Baryshkov
> ---
> src/lib/libcrypto/gost/gostr341001_ameth.c | 2 +-
> src/lib/libcrypto/gost/gostr341001_key.c | 14 +++---
> src/lib/libcrypto/gost/gostr341001_pmeth.c | 2 +-
> 3 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/src/lib/libcrypto/gost/gostr341001_ameth.c
> b/src/lib/libcrypto/gost/gostr341001_ameth.c
> index be621d0185dd..28ed55e6992f 100644
> --- a/src/lib/libcrypto/gost/gostr341001_ameth.c
> +++ b/src/lib/libcrypto/gost/gostr341001_ameth.c
> @@ -547,7 +547,7 @@ param_decode_gost01(EVP_PKEY *pkey, const unsigned char
> **pder, int derlen)
> }
> group = EC_GROUP_new_by_curve_name(nid);
> if (group == NULL) {
> - GOSTerror(EC_R_EC_GROUP_NEW_BY_NAME_FAILURE);
> + ECerror(EC_R_EC_GROUP_NEW_BY_NAME_FAILURE);
> GOST_KEY_free(ec);
> return 0;
> }
> diff --git a/src/lib/libcrypto/gost/gostr341001_key.c
> b/src/lib/libcrypto/gost/gostr341001_key.c
> index 0af39f21bf33..74f8cab9d86c 100644
> --- a/src/lib/libcrypto/gost/gostr341001_key.c
> +++ b/src/lib/libcrypto/gost/gostr341001_key.c
> @@ -121,7 +121,7 @@ GOST_KEY_check_key(const GOST_KEY *key)
> return 0;
> }
> if (EC_POINT_is_at_infinity(key->group, key->pub_key) != 0) {
> - GOSTerror(EC_R_POINT_AT_INFINITY);
> + ECerror(EC_R_POINT_AT_INFINITY);
> goto err;
> }
> if ((ctx = BN_CTX_new()) == NULL)
> @@ -131,14 +131,14 @@ GOST_KEY_check_key(const GOST_KEY *key)
>
> /* testing whether the pub_key is on the elliptic curve */
> if (EC_POINT_is_on_curve(key->group, key->pub_key, ctx) == 0) {
> - GOSTerror(EC_R_POINT_IS_NOT_ON_CURVE);
> + ECerror(EC_R_POINT_IS_NOT_ON_CURVE);
> goto err;
> }
> /* testing whether pub_key * order is the point at infinity */
> if ((order = BN_new()) == NULL)
> goto err;
> if (EC_GROUP_get_order(key->group, order, ctx) == 0) {
> - GOSTerror(EC_R_INVALID_GROUP_ORDER);
> + ECerror(EC_R_INVALID_GROUP_ORDER);
> goto err;
> }
> if (EC_POINT_mul(key->group, point, NULL, key->pub_key, order,
> @@ -147,7 +147,7 @@ GOST_KEY_check_key(const GOST_KEY *key)
> goto err;
> }
> if (EC_POINT_is_at_infinity(key->group, point) == 0) {
> - GOSTerror(EC_R_WRONG_ORDER);
> + ECerror(EC_R_WRONG_ORDER);
> goto err;
> }
> /*
> @@ -156,7 +156,7 @@ GOST_KEY_check_key(const GOST_KEY *key)
>*/
> if (key->priv_key != NULL) {
> if (BN_cmp(key->priv_key, order) >= 0) {
> - GOSTerror(EC_R_WRONG_ORDER);
> + ECerror(EC_R_WRONG_ORDER);
> goto err;
> }
> if (EC_POINT_mul(key->group, point, key->priv_key, NULL, NULL,
> @@ -165,7 +165,7 @@ GOST_KEY_check_key(const GOST_KEY *key)
> goto err;
> }
> if (EC_POINT_cmp(key->group, point, key->pub_key, ctx) != 0) {
> - GOSTerror(EC_R_INVALID_PRIVATE_KEY);
> + ECerror(EC_R_INVALID_PRIVATE_KEY);
> goto err;
> }
> }
> @@ -212,7 +212,7 @@ GOST_KEY_set_public_key_affine_coordinates(GOST_KEY *key,
> BIGNUM *x, BIGNUM *y)
>* out of range.
>*/
> if (BN_cmp(x, tx) != 0 || BN_cmp(y, ty) != 0) {
> - GOSTerror(EC_R_COORDINATES_OUT_OF_RANGE);
> + ECerror(EC_R_COORDINATES_OUT_OF_RANGE);
> goto err;
> }
> if (GOST_KEY_set_public_key(key, point) == 0)
> diff --git a/src/lib/libcrypto/gost/gostr341001_pmeth.c
> b/src/lib/libcrypto/gost/gostr341001_pmeth.c
> index 0eb1d873deaf..0e0cae99e3fc 100644
> --- a/src/lib/libcrypto/gost/gostr341001_pmeth.c
> +++ b/src/lib/libcrypto/gost/gostr341001_pmeth.c
> @@ -246,7 +246,7 @@ pkey_gost01_sign(EVP_PKEY_CTX *ctx, unsigned char *sig,
> size_t *siglen,
> *siglen = 2 * size;
> return 1;
> } else if (*siglen < 2 * size) {
> - GOSTerror(EC_R_BUFFER_TOO_SMALL);
> + ECerror(EC_R_BUFFER_TOO_SMALL);
> return 0;
> }
> if (tbs_len != 32 && tbs_len != 64) {
> --
> 2.25.1
>
Re: [PATCH 1/2] gost: add missing error reporting
I had checked this by portable build and all regresses passed.
I'm ok with this diff.
On Thu, Mar 26, 2020 at 09:28:01PM +0300, dbarysh...@gmail.com wrote:
> From: Dmitry Baryshkov
>
> Add few more error reports to help debugging.
>
> Sponsored by ROSA Linux.
>
> Signed-off-by: Dmitry Baryshkov
> ---
> src/lib/libcrypto/gost/gostr341001_ameth.c | 20 +++-
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/src/lib/libcrypto/gost/gostr341001_ameth.c
> b/src/lib/libcrypto/gost/gostr341001_ameth.c
> index 16295996dce7..be621d0185dd 100644
> --- a/src/lib/libcrypto/gost/gostr341001_ameth.c
> +++ b/src/lib/libcrypto/gost/gostr341001_ameth.c
> @@ -96,15 +96,19 @@ decode_gost01_algor_params(EVP_PKEY *pkey, const unsigned
> char **p, int len)
> ec = pkey->pkey.gost;
> if (ec == NULL) {
> ec = GOST_KEY_new();
> - if (ec == NULL)
> + if (ec == NULL) {
> + GOSTerror(ERR_R_MALLOC_FAILURE);
> return 0;
> + }
> if (EVP_PKEY_assign_GOST(pkey, ec) == 0)
> return 0;
> }
>
> group = EC_GROUP_new_by_curve_name(param_nid);
> - if (group == NULL)
> + if (group == NULL) {
> + ECerror(EC_R_EC_GROUP_NEW_BY_NAME_FAILURE);
> return 0;
> + }
> EC_GROUP_set_asn1_flag(group, OPENSSL_EC_NAMED_CURVE);
> if (GOST_KEY_set_group(ec, group) == 0) {
> EC_GROUP_free(group);
> @@ -207,8 +211,10 @@ pub_decode_gost01(EVP_PKEY *pk, X509_PUBKEY *pub)
> return 0;
> }
> p = pval->data;
> - if (decode_gost01_algor_params(pk, , pval->length) == 0)
> + if (decode_gost01_algor_params(pk, , pval->length) == 0) {
> + GOSTerror(GOST_R_BAD_KEY_PARAMETERS_FORMAT);
> return 0;
> + }
>
> octet = d2i_ASN1_OCTET_STRING(NULL, _buf, pub_len);
> if (octet == NULL) {
> @@ -407,8 +413,10 @@ priv_decode_gost01(EVP_PKEY *pk, const
> PKCS8_PRIV_KEY_INFO *p8inf)
> int ptype = V_ASN1_UNDEF;
> ASN1_STRING *pval = NULL;
>
> - if (PKCS8_pkey_get0(_obj, _buf, _len, , p8inf) == 0)
> + if (PKCS8_pkey_get0(_obj, _buf, _len, , p8inf) ==
> 0) {
> + GOSTerror(GOST_R_BAD_KEY_PARAMETERS_FORMAT);
> return 0;
> + }
> (void)EVP_PKEY_assign_GOST(pk, NULL);
> X509_ALGOR_get0(NULL, , (const void **), palg);
> if (ptype != V_ASN1_SEQUENCE) {
> @@ -416,8 +424,10 @@ priv_decode_gost01(EVP_PKEY *pk, const
> PKCS8_PRIV_KEY_INFO *p8inf)
> return 0;
> }
> p = pval->data;
> - if (decode_gost01_algor_params(pk, , pval->length) == 0)
> + if (decode_gost01_algor_params(pk, , pval->length) == 0) {
> + GOSTerror(GOST_R_BAD_KEY_PARAMETERS_FORMAT);
> return 0;
> + }
> p = pkey_buf;
> if (V_ASN1_OCTET_STRING == *p) {
> /* New format - Little endian octet string */
> --
> 2.25.1
>
Re: [PATCH] ec: add support for several more GOST curves
Hi,
I have a 3 questions,
- parameter set values for Twisted Edwards
- description in _ec_list_element_st
- naming about object identifier
details are described below.
On Thu, Mar 26, 2020 at 09:25:57PM +0300, dbarysh...@gmail.com wrote:
> From: Dmitry Baryshkov
>
> Add support for GOST curves defined by RFC 7836 and
> draft-deremin-rfc4491-bis. Add aliases for 256-bit GOST curves (see
> draft-smyshlyaev-tls12-gost-suites).
>
> Sponsored by ROSA Linux.
>
> Signed-off-by: Dmitry Baryshkov
> ---
> src/lib/libcrypto/ec/ec_curve.c | 158 +-
> src/lib/libcrypto/objects/obj_mac.num | 6 +
> src/lib/libcrypto/objects/objects.txt | 10 +-
> 3 files changed, 168 insertions(+), 6 deletions(-)
>
> diff --git a/src/lib/libcrypto/ec/ec_curve.c b/src/lib/libcrypto/ec/ec_curve.c
> index e075b1ed3ea5..a1bc88ee2cc6 100644
> --- a/src/lib/libcrypto/ec/ec_curve.c
> +++ b/src/lib/libcrypto/ec/ec_curve.c
> @@ -2900,11 +2900,101 @@ static const struct {
> }
> };
>
> +static const struct {
> + EC_CURVE_DATA h;
> + unsigned char data[0 + 32 * 6];
> +}
> + _EC_GOST_2012_256_TC26_A = {
> + {
> + NID_X9_62_prime_field, 0, 32, 1
> + },
> + { /* no seed */
> + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
> /* p */
> + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
> + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
> + 0xFD, 0x97,
> + 0xc2, 0x17, 0x3f, 0x15, 0x13, 0x98, 0x16, 0x73, 0xaf, 0x48,
> /* a */
> + 0x92, 0xc2, 0x30, 0x35, 0xa2, 0x7c, 0xe2, 0x5e, 0x20, 0x13,
> + 0xbf, 0x95, 0xaa, 0x33, 0xb2, 0x2c, 0x65, 0x6f, 0x27, 0x7e,
> + 0x73, 0x35,
> + 0x29, 0x5f, 0x9b, 0xae, 0x74, 0x28, 0xed, 0x9c, 0xcc, 0x20,
> /* b */
> + 0xe7, 0xc3, 0x59, 0xa9, 0xd4, 0x1a, 0x22, 0xfc, 0xcd, 0x91,
> + 0x08, 0xe1, 0x7b, 0xf7, 0xba, 0x93, 0x37, 0xa6, 0xf8, 0xae,
> + 0x95, 0x13,
> + 0x91, 0xe3, 0x84, 0x43, 0xa5, 0xe8, 0x2c, 0x0d, 0x88, 0x09,
> /* x */
> + 0x23, 0x42, 0x57, 0x12, 0xb2, 0xbb, 0x65, 0x8b, 0x91, 0x96,
> + 0x93, 0x2e, 0x02, 0xc7, 0x8b, 0x25, 0x82, 0xfe, 0x74, 0x2d,
> + 0xaa, 0x28,
> + 0x32, 0x87, 0x94, 0x23, 0xab, 0x1a, 0x03, 0x75, 0x89, 0x57,
> /* y */
> + 0x86, 0xc4, 0xbb, 0x46, 0xe9, 0x56, 0x5f, 0xde, 0x0b, 0x53,
> + 0x44, 0x76, 0x67, 0x40, 0xaf, 0x26, 0x8a, 0xdb, 0x32, 0x32,
> + 0x2e, 0x5c,
> + 0x40, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> /* order */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0f, 0xd8, 0xcd, 0xdf,
> + 0xc8, 0x7b, 0x66, 0x35, 0xc1, 0x15, 0xaf, 0x55, 0x6c, 0x36,
> + 0x0c, 0x67,
> + }
> +};
> +
This diff adds * below, and 2 Twisted Edwards one misses m,e,d,u,v.
Is this as you expected for now ?
Canonical:
*id-tc26-gost-3410-2012-512-paramSetTest order = m = q
id-tc26-gost-3410-2012-512-paramSetAorder = m = q
id-tc26-gost-3410-2012-512-paramSetBorder = m = q
Twisted Edwards:
*id-tc26-gost-3410-2012-512-paramSetCorder = q, misses m,e,d,u,v
*id-tc26-gost-3410-2012-256-paramSetAorder = q, misses m,e,d,u,v
> static const struct {
> EC_CURVE_DATA h;
> unsigned char data[0 + 64 * 6];
> }
> - _EC_GOST_2012_TC26_A = {
> + _EC_GOST_2012_512_Test = {
> + {
> + NID_X9_62_prime_field, 0, 64, 1
> + },
> + { /* no seed */
> + 0x45, 0x31, 0xac, 0xd1, 0xfe, 0x00, 0x23, 0xc7, 0x55, 0x0d,
> /* p */
> + 0x26, 0x7b, 0x6b, 0x2f, 0xee, 0x80, 0x92, 0x2b, 0x14, 0xb2,
> + 0xff, 0xb9, 0x0f, 0x04, 0xd4, 0xeb, 0x7c, 0x09, 0xb5, 0xd2,
> + 0xd1, 0x5d, 0xf1, 0xd8, 0x52, 0x74, 0x1a, 0xf4, 0x70, 0x4a,
> + 0x04, 0x58, 0x04, 0x7e, 0x80, 0xe4, 0x54, 0x6d, 0x35, 0xb8,
> + 0x33, 0x6f, 0xac, 0x22, 0x4d, 0xd8, 0x16, 0x64, 0xbb, 0xf5,
> + 0x28, 0xbe, 0x63, 0x73,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> /* a */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x07,
> + 0x1c, 0xff, 0x08, 0x06, 0xa3, 0x11, 0x16, 0xda, 0x29, 0xd8,
> /* b */
> + 0xcf, 0xa5, 0x4e, 0x57, 0xeb, 0x74, 0x8b, 0xc5, 0xf3, 0x77,
> + 0xe4, 0x94, 0x00, 0xfd, 0xd7, 0x88, 0xb6, 0x49, 0xec, 0xa1,
> + 0xac, 0x43, 0x61, 0x83, 0x40, 0x13, 0xb2, 0xad, 0x73, 0x22,
> + 0x48, 0x0a,
Re: openssl.1: Tag command names
> I like the idea! I agree. > To me it would be more logical to put .Tg above .Sh, but that is a minor > thing. I also think that it would better to place .Tg above .Sh . On Mon, Feb 17, 2020 at 11:20:34PM +0100, Remi Locherer wrote: > On Mon, Feb 17, 2020 at 05:19:27PM +0100, Klemens Nanni wrote: > > > > I'd like to commit this soon, it allows me to jump to the command I'm > > looking for, e.g. ":tx509" shows me the synopsis right away. > > > > FWIW, some Linux distributions ship with separate manuals, e.g. x509(1SSL). > > > > Patch was done with a VIM macro by adding a new line after each `.Sh' > > line with the respective name but lowercased, so no typos in the added > > strings. > > > > Specifying it is required since the markup following the `.Tg' markup > > always starts with "openssl"; the tag must not include it (`.Tg' > > accepts at most one word anyway). > > > > I like the idea! > > To me it would be more logical to put .Tg above .Sh, but that is a minor > thing. > > > > > Index: openssl.1 > > === > > RCS file: /cvs/src/usr.bin/openssl/openssl.1,v > > retrieving revision 1.119 > > diff -u -p -U1 -r1.119 openssl.1 > > --- openssl.1 16 Feb 2020 16:39:01 - 1.119 > > +++ openssl.1 17 Feb 2020 16:11:22 - > > @@ -203,2 +203,3 @@ itself. > > .Sh ASN1PARSE > > +.Tg asn1parse > > .Bl -hang -width "openssl asn1parse" > > @@ -299,2 +300,3 @@ into a nested structure. > > .Sh CA > > +.Tg ca > > .Bl -hang -width "openssl ca" > > @@ -848,2 +850,3 @@ The same as > > .Sh CIPHERS > > +.Tg ciphers > > .Nm openssl ciphers > > @@ -880,2 +883,3 @@ but without cipher suite codes. > > .Sh CMS > > +.Tg cms > > .Bl -hang -width "openssl cms" > > @@ -1396,2 +1400,3 @@ is specified. > > .Sh CRL > > +.Tg crl > > .Bl -hang -width "openssl crl" > > @@ -1472,2 +1477,3 @@ Verify the signature on the CRL. > > .Sh CRL2PKCS7 > > +.Tg crl2pkcs7 > > .Bl -hang -width "openssl crl2pkcs7" > > @@ -1517,2 +1523,3 @@ The output format. > > .Sh DGST > > +.Tg dgst > > .Bl -hang -width "openssl dgst" > > @@ -1631,2 +1638,3 @@ If no files are specified then standard > > .Sh DHPARAM > > +.Tg dhparam > > .Bl -hang -width "openssl dhparam" > > @@ -1707,2 +1715,3 @@ parameters are generated instead. > > .Sh DSA > > +.Tg dsa > > .Bl -hang -width "openssl dsa" > > @@ -1795,2 +1804,3 @@ Print the public/private key in plain te > > .Sh DSAPARAM > > +.Tg dsaparam > > .Bl -hang -width "openssl dsaparam" > > @@ -1847,2 +1857,3 @@ If this option is included, the input fi > > .Sh EC > > +.Tg ec > > .Bl -hang -width "openssl ec" > > @@ -1959,2 +1970,3 @@ Print the public/private key in plain te > > .Sh ECPARAM > > +.Tg ecparam > > .Bl -hang -width "openssl ecparam" > > @@ -2054,2 +2066,3 @@ Print the EC parameters in plain text. > > .Sh ENC > > +.Tg enc > > .Bl -hang -width "openssl enc" > > @@ -2217,2 +2230,3 @@ Print extra details about the processing > > .Sh ERRSTR > > +.Tg errstr > > .Nm openssl errstr > > @@ -2247,2 +2261,3 @@ Print debugging statistics about various > > .Sh GENDSA > > +.Tg gendsa > > .Bl -hang -width "openssl gendsa" > > @@ -2293,2 +2308,3 @@ The parameters in this file determine th > > .Sh GENPKEY > > +.Tg genpkey > > .Bl -hang -width "openssl genpkey" > > @@ -2397,2 +2413,3 @@ Print the private/public key in plain te > > .Sh GENRSA > > +.Tg genrsa > > .Bl -hang -width "openssl genrsa" > > @@ -2454,2 +2471,3 @@ The default is 2048. > > .Sh NSEQ > > +.Tg nseq > > .Nm openssl nseq > > @@ -2484,2 +2502,3 @@ a Netscape certificate sequence is creat > > .Sh OCSP > > +.Tg ocsp > > .Bl -hang -width "openssl ocsp" > > @@ -2836,2 +2855,3 @@ option. > > .Sh PASSWD > > +.Tg passwd > > .Bl -hang -width "openssl passwd" > > @@ -2899,2 +2919,3 @@ to each password hash. > > .Sh PKCS7 > > +.Tg pkcs7 > > .Bl -hang -width "openssl pkcs7" > > @@ -2944,2 +2965,3 @@ Print certificate details in full rather > > .Sh PKCS8 > > +.Tg pkcs8 > > .Bl -hang -width "openssl pkcs8" > > @@ -3027,2 +3049,3 @@ It is recommended that des3 is used. > > .Sh PKCS12 > > +.Tg pkcs12 > > .Bl -hang -width "openssl pkcs12" > > @@ -3244,2 +3267,3 @@ is equivalent to > > .Sh PKEY > > +.Tg pkey > > .Bl -hang -width "openssl pkey" > > @@ -3307,2 +3331,3 @@ even if a private key is being processed > > .Sh PKEYPARAM > > +.Tg pkeyparam > > .Cm openssl pkeyparam > > @@ -3332,2 +3357,3 @@ Print the parameters in plain text. > > .Sh PKEYUTL > > +.Tg pkeyutl > > .Bl -hang -width "openssl pkeyutl" > > @@ -3484,2 +3510,3 @@ Verify the input data and output the rec > > .Sh PRIME > > +.Tg prime > > .Cm openssl prime > > @@ -3528,2 +3555,3 @@ is prime. > > .Sh RAND > > +.Tg rand > > .Bl -hang -width "openssl rand" > > @@ -3555,2 +3583,3 @@ or standard output if not specified. > > .Sh REQ > > +.Tg req > > .Bl -hang -width "openssl req" > > @@ -4004,2 +4033,3 @@ Any additional fields will be treated as > > .Sh RSA > > +.Tg rsa > >
Re: Fix manual description in SSL_CTX_add_extra_chain_cert.3
Hello Ingo-san, Thanks for your answer. Today, I had read through this manual, and I just thought that the newly added function was missed in the description section at last update. But now I had understood the status of manual maintenance by your explanation. I just read only one manual page and had not took consider about others. I should have thought about this from comprehensive perspective of view. I would like to abandon this patch for now. Regards, Kinichiro Inoguchi On Sun, Jan 12, 2020 at 10:46:26AM +0100, Ingo Schwarze wrote: > Hello Kinichiro-san, > > Kinichiro Inoguchi wrote on Sun, Jan 12, 2020 at 05:09:52PM +0900: > > > I think both SSL_CTX_get_extra_chain_certs and > > SSL_CTX_get_extra_chain_certs_only should be described here. > > I think the text describing what to do with internal pointers > returned from LibreSSL functions is still quite a mess in general. > In some cases (like this one) something is said explicitly, > but there are many different wordings. In other cases, nothing > is said explicitly. > > In the case at hand, the sentence you are adding to feels somewhat > redundant because the main description already says "retrieves an > internal pointer". The only reason i didn't delete the sentence > when last editing the page is because we didn't come round to a > general cleanup of such statements yet, and adding or deleting such > statements piece-meal without a comprehensive plan didn't feel like > a big improvement. > > Regarding the particular change you propose, on top of the above, > i think what you are adding is already clear from the text above, > which explains the the pointers potentially returned from > SSL_CTX_get_extra_chain_certs_only(3) are a subset of those potentially > returned from SSL_CTX_get_extra_chain_certs(3). So if the latter > mustn't be freed, that includes the former. > > > ok? > > I don't consider the patch a real improvement, but i don't strongly > object to it either. What you are proposing to say is certainly > not wrong. > > > More generally, what do people think to switching to the following > concise wording: > > SOMEOBJ_add0(obj, p) "transfers ownership of t to obj" > (or "sets the FOO of obj to p, transfering > ownership", depending on the context) > SOMEOBJ_add1(obj, p) "sets the FOO of obj to a (deep|shallow) > copy of p" > p = SOMEOBJ_get0(obj) "retrieves an internal pointer to the FOO of obj" > p = SOMEOBJ_get1(obj) "returns a (deep|shallow) copy of the FOO of obj" > > And then delete all the repetitive wordings like "you must free" > or "you must not free"? > > Yours, > Ingo > > > > Index: SSL_CTX_add_extra_chain_cert.3 > > === > > RCS file: /cvs/src/lib/libssl/man/SSL_CTX_add_extra_chain_cert.3,v > > retrieving revision 1.7 > > diff -u -p -u -p -r1.7 SSL_CTX_add_extra_chain_cert.3 > > --- SSL_CTX_add_extra_chain_cert.3 2 Jan 2020 09:09:16 - 1.7 > > +++ SSL_CTX_add_extra_chain_cert.3 12 Jan 2020 08:06:49 - > > @@ -115,7 +115,9 @@ An application should not free the > > object, nor the > > .Pf * Fa certs > > object retrieved by > > -.Fn SSL_CTX_get_extra_chain_certs . > > +.Fn SSL_CTX_get_extra_chain_certs > > +and > > +.Fn SSL_CTX_get_extra_chain_certs_only . > > .Sh RETURN VALUES > > These functions return 1 on success or 0 for failure. > > Check out the error stack to find out the reason for failure.
Fix manual description in SSL_CTX_add_extra_chain_cert.3
I think both SSL_CTX_get_extra_chain_certs and SSL_CTX_get_extra_chain_certs_only should be described here. ok? Index: SSL_CTX_add_extra_chain_cert.3 === RCS file: /cvs/src/lib/libssl/man/SSL_CTX_add_extra_chain_cert.3,v retrieving revision 1.7 diff -u -p -u -p -r1.7 SSL_CTX_add_extra_chain_cert.3 --- SSL_CTX_add_extra_chain_cert.3 2 Jan 2020 09:09:16 - 1.7 +++ SSL_CTX_add_extra_chain_cert.3 12 Jan 2020 08:06:49 - @@ -115,7 +115,9 @@ An application should not free the object, nor the .Pf * Fa certs object retrieved by -.Fn SSL_CTX_get_extra_chain_certs . +.Fn SSL_CTX_get_extra_chain_certs +and +.Fn SSL_CTX_get_extra_chain_certs_only . .Sh RETURN VALUES These functions return 1 on success or 0 for failure. Check out the error stack to find out the reason for failure.
Re: [PATCH] Extend OAEP support
I thought this patch could give an ability to handle OAEP label with
openssl(1) pkeyutl command, and encryption works fine, but decryption fails.
--
openssl genrsa -out rsakey.pem
echo "abcd" | openssl pkeyutl -encrypt -inkey rsakey.pem \
-pkeyopt rsa_padding_mode:oaep -pkeyopt rsa_oaep_label:0011223344556677 \
-out rsaoaep.enc
openssl pkeyutl -decrypt -inkey rsakey.pem
-pkeyopt rsa_padding_mode:oaep -pkeyopt rsa_oaep_label:0011223344556677 \
-in rsaoaep.enc
--
Last command fails with this message.
--
Public Key operation error
2798804220048:error:04FFF079:rsa routines:CRYPTO_internal:oaep decoding
error:rsa/rsa_oaep.c:215:
--
These commands had succeeded with OpenSSL 1.0.2.
RSA_padding_check_PKCS1_OAEP_mgf1() appears to cause this.
On Wed, Sep 04, 2019 at 06:41:21AM +0300, Stefan Strogin wrote:
> Provide methods: EVP_PKEY_CTX_{g,s}et_rsa_oaep_md,
> EVP_PKEY_CTX_{g,s}et0_rsa_oaep_label.
>
> Based on Stephen Henson's patches for OpenSSL 1.1.0:
> https://github.com/openssl/openssl/commit/271fef0ef39a1c0cb5233a5adf3ff8733abb375e
> https://github.com/openssl/openssl/commit/211a14f6279f127f7a5a59948819bd939131b0b6
Re: [PATCH v2] Provide static_ASN1_*(). From OpenSSL 1.1.0 API
Hi,
This patch was applied, and thanks for your help.
Sorry for late, since we couldn't have time to review.
On Sat, Aug 17, 2019 at 07:11:28AM +0300, Stefan Strogin wrote:
> v1->v2:
> Use correct static_ASN1_ITEM_start macros instead of ASN1_ITEM_start in
> static_ASN1_SEQUENCE_END_ref definition.
> ---
> src/lib/libcrypto/asn1/asn1t.h | 62 ++
> 1 file changed, 62 insertions(+)
>
> diff --git a/src/lib/libcrypto/asn1/asn1t.h b/src/lib/libcrypto/asn1/asn1t.h
> index ba380bdf4..e0d5bc1f7 100644
> --- a/src/lib/libcrypto/asn1/asn1t.h
> +++ b/src/lib/libcrypto/asn1/asn1t.h
> @@ -81,6 +81,9 @@ extern "C" {
> #define ASN1_ITEM_start(itname) \
> const ASN1_ITEM itname##_it = {
>
> +#define static_ASN1_ITEM_start(itname) \
> + static const ASN1_ITEM itname##_it = {
> +
> #define ASN1_ITEM_end(itname) \
> };
>
> @@ -102,6 +105,17 @@ extern "C" {
> 0,\
> #tname \
> ASN1_ITEM_end(tname)
> +#define static_ASN1_ITEM_TEMPLATE_END(tname) \
> + ;\
> + static_ASN1_ITEM_start(tname) \
> + ASN1_ITYPE_PRIMITIVE,\
> + -1,\
> + ##_item_tt,\
> + 0,\
> + NULL,\
> + 0,\
> + #tname \
> + ASN1_ITEM_end(tname)
>
>
> /* This is a ASN1 type which just embeds a template */
> @@ -130,6 +144,7 @@ extern "C" {
> static const ASN1_TEMPLATE tname##_seq_tt[]
>
> #define ASN1_SEQUENCE_END(stname) ASN1_SEQUENCE_END_name(stname, stname)
> +#define static_ASN1_SEQUENCE_END(stname)
> static_ASN1_SEQUENCE_END_name(stname, stname)
>
> #define ASN1_SEQUENCE_END_name(stname, tname) \
> ;\
> @@ -142,6 +157,17 @@ extern "C" {
> sizeof(stname),\
> #stname \
> ASN1_ITEM_end(tname)
> +#define static_ASN1_SEQUENCE_END_name(stname, tname) \
> + ;\
> + static_ASN1_ITEM_start(tname) \
> + ASN1_ITYPE_SEQUENCE,\
> + V_ASN1_SEQUENCE,\
> + tname##_seq_tt,\
> + sizeof(tname##_seq_tt) / sizeof(ASN1_TEMPLATE),\
> + NULL,\
> + sizeof(stname),\
> + #stname \
> + ASN1_ITEM_end(tname)
>
> #define ASN1_NDEF_SEQUENCE(tname) \
> ASN1_SEQUENCE(tname)
> @@ -176,12 +202,24 @@ extern "C" {
> sizeof(tname),\
> #tname \
> ASN1_ITEM_end(tname)
> +#define static_ASN1_NDEF_SEQUENCE_END(tname) \
> + ;\
> + static_ASN1_ITEM_start(tname) \
> + ASN1_ITYPE_NDEF_SEQUENCE,\
> + V_ASN1_SEQUENCE,\
> + tname##_seq_tt,\
> + sizeof(tname##_seq_tt) / sizeof(ASN1_TEMPLATE),\
> + NULL,\
> + sizeof(tname),\
> + #tname \
> + ASN1_ITEM_end(tname)
>
> #define ASN1_BROKEN_SEQUENCE_END(stname) ASN1_SEQUENCE_END_ref(stname,
> stname)
>
> #define ASN1_SEQUENCE_END_enc(stname, tname) ASN1_SEQUENCE_END_ref(stname,
> tname)
>
> #define ASN1_SEQUENCE_END_cb(stname, tname) ASN1_SEQUENCE_END_ref(stname,
> tname)
> +#define static_ASN1_SEQUENCE_END_cb(stname, tname)
> static_ASN1_SEQUENCE_END_ref(stname, tname)
>
> #define ASN1_SEQUENCE_END_ref(stname, tname) \
> ;\
> @@ -194,6 +232,17 @@ extern "C" {
> sizeof(stname),\
> #stname \
> ASN1_ITEM_end(tname)
> +#define static_ASN1_SEQUENCE_END_ref(stname, tname) \
> + ;\
> + static_ASN1_ITEM_start(tname) \
> + ASN1_ITYPE_SEQUENCE,\
> + V_ASN1_SEQUENCE,\
> + tname##_seq_tt,\
> + sizeof(tname##_seq_tt) / sizeof(ASN1_TEMPLATE),\
> + ##_aux,\
> + sizeof(stname),\
> + #stname \
> + ASN1_ITEM_end(tname)
>
> #define ASN1_NDEF_SEQUENCE_END_cb(stname, tname) \
> ;\
> @@ -238,8 +287,10 @@ extern "C" {
> ASN1_CHOICE(tname)
>
> #define ASN1_CHOICE_END(stname) ASN1_CHOICE_END_name(stname, stname)
> +#define static_ASN1_CHOICE_END(stname) static_ASN1_CHOICE_END_name(stname,
> stname)
>
> #define ASN1_CHOICE_END_name(stname, tname) ASN1_CHOICE_END_selector(stname,
> tname, type)
> +#define static_ASN1_CHOICE_END_name(stname, tname)
> static_ASN1_CHOICE_END_selector(stname, tname, type)
>
> #define ASN1_CHOICE_END_selector(stname, tname, selname) \
> ;\
> @@ -252,6 +303,17 @@ extern "C" {
> sizeof(stname),\
> #stname \
> ASN1_ITEM_end(tname)
> +#define static_ASN1_CHOICE_END_selector(stname, tname, selname) \
> + ;\
> + static_ASN1_ITEM_start(tname) \
> + ASN1_ITYPE_CHOICE,\
> + offsetof(stname,selname) ,\
> + tname##_ch_tt,\
> + sizeof(tname##_ch_tt) / sizeof(ASN1_TEMPLATE),\
> + NULL,\
> + sizeof(stname),\
> + #stname \
> + ASN1_ITEM_end(tname)
>
> #define ASN1_CHOICE_END_cb(stname, tname, selname) \
> ;\
> --
> 2.22.1
>
Re: [PATCH] Provide static_ASN1_*(). From OpenSSL 1.1.0 API.
Hi, +#define static_ASN1_SEQUENCE_END_ref(stname, tname) \ + ;\ + ASN1_ITEM_start(tname) \ I think this should be "static_ASN1_ITEM_start" instead "ASN1_ITEM_start".
Re: openssl(1) s_socket.c: don't leak `name' (CID #154702)
OK @inoguchi
2018/08/19 22:44 "Theo Buehler" :
> On Sun, Aug 19, 2018 at 09:53:32PM +0900, Kinichiro Inoguchi wrote:
> > I feel that "error case free" should be done in do_accept() rather than
> caller.
> > After strdup(), there are 2 "return (0)".
> > How about adding "free(*host)" before these 2 "return (0)" ?
>
> You're right, that makes more sense.
>
> > I worried that error return occurs before strdup() in do_accept().
>
> That would be harmless as name = NULL.
>
> > On Sun, Aug 19, 2018 at 10:40:55AM +0200, Theo Buehler wrote:
> > > do_accept() may strdup() the host name and store it in `name', so we
> > > need to free it before exiting. Perhaps a refactor might be more
> > > appropriate, but I'm not sure I want to touch this mess.
>
> Index: s_socket.c
> ===
> RCS file: /var/cvs/src/usr.bin/openssl/s_socket.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 s_socket.c
> --- s_socket.c 7 Feb 2018 05:47:55 - 1.9
> +++ s_socket.c 19 Aug 2018 13:42:59 -
> @@ -276,11 +276,13 @@ do_accept(int acc_sock, int *sock, char
> if (h2 == NULL) {
> BIO_printf(bio_err, "gethostbyname failure\n");
> close(ret);
> + free(*host);
> return (0);
> }
> if (h2->h_addrtype != AF_INET) {
> BIO_printf(bio_err, "gethostbyname addr is not
> AF_INET\n");
> close(ret);
> + free(*host);
> return (0);
> }
> }
>
Re: openssl(1) s_socket.c: don't leak `name' (CID #154702)
I feel that "error case free" should be done in do_accept() rather than caller.
After strdup(), there are 2 "return (0)".
How about adding "free(*host)" before these 2 "return (0)" ?
I worried that error return occurs before strdup() in do_accept().
On Sun, Aug 19, 2018 at 10:40:55AM +0200, Theo Buehler wrote:
> do_accept() may strdup() the host name and store it in `name', so we
> need to free it before exiting. Perhaps a refactor might be more
> appropriate, but I'm not sure I want to touch this mess.
>
> Index: s_socket.c
> ===
> RCS file: /var/cvs/src/usr.bin/openssl/s_socket.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 s_socket.c
> --- s_socket.c7 Feb 2018 05:47:55 - 1.9
> +++ s_socket.c19 Aug 2018 07:13:49 -
> @@ -151,6 +151,7 @@ do_server(int port, int type, int *ret,
> if (do_accept(accept_socket, , ) == 0) {
> shutdown(accept_socket, SHUT_RD);
> close(accept_socket);
> + free(name);
> return (0);
> }
> } else
Re: CID #183499: don't leak db in RSA_padding_check_PKCS1_OAEP()
It looks good to me.
OK inoguchi@
On Sun, Aug 19, 2018 at 08:44:24AM +0200, Theo Buehler wrote:
> Coverity complains about the case where EVP_Digest() fails, but there
> are a couple more.
>
> Index: rsa/rsa_oaep.c
> ===
> RCS file: /var/cvs/src/lib/libcrypto/rsa/rsa_oaep.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 rsa_oaep.c
> --- rsa/rsa_oaep.c5 Aug 2018 13:30:04 - 1.27
> +++ rsa/rsa_oaep.c19 Aug 2018 06:38:52 -
> @@ -126,8 +126,7 @@ RSA_padding_check_PKCS1_OAEP(unsigned ch
> }
>
> dblen = num - SHA_DIGEST_LENGTH;
> - db = malloc(dblen + num);
> - if (db == NULL) {
> + if ((db = malloc(dblen + num)) == NULL) {
> RSAerror(ERR_R_MALLOC_FAILURE);
> return -1;
> }
> @@ -143,17 +142,17 @@ RSA_padding_check_PKCS1_OAEP(unsigned ch
> maskeddb = padded_from + SHA_DIGEST_LENGTH;
>
> if (MGF1(seed, SHA_DIGEST_LENGTH, maskeddb, dblen))
> - return -1;
> + goto decoding_err;
> for (i = 0; i < SHA_DIGEST_LENGTH; i++)
> seed[i] ^= padded_from[i];
>
> if (MGF1(db, dblen, seed, SHA_DIGEST_LENGTH))
> - return -1;
> + goto decoding_err;
> for (i = 0; i < dblen; i++)
> db[i] ^= maskeddb[i];
>
> if (!EVP_Digest((void *)param, plen, phash, NULL, EVP_sha1(), NULL))
> - return -1;
> + goto decoding_err;
>
> if (timingsafe_memcmp(db, phash, SHA_DIGEST_LENGTH) != 0 || bad)
> goto decoding_err;
> @@ -177,7 +176,7 @@ RSA_padding_check_PKCS1_OAEP(unsigned ch
> free(db);
> return mlen;
>
> -decoding_err:
> + decoding_err:
> /*
>* To avoid chosen ciphertext attacks, the error message should not
>* reveal which kind of decoding error happened
Re: use BN_swap_ct() instead of BN_consttime_swap() in ec_GF2m_montgomery_point_multiply()
looks good to me.
ok inoguchi@
On Sat, Jul 14, 2018 at 02:46:17PM +0200, Theo Buehler wrote:
> The new BN_swap_ct() API is an improved version of the public
> BN_consttime_swap() function: it allows for error checking doesn't
> assert() and has fewer assumptions on the input.
>
> This eliminates the last use of the latter in our tree. With the next
> major libcrypto bump, we could replace BN_consttime_swap() with the new
> version. In the meantime let's just avoid using it.
>
> This adds a second reacharound from ec/ to bn/ but that is hopefully
> only temporary.
>
> Index: ec/ec2_mult.c
> ===
> RCS file: /var/cvs/src/lib/libcrypto/ec/ec2_mult.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 ec2_mult.c
> --- ec/ec2_mult.c 10 Jul 2018 22:06:14 - 1.10
> +++ ec/ec2_mult.c 14 Jul 2018 12:34:47 -
> @@ -71,6 +71,7 @@
>
> #include
>
> +#include "bn_lcl.h"
> #include "ec_lcl.h"
>
> #ifndef OPENSSL_NO_EC2M
> @@ -324,14 +325,18 @@ ec_GF2m_montgomery_point_multiply(const
> for (; i >= 0; i--) {
> word = scalar->d[i];
> while (mask) {
> - BN_consttime_swap(word & mask, x1, x2,
> group->field.top);
> - BN_consttime_swap(word & mask, z1, z2,
> group->field.top);
> + if (!BN_swap_ct(word & mask, x1, x2, group->field.top))
> + goto err;
> + if (!BN_swap_ct(word & mask, z1, z2, group->field.top))
> + goto err;
> if (!gf2m_Madd(group, >X, x2, z2, x1, z1, ctx))
> goto err;
> if (!gf2m_Mdouble(group, x1, z1, ctx))
> goto err;
> - BN_consttime_swap(word & mask, x1, x2,
> group->field.top);
> - BN_consttime_swap(word & mask, z1, z2,
> group->field.top);
> + if (!BN_swap_ct(word & mask, x1, x2, group->field.top))
> + goto err;
> + if (!BN_swap_ct(word & mask, z1, z2, group->field.top))
> + goto err;
> mask >>= 1;
> }
> mask = BN_TBIT;
>
Re: BN_swap_ct() use size_t for byte count
I checked this diff libressl portable build.
ok inoguchi@
On Sat, Jul 14, 2018 at 02:30:12PM +0200, Theo Buehler wrote:
> As pointed out by jsing, using size_t for nwords would be more
> appropriate for the new internal API BN_swap_ct(). Let's switch to it
> and cast to an int internally after checking the size to avoid overflow.
>
> Index: bn/bn_lib.c
> ===
> RCS file: /var/cvs/src/lib/libcrypto/bn/bn_lib.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 bn_lib.c
> --- bn/bn_lib.c 13 Jul 2018 08:43:31 - 1.44
> +++ bn/bn_lib.c 14 Jul 2018 12:17:35 -
> @@ -897,16 +897,19 @@ BN_consttime_swap(BN_ULONG condition, BI
> * nwords is the number of words to swap.
> */
> int
> -BN_swap_ct(BN_ULONG condition, BIGNUM *a, BIGNUM *b, int nwords)
> +BN_swap_ct(BN_ULONG condition, BIGNUM *a, BIGNUM *b, size_t nwords)
> {
> BN_ULONG t;
> - int i;
> + int i, words;
>
> if (a == b)
> return 1;
> - if (bn_wexpand(a, nwords) == NULL || bn_wexpand(b, nwords) == NULL)
> + if (nwords > INT_MAX)
> return 0;
> - if (a->top > nwords || b->top > nwords) {
> + words = (int)nwords;
> + if (bn_wexpand(a, words) == NULL || bn_wexpand(b, words) == NULL)
> + return 0;
> + if (a->top > words || b->top > words) {
> BNerror(BN_R_INVALID_LENGTH);
> return 0;
> }
> @@ -930,7 +933,7 @@ BN_swap_ct(BN_ULONG condition, BIGNUM *a
> b->flags ^= t;
>
> /* swap the data */
> - for (i = 0; i < nwords; i++) {
> + for (i = 0; i < words; i++) {
> t = (a->d[i] ^ b->d[i]) & condition;
> a->d[i] ^= t;
> b->d[i] ^= t;
> Index: bn/bn_lcl.h
> ===
> RCS file: /var/cvs/src/lib/libcrypto/bn/bn_lcl.h,v
> retrieving revision 1.28
> diff -u -p -r1.28 bn_lcl.h
> --- bn/bn_lcl.h 10 Jul 2018 21:52:07 - 1.28
> +++ bn/bn_lcl.h 14 Jul 2018 12:15:51 -
> @@ -606,7 +606,7 @@ BIGNUM *BN_mod_inverse_nonct(BIGNUM *ret
> int BN_gcd_ct(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, BN_CTX *ctx);
> int BN_gcd_nonct(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, BN_CTX *ctx);
>
> -int BN_swap_ct(BN_ULONG swap, BIGNUM *a, BIGNUM *b, int nwords);
> +int BN_swap_ct(BN_ULONG swap, BIGNUM *a, BIGNUM *b, size_t nwords);
>
> __END_HIDDEN_DECLS
> #endif
>
Re: Replace getprogname() to argv[0] in bnaddsub
Currently, argv[0] is used in regression tests and command line tools only,
and it is not in libraries.
But, I'll try to think of that.
Thanks.
On Fri, Jul 20, 2018 at 05:56:07AM -0600, Theo de Raadt wrote:
> Disagree with this. argv[0] could be a long path, which could info
> disclose directory hierarchy into a context that bounces stderr to
> the wrong place.
>
> Instead, the Windows compat should have a have a getprogname() stub
> which does the right thing.
>
> Kinichiro Inoguchi wrote:
>
> > To run regress bnaddsub on Windows, I would like to supersede getprogname
> > with argv[0] since it is not on Windows.
> >
> > OK ?
> >
> > Index: regress/lib/libcrypto/bn/addsub/bnaddsub.c
> > ===
> > RCS file: /cvs/src/regress/lib/libcrypto/bn/addsub/bnaddsub.c,v
> > retrieving revision 1.1
> > diff -u -p -u -p -r1.1 bnaddsub.c
> > --- regress/lib/libcrypto/bn/addsub/bnaddsub.c 10 Jul 2018 16:57:50
> > - 1.1
> > +++ regress/lib/libcrypto/bn/addsub/bnaddsub.c 17 Jul 2018 13:01:08
> > -
> > @@ -216,7 +216,7 @@ main(int argc, char *argv[])
> >
> > if ((bio_err = BIO_new_fp(stderr, BIO_NOCLOSE)) == NULL) {
> > fprintf(stderr, "%s: failed to initialize bio_err",
> > - getprogname());
> > + argv[0]);
> > return 1;
> > }
> >
> >
Re: Replace getprogname() to argv[0] in bnaddsub
__progname is not portable, and I once requested to eliminate it.
For porable reason, I prefer to using argv[0].
Anyway, thanks for your comment.
Kinichiro Inoguchi
On Tue, Jul 17, 2018 at 12:01:59PM -0300, Gleydson Soares wrote:
> On Tue, Jul 17, 2018 at 10:09:37PM +0900, Kinichiro Inoguchi wrote:
> > To run regress bnaddsub on Windows, I would like to supersede getprogname
> > with argv[0] since it is not on Windows.
> >
> > OK ?
> >
> > Index: regress/lib/libcrypto/bn/addsub/bnaddsub.c
> > ===
> > RCS file: /cvs/src/regress/lib/libcrypto/bn/addsub/bnaddsub.c,v
> > retrieving revision 1.1
> > diff -u -p -u -p -r1.1 bnaddsub.c
> > --- regress/lib/libcrypto/bn/addsub/bnaddsub.c 10 Jul 2018 16:57:50
> > - 1.1
> > +++ regress/lib/libcrypto/bn/addsub/bnaddsub.c 17 Jul 2018 13:01:08
> > -
> > @@ -216,7 +216,7 @@ main(int argc, char *argv[])
> >
> > if ((bio_err = BIO_new_fp(stderr, BIO_NOCLOSE)) == NULL) {
> > fprintf(stderr, "%s: failed to initialize bio_err",
> > - getprogname());
> > + argv[0]);
> > return 1;
> > }
> >
> >
>
> how about of using __progname ?
Replace getprogname() to argv[0] in bnaddsub
To run regress bnaddsub on Windows, I would like to supersede getprogname
with argv[0] since it is not on Windows.
OK ?
Index: regress/lib/libcrypto/bn/addsub/bnaddsub.c
===
RCS file: /cvs/src/regress/lib/libcrypto/bn/addsub/bnaddsub.c,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 bnaddsub.c
--- regress/lib/libcrypto/bn/addsub/bnaddsub.c 10 Jul 2018 16:57:50 -
1.1
+++ regress/lib/libcrypto/bn/addsub/bnaddsub.c 17 Jul 2018 13:01:08 -
@@ -216,7 +216,7 @@ main(int argc, char *argv[])
if ((bio_err = BIO_new_fp(stderr, BIO_NOCLOSE)) == NULL) {
fprintf(stderr, "%s: failed to initialize bio_err",
- getprogname());
+ argv[0]);
return 1;
}
Re: make X509_CRL_METHOD_free() NULL-safe
OK inoguchi@
Re: LibreSSL patch for AIX
Hi, I would like to see that patch. Since I thought autotools could adjust the differences between OSs, I didn't know that a special patch needed for AIX. Best regards, Kinichiro Inoguchi
Re: Reported problem: postfix, libressl 2.6.x, DHE-RSA-AES256-SHA
Same kind of issue had been raised for Postfix and LibreSSL 2.2.2 2 years ago. http://postfix.1071664.n5.nabble.com/SSL-accept-errors-after-recent-upgrade-to-LibreSSL-2-2-2-td78774.html In this case, SSLv2 caused some wrong behavior.
Re: Reported problem: postfix, libressl 2.6.x, DHE-RSA-AES256-SHA
Hi, In my OpenBSD 6.2, LibreSSL server and OpenSSL client can communicate successfully via cipher DHE-RSA-AES256-SHA. -- # Server LibreSSL 2.6.3 $ openssl s_server -cert server.pem -- # Client OpenSSL 1.0.2l $ eopenssl s_client -CAfile ca.pem -msg -cipher DHE-RSA-AES256-SHA ... Verify return code: 0 (ok) -- Can we know about SSL/TLS stack at client side ? Are there any hint to investigate this issue ? Thanks On Fri, Nov 10, 2017 at 11:58:04AM +, Stuart Henderson wrote: > From an irc contact using LibreSSL 2.6.3 on FreeBSD:
Re: [libcrypto] Don't build empty ecp_nistp* objects
Absolutely. ok inoguchi@ On Mon, Jul 17, 2017 at 06:26:30AM -0500, Brent Cook wrote: > OPENSSL_NO_EC_NISTP_64_GCC_128 has been defined in opensslfeatures.h for a > long time, which effectively means that ecp_nistp* are all empty files. So, > there is no reason to build them in the first place. OK? > > Index: Makefile > === > RCS file: /cvs/src/lib/libcrypto/Makefile,v > retrieving revision 1.20 > diff -u -p -u -p -r1.20 Makefile > --- Makefile10 Jul 2017 21:30:37 - 1.20 > +++ Makefile17 Jul 2017 11:21:23 - > @@ -126,7 +126,6 @@ SRCS+= dso_openssl.c > SRCS+= ec_lib.c ecp_smpl.c ecp_mont.c ecp_nist.c ec_cvt.c ec_mult.c > SRCS+= ec_err.c ec_curve.c ec_check.c ec_print.c ec_asn1.c ec_key.c > SRCS+= ec2_smpl.c ec2_mult.c ec_ameth.c ec_pmeth.c eck_prn.c > -SRCS+= ecp_nistp224.c ecp_nistp256.c ecp_nistp521.c ecp_nistputil.c > SRCS+= ecp_oct.c ec2_oct.c ec_oct.c > > # ecdh/
Re: chachatest.c patch
Hi, I have a question for this patch. Do you refer to the document below ? https://tools.ietf.org/html/draft-strombergson-chacha-test-vectors-01 Best regards, Kinichiro Inoguchi On Sat, Mar 18, 2017 at 07:56:19AM -0700, Steven Roberts wrote: > TC3 contained the data for TC4. > TC4 contained incorrect data. > > Moved the data from TC3 into TC4. > Generated correct data for TC3. > > Index: chachatest.c
LibreSSL should not allow too many consecutive warning alerts
Hi,
This patch is for CVE-2016-8610.
See http://seclists.org/oss-sec/2016/q4/224 .
- Don't allow too many consecutive warning alerts up to MAX_WARN_ALERT_COUNT
OpenSSL seems not to fix this issue on branch 1.0.1.
Then I refer to these 2 commits on branch 1.0.2.
- Don't allow too many consecutive warning alerts
https://github.com/openssl/openssl/commit/22646a075e75991b4e8f5d67171e45a6aead5b48
- Add missing error string for SSL_R_TOO_MANY_WARN_ALERTS
https://github.com/openssl/openssl/commit/f1f97699cb5e01f1b7e37f4c92df1a9bce6772f5
With this patch, I could build LibreSSL portable and confirmed all regressions
were passed.
But, I could not test by POC or exploit code since it is not opened.
This patch is created by command `cvs diff -Nau`.
ok ?
Index: lib/libssl/d1_pkt.c
===
RCS file: /cvs/src/lib/libssl/d1_pkt.c,v
retrieving revision 1.48
diff -u -p -a -u -r1.48 d1_pkt.c
--- lib/libssl/d1_pkt.c 11 Sep 2015 18:08:21 - 1.48
+++ lib/libssl/d1_pkt.c 26 Oct 2016 09:35:28 -
@@ -722,6 +722,13 @@ start:
goto start;
}
+ /*
+* Reset the count of consecutive warning alerts if we've got a
non-empty
+* record that isn't an alert.
+*/
+ if (rr->type != SSL3_RT_ALERT && rr->length != 0)
+ s->cert->alert_count = 0;
+
/* we now have a packet which can be read and processed */
if (s->s3->change_cipher_spec /* set when we receive ChangeCipherSpec,
@@ -932,15 +939,21 @@ start:
cb(s, SSL_CB_READ_ALERT, j);
}
- if (alert_level == 1) /* warning */
- {
+ if (alert_level == SSL3_AL_WARNING) {
s->s3->warn_alert = alert_descr;
+
+ s->cert->alert_count++;
+ if (s->cert->alert_count == MAX_WARN_ALERT_COUNT) {
+ al = SSL_AD_UNEXPECTED_MESSAGE;
+ SSLerr(SSL_F_DTLS1_READ_BYTES,
SSL_R_TOO_MANY_WARN_ALERTS);
+ goto f_err;
+ }
+
if (alert_descr == SSL_AD_CLOSE_NOTIFY) {
s->shutdown |= SSL_RECEIVED_SHUTDOWN;
return (0);
}
- } else if (alert_level == 2) /* fatal */
- {
+ } else if (alert_level == SSL3_AL_FATAL) {
s->rwstate = SSL_NOTHING;
s->s3->fatal_alert = alert_descr;
SSLerr(SSL_F_DTLS1_READ_BYTES, SSL_AD_REASON_OFFSET +
alert_descr);
Index: lib/libssl/s3_pkt.c
===
RCS file: /cvs/src/lib/libssl/s3_pkt.c,v
retrieving revision 1.58
diff -u -p -a -u -r1.58 s3_pkt.c
--- lib/libssl/s3_pkt.c 10 Jul 2016 23:07:34 - 1.58
+++ lib/libssl/s3_pkt.c 26 Oct 2016 09:35:29 -
@@ -914,6 +914,13 @@ start:
return (ret);
}
+ /*
+* Reset the count of consecutive warning alerts if we've got a
non-empty
+* record that isn't an alert.
+*/
+ if (rr->type != SSL3_RT_ALERT && rr->length != 0)
+ s->cert->alert_count = 0;
+
/* we now have a packet which can be read and processed */
if (s->s3->change_cipher_spec /* set when we receive ChangeCipherSpec,
@@ -1103,9 +1110,17 @@ start:
cb(s, SSL_CB_READ_ALERT, j);
}
- if (alert_level == 1) {
+ if (alert_level == SSL3_AL_WARNING) {
/* warning */
s->s3->warn_alert = alert_descr;
+
+ s->cert->alert_count++;
+ if (s->cert->alert_count == MAX_WARN_ALERT_COUNT) {
+ al = SSL_AD_UNEXPECTED_MESSAGE;
+ SSLerr(SSL_F_SSL3_READ_BYTES,
SSL_R_TOO_MANY_WARN_ALERTS);
+ goto f_err;
+ }
+
if (alert_descr == SSL_AD_CLOSE_NOTIFY) {
s->shutdown |= SSL_RECEIVED_SHUTDOWN;
return (0);
@@ -1125,7 +1140,7 @@ start:
SSL_R_NO_RENEGOTIATION);
goto f_err;
}
- } else if (alert_level == 2) {
+ } else if (alert_level == SSL3_AL_FATAL) {
/* fatal */
s->rwstate = SSL_NOTHING;
s->s3->fatal_alert = alert_descr;
Index: lib/libssl/ssl.h
===
RCS file: /cvs/src/lib/libssl/ssl.h,v
retrieving revision 1.96
diff -u -p -a -u -r1.96 ssl.h
--- lib/libssl/ssl.h25 Oct 2015 16:07:04 - 1.96
+++
Re: Fix boundary issue in chacha code
Sorry for my misunderstanding, and thanks for teaching me.
I had read C99 standard document.
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf
On p.130 6.7.8 Initialization "32 EXAMPLE 8 The declaration" says,
-
char s[] = "abc", t[3] = "abc";
defines ‘‘plain’’ char array objects s and t whose elements are initialized
with character string literals.
This declaration is identical to
char s[] = { 'a', 'b', 'c', '\0' },
t[] = { 'a', 'b', 'c' };
-
I did not know about this type of initialization.
Best regards,
Kinichiro
Re: Explicitly cast the return variable in tls_load_file()
I could have an answer that this compilation error was a bug of compiler, and that bug will be tracked. https://software.intel.com/en-us/forums/intel-c-compiler/topic/698109 I saw the type of buf was changed in cvs, then I can avoid this compilation problem. Thanks. Kinichiro
Re: Explicitly cast the return variable in tls_load_file()
Thanks, that is apparently better than I suggested and reasonable. And I confirmed it can also avoid the issue. I appreciate if this is applied. And, yes I believe compilation error is bug of compiler, not source code. I posted this compilation error to Intel C++ compiler forum on Sunday. https://software.intel.com/en-us/forums/intel-c-compiler My post is not showed up yet since it is not approved on weekend, though, I would like to confirm the thoughts of compiler side.
Explicitly cast the return variable in tls_load_file()
I would like to cast the return variable explicitly in tls_load_file(). This fix also avoiding Intel C++ compiler "assertion failed" described here. https://github.com/libressl-portable/portable/issues/209#issuecomment-249587024 ok ? Index: tls_util.c === RCS file: /cvs/src/lib/libtls/tls_util.c,v retrieving revision 1.3 diff -u -p -r1.3 tls_util.c --- tls_util.c 9 Sep 2015 19:49:07 - 1.3 +++ tls_util.c 1 Oct 2016 11:30:28 - @@ -154,7 +154,7 @@ tls_load_file(const char *name, size_t * done: *len = size; - return (buf); + return ((uint8_t *)buf); fail: free(buf);
Re: LibreSSL selects weak digest for (EC)DH
Thanks, Brent. I appreciate if you commit this. Kinichiro
LibreSSL selects weak digest for (EC)DH
Hi,
I would like to fix this SNI issue.
reported by @davidben
https://github.com/libressl-portable/openbsd/issues/69
#3560: OpenSSL selects weak digest for (EC)DH
https://rt.openssl.org/Ticket/Display.html?id=3560
original OpenSSL commit is here.
https://git.openssl.org/gitweb/?p=openssl.git;a=commit;h=4e05aedbcab7f7f83a887e952ebdcc5d4f2291e4
I will add a patch for this.
ok ?
Index: ssl_lib.c
===
RCS file: /cvs/src/lib/libssl/ssl_lib.c,v
retrieving revision 1.116
diff -u -p -u -r1.116 ssl_lib.c
--- ssl_lib.c 25 Oct 2015 15:52:49 - 1.116
+++ ssl_lib.c 15 Sep 2016 06:52:52 -
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_lib.c,v 1.116 2015/10/25 15:52:49 doug Exp $ */
+/* $OpenBSD: ssl_lib.c,v 1.115 2015/10/19 17:59:39 beck Exp $ */
/* Copyright (C) 1995-1998 Eric Young (e...@cryptsoft.com)
* All rights reserved.
*
@@ -2847,13 +2847,22 @@ SSL_get_SSL_CTX(const SSL *ssl)
SSL_CTX *
SSL_set_SSL_CTX(SSL *ssl, SSL_CTX* ctx)
{
+ CERT *ocert = ssl->cert;
if (ssl->ctx == ctx)
return (ssl->ctx);
if (ctx == NULL)
ctx = ssl->initial_ctx;
- if (ssl->cert != NULL)
- ssl_cert_free(ssl->cert);
ssl->cert = ssl_cert_dup(ctx->cert);
+ if (ocert != NULL) {
+ int i;
+ /* Copy negotiated digests from original */
+ for (i = 0; i < SSL_PKEY_NUM; i++) {
+ CERT_PKEY *cpk = ocert->pkeys + i;
+ CERT_PKEY *rpk = ssl->cert->pkeys + i;
+ rpk->digest = cpk->digest;
+ }
+ ssl_cert_free(ocert);
+ }
CRYPTO_add(>references, 1, CRYPTO_LOCK_SSL_CTX);
SSL_CTX_free(ssl->ctx); /* decrement reference count */
ssl->ctx = ctx;
openssl s_time error with -time option
Hi,
openssl s_time command gets "invalid seconds argument for -time".
I think options_parse() should increment i for OPTION_ARG_TIME, too.
I attached the patch for this issue.
Sorry, now I am trying to learn about /usr/src source tree and cvs, still.
Then this patch is made from git repo of libressl-portable.
Best regards,
Kinichiro Inoguchi
diff --git src/usr.bin/openssl/apps.c src/usr.bin/openssl/apps.c
index d1d0d14..525166e 100644
--- src/usr.bin/openssl/apps.c
+++ src/usr.bin/openssl/apps.c
@@ -2218,7 +2218,8 @@ options_parse(int argc, char **argv, struct option *opts,
char **unnamed,
opt->type == OPTION_ARG_FORMAT ||
opt->type == OPTION_ARG_FUNC ||
opt->type == OPTION_ARG_INT ||
- opt->type == OPTION_ARG_LONG) {
+ opt->type == OPTION_ARG_LONG ||
+ opt->type == OPTION_ARG_TIME) {
if (++i >= argc) {
fprintf(stderr, "missing %s argument for -%s\n",
opt->argname, opt->name);
Parsing tlsext strictly in libressl
Hi,
I verified that regression test
src/regress/lib/libssl/unit/tls_ext_alpn.c fails on these cases;
- proto_invalid_len5, 7, 8
- proto_invalid_missing1 - 5
- proto_invalid_missing8, 9
To correct these failures, ssl_parse_clienthello_tlsext() and
ssl_parse_serverhello_tlsext() in ssl/t1_lib.c should be fixed.
I attached the patch and it resolves the issues above.
BTW, ssl_parse_serverhello_tlsext() and ssl_parse_clienthello_tlsext()
of OpenSSL codebase seems to have issues below, I believe.
- fixes in ssl_parse_clienthello_tlsext() is NOT reflected to
ssl_parse_serverhello_tlsext() (ex. limit, parsing logic, etc.)
- ssl_parse_serverhello_tlsext() always `goto ri_check`
if data structure is invalid.
If I'm wrong, please let me know.
Thanks.
diff --git src/lib/libssl/src/ssl/t1_lib.c src/lib/libssl/src/ssl/t1_lib.c
index b225bb3..01c341f 100644
--- src/lib/libssl/src/ssl/t1_lib.c
+++ src/lib/libssl/src/ssl/t1_lib.c
@@ -1214,20 +1214,25 @@ ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p,
unsigned char *d,
s->s3->next_proto_neg_seen = 0;
free(s->s3->alpn_selected);
s->s3->alpn_selected = NULL;
+ s->srtp_profile = NULL;
- if (data >= (d + n - 2))
+ if (data == (d + n))
goto ri_check;
- n2s(data, len);
- if (data > (d + n - len))
- goto ri_check;
+ if ((d + n) - data < 2)
+ goto err;
+
+ n2s(data, len);
+ if ((d + n) - data != len)
+ goto err;
- while (data <= (d + n - 4)) {
+ while ((d + n) - data >= 4) {
n2s(data, type);
n2s(data, size);
- if (data + size > (d + n))
- goto ri_check;
+ if ((d + n) - data < size)
+ goto err;
+
if (s->tlsext_debug_cb)
s->tlsext_debug_cb(s, 0, type, data, size,
s->tlsext_debug_arg);
@@ -1560,6 +1565,10 @@ ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p,
unsigned char *d,
data += size;
}
+ /* Spurious data on the end */
+ if (data != (d + n))
+ goto err;
+
*p = data;
ri_check:
@@ -1574,6 +1583,10 @@ ri_check:
}
return 1;
+
+err:
+ *al = SSL_AD_DECODE_ERROR;
+ return 0;
}
/*
@@ -1599,9 +1612,9 @@ int
ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
int n, int *al)
{
- unsigned short length;
unsigned short type;
unsigned short size;
+ unsigned short len;
unsigned char *data = *p;
int tlsext_servername = 0;
int renegotiate_seen = 0;
@@ -1610,21 +1623,22 @@ ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p,
unsigned char *d,
free(s->s3->alpn_selected);
s->s3->alpn_selected = NULL;
- if (data >= (d + n - 2))
+ if (data == (d + n))
goto ri_check;
- n2s(data, length);
- if (data + length != d + n) {
- *al = SSL_AD_DECODE_ERROR;
- return 0;
- }
+ if ((d + n) - data < 2)
+ goto err;
+
+ n2s(data, len);
+ if ((d + n) - data != len)
+ goto err;
- while (data <= (d + n - 4)) {
+ while ((d + n) - data >= 4) {
n2s(data, type);
n2s(data, size);
- if (data + size > (d + n))
- goto ri_check;
+ if ((d + n) - data < size)
+ goto err;
if (s->tlsext_debug_cb)
s->tlsext_debug_cb(s, 1, type, data, size,
@@ -1818,6 +1832,10 @@ ri_check:
}
return 1;
+
+err:
+ *al = SSL_AD_DECODE_ERROR;
+ return 0;
}
int
initialize variables patch for bn_nist.c
Hi,
When I build LibreSSL portable on HP-UX 11.3 with HP C/aC++ compiler,
this warning is detected.
...
"bn/bn_nist.c", line 611: warning #2549-D: variable "buf" is used before its
value is set
nist_set_224(buf.bn, c_d, 14, 13, 12, 11, 10, 9, 8);
^
...
To initialize these variables before using, I would like to apply the patch.
OK ?
Here is original topic on GitHub.
https://github.com/libressl-portable/openbsd/pull/19
Best Regards,
kinichiro inoguchi
diff --git src/lib/libssl/src/crypto/bn/bn_nist.c
src/lib/libssl/src/crypto/bn/bn_nist.c
index 4d3a612..a87309b 100644
--- src/lib/libssl/src/crypto/bn/bn_nist.c
+++ src/lib/libssl/src/crypto/bn/bn_nist.c
@@ -510,7 +510,7 @@ BN_nist_mod_192(BIGNUM *r, const BIGNUM *a, const BIGNUM
*field, BN_CTX *ctx)
}
#else
{
- BN_ULONG t_d[BN_NIST_192_TOP];
+ BN_ULONG t_d[BN_NIST_192_TOP] = {0};
nist_set_192(t_d, buf.bn, 0, 3, 3);
carry = (int)bn_add_words(r_d, r_d, t_d, BN_NIST_192_TOP);
@@ -568,7 +568,7 @@ BN_nist_mod_224(BIGNUM *r, const BIGNUM *a, const BIGNUM
*field, BN_CTX *ctx)
BN_ULONG bn[BN_NIST_224_TOP];
unsigned int ui[BN_NIST_224_TOP *
sizeof(BN_ULONG) / sizeof(unsigned int)];
- } buf;
+ } buf = {0};
BN_ULONG c_d[BN_NIST_224_TOP], *res;
uintptr_t mask;
union {
@@ -673,7 +673,7 @@ BN_nist_mod_224(BIGNUM *r, const BIGNUM *a, const BIGNUM
*field, BN_CTX *ctx)
}
#else
{
- BN_ULONG t_d[BN_NIST_224_TOP];
+ BN_ULONG t_d[BN_NIST_224_TOP] = {0};
nist_set_224(t_d, buf.bn, 10, 9, 8, 7, 0, 0, 0);
carry = (int)bn_add_words(r_d, r_d, t_d, BN_NIST_224_TOP);
@@ -746,7 +746,7 @@ BN_nist_mod_256(BIGNUM *r, const BIGNUM *a, const BIGNUM
*field, BN_CTX *ctx)
unsigned int ui[BN_NIST_256_TOP *
sizeof(BN_ULONG) / sizeof(unsigned int)];
} buf;
- BN_ULONG c_d[BN_NIST_256_TOP], *res;
+ BN_ULONG c_d[BN_NIST_256_TOP] = {0}, *res;
uintptr_t mask;
union {
bn_addsub_f f;
@@ -879,7 +879,7 @@ BN_nist_mod_256(BIGNUM *r, const BIGNUM *a, const BIGNUM
*field, BN_CTX *ctx)
}
#else
{
- BN_ULONG t_d[BN_NIST_256_TOP];
+ BN_ULONG t_d[BN_NIST_256_TOP] = {0};
/*S1*/
nist_set_256(t_d, buf.bn, 15, 14, 13, 12, 11, 0, 0, 0);
@@ -1133,7 +1133,7 @@ BN_nist_mod_384(BIGNUM *r, const BIGNUM *a, const BIGNUM
*field, BN_CTX *ctx)
}
#else
{
- BN_ULONG t_d[BN_NIST_384_TOP];
+ BN_ULONG t_d[BN_NIST_384_TOP] = {0};
/*S1*/
nist_set_256(t_d, buf.bn, 0, 0, 0, 0, 0, 23 - 4, 22 - 4,
add error check to ocsp_test.c
Hi,
I would like to add error check for CAfile loading
since some OS doesn't have /etc/ssl/cert.pem.
Best regards,
Kinichiro Inoguchi
diff --git src/regress/lib/libcrypto/ocsp/ocsp_test.c
src/regress/lib/libcrypto/ocsp/ocsp_test.c
index 8867536..31594fa 100644
--- src/regress/lib/libcrypto/ocsp/ocsp_test.c
+++ src/regress/lib/libcrypto/ocsp/ocsp_test.c
@@ -47,6 +47,7 @@ int main(int argc, char *argv[]) {
X509_STORE *st = NULL;
STACK_OF(X509) *ch = NULL;
char *host, *port;
+ char *cafile = "/etc/ssl/cert.pem";
SSL *ssl;
SSL_CTX *ctx;
@@ -56,7 +57,10 @@ int main(int argc, char *argv[]) {
ctx = SSL_CTX_new(SSLv23_client_method());
- SSL_CTX_load_verify_locations(ctx, "/etc/ssl/cert.pem", NULL);
+ if (!SSL_CTX_load_verify_locations(ctx, cafile, NULL)) {
+ printf("failed to load %s\n", cafile);
+ exit(-1);
+ }
if (argc != 3)
errx(-1, "need a host and port to connect to");
Re: Add libtls functionality for OCSP, and OCSP stapling support
Hi, I have 2 questions about this implementation. 1) Can the OCSP client put multiple certificates to check in the request ? like this. $ openssl ocsp -reqin ocsp_req.der -req_text OCSP Request Data: Version: 1 (0x0) Requestor List: Certificate ID: Hash Algorithm: sha1 Issuer Name Hash: 3429CF3BC59A76F61C3296E597B1F9D5F4A52B3A Issuer Key Hash: 68DBFBB578936A6704433C981F7ECE61819838C7 Serial Number: 03 Certificate ID: Hash Algorithm: sha1 Issuer Name Hash: 3429CF3BC59A76F61C3296E597B1F9D5F4A52B3A Issuer Key Hash: 68DBFBB578936A6704433C981F7ECE61819838C7 Serial Number: D0F00ED53778C7C5 Request Extensions: OCSP Nonce: 04104C65A6FA1D4839916C3B8C18A4EF2E5D 2) Is it available signing to OCSP request by client ? I indicate this https://tools.ietf.org/html/rfc6960#section-4.1.2 "The requestor MAY choose to sign the OCSP request." These 2 functionality might NOT need when we're doing OCSP stapling. (server cert to verify by OCSP stapling will be always single ...) Best regards, Kinichiro Inoguchi
