Re: Assigning OpenSSL 3.0.0 beta1 issues

2020-10-17 Thread Benjamin Kaduk
On Wed, Oct 14, 2020 at 04:36:39PM +, Salz, Rich wrote:
> On 14/10/2020 17:08, Salz, Rich wrote:
> > I am interested in helping out with the deprecation tasks. Should I 
> assume that Richard's PR to change how it's done will be going in?
> > 
> 
> I'm not sure which of Richard's PRs you're referring to?
> 
> I thought he had a "rethinking how we deprecate" PR or issue, but I can't 
> find it now.

I think it was https://github.com/openssl/openssl/pull/13074 (now merged).

-Ben


Re: Vote proposal: Technical items still to be done

2020-10-07 Thread Benjamin Kaduk
On Wed, Oct 07, 2020 at 12:35:28PM +0100, Matt Caswell wrote:
> I had an action from the OTC meeting today to raise a vote on the OTC
> list of technical items still to be done. Here is my proposed vote text.
> There will be a subsequent vote on the "beta readiness checklist" which
> is a separate list.

Thanks, Matt; this is a fine list of desirable things.  (I tried to resist
commenting on specific items, but building the 1.1.1 apps+tests against 3.0
libraries is a solid way to help meet our stability goals, and should
arguably be something that run-checker just does, all the time.)

I've trimmed the list, though, to make a broader point, which could be
summarized as "the perfect is the enemy of the good".

It's a natural consequence for a software project that has long-term
supported releases, strong API stability guarantees, and infrequent
releases, to feel that each major release is a critical threshold and that
we need to do a bunch of tidying before the release to wrap it up nicely.
Paying off technical debt is often a bit part of the tidying that is
perceived necessary, as is attempting to be future looking -- missing the
release, after all, is the difference between needing to support some
bad/annoying thing for (say) 8 years instead of "only" 5.

There is value in doing this fixup, yes, but there is also cost in keeping
all the good things (and other fixup) that is already done out of the hands
of those who wish to consume it.  I've seen this phenomenon bite various
projects over time with effects of different magnitude, varying from
FreeBSD 5.0+SMP that had nearly existential consequences on the project, to
OpenAFS 1.6.0 where release delays produced enough frustration to lead to a
bit of a rush-job final release that was a bit unstable; I was lucky enough
to have missed the worst of this effect for krb5, and managed to do a
little better (but still not great) for OpenAFS 1.8.0.

Projects that get hit really badly by this phenomenon tend to correct for
it and end up on a fixed time-based cadence of releases, and in order to
stick to that cadence end up having to get comfortable shipping releases
without a desired feature or that are known to have incomplete parts of one
feature or another.  It also requires discipline to keep the main
development branch always (or nearly so) in a releasable state, but in my
opinion we are already doing a pretty good job of that with our policies
for code review and unit tests.  (We could, of course, do better about
monitoring the extended tests, run checker, and the like, but when we do
have regressions getting them fixed is not an invasive mess.)

To list some examples:

FreeBSD aims for new major ("dot zero") releases every two years.
MIT krb5 puts out new releases annually.
Google Chrome puts out releases every 6 weeks.
[Okay, I haven't exactly moved OpenAFS over to this schedule yet, but I did
just today cut a 1.9.0 and am going to try to put out regular 1.9.x
versions.]

I would urge the OTC (and OMC) to be careful around the pitfalls of making
the perfect the enemy of the good, and be willing to accept some level of
"uncleanliness" in the interest of getting all the good things we have
already done out there in production releases.  (And also trying to not
slip the published schedule too much.)  It is unpleasant, yes, but
sometimes it is the best choice overall.

Thanks,

Ben


Re: Freeze?

2020-09-25 Thread Benjamin Kaduk
On Thu, Sep 24, 2020 at 12:33:56PM +1000, Dr Paul Dale wrote:
> I’m seeing quite a bit of activity going on which isn’t related to the 
> 3.0beta1 milestone.
> We’re well past the cutoff date announced for new features in the code.
> 
> Should we be limiting the “new” stuff going in?
> 
> I’m fine with bug fixes, they make sense.  I’m fine with the list of beta1 
> pull requests continuing.
> It’s the rest that is more concerning.
> 
> Does anyone else have a similar view?

I think we should probably avoid putting in large or potentially
destabilizing changes, but don't see much reason to put a total freeze in
place (even with your listed exceptions).

-Ben


Re: Backports to 1.1.1 and what is allowed

2020-06-21 Thread Benjamin Kaduk
Hi Tim,

On Sat, Jun 20, 2020 at 10:11:04AM +1000, Tim Hudson wrote:
> I suggest everyone takes a read through
> https://en.wikipedia.org/wiki/Long-term_support as to what LTS is actually
> meant to be focused on.

You may have hit on a key aspect of how we are disconnected, here.

I was under the impression that (as is the case for many OSS projects), the
fact that 1.1.1 is an LTS release means that we enter a separate "LTS mode"
at the "beginning of a long-term support period" (as Wikipedia puts it) but
that there is some period prior to the start of the long-term support
period for which the STS policies apply.

So, are you considering that 1.1.1 is now, and has always been, in LTS mode
because it is marked as an "LTS release"?  Or is there a separate STS
period before it transitions to "LTS mode"?

> What you (Ben and Matt) are both describing is not LTS but STS ... these
> are different concepts.

AFAICT the thread started with just talk of merging to stable release
branches; LTS didn't come up until Kurt's note.  I certainly was not
considering the LTS policy in my earlier comments.

> For LTS the focus is *stability *and *reduced risk of disruption *which
> alters the judgement on what is appropriate to put into a release.
> It then becomes a test of "is this bug fix worth the risk" with the general
> focus on lowest possible risk which stops this sort of thing happening
> unless a critical feature is broken.
> 
> All of the "edge case" examples presented all involve substantial changes
> to implementations and carry an inherent risk of breaking something else -
> and that is the issue.

IMO, if we're in LTS mode, if any of the proposed edge cases came up that
would indicate that we failed at the LTS policy in the first place.

> It would be different if we had a full regression test suite across all the
> platforms and variations on platforms that our users are operating on - but
> we don't.
> 
> We don't compare performance between releases or for any updates in our
> test suite so it isn't part of our scope for release readiness - if this is
> such a critical thing that must be fixed then it is something that we
> should be measuring and checking as a release condition - but we don't -
> because it actually isn't that critical.

It can translate to real monetary impact in some cases (e.g., at scale).
But I can't directly refute your point, it is true.

-Ben


Re: Backports to 1.1.1 and what is allowed

2020-06-19 Thread Benjamin Kaduk
On Fri, Jun 19, 2020 at 11:46:15PM +0100, Matt Caswell wrote:
> 
> 
> On 19/06/2020 23:34, Tim Hudson wrote:
> > 
> > 
> > On Sat, 20 Jun 2020, 8:14 am Benjamin Kaduk,  > <mailto:ka...@mit.edu>> wrote:
> > 
> > On Sat, Jun 20, 2020 at 08:11:16AM +1000, Tim Hudson wrote:
> > > The general concept is to only fix serious bugs in stable releases.
> > > Increasing performance is not fixing a bug - it is a feature.
> > 
> > Is remediating a significant performance regression fixing a bug?
> > 
> > 
> > It would be a bug - but not a serious bug. So no.
> > It works. It was released. 
> 
> I don't recall us ever saying that we would only fix serious bugs.
> AFAIK, if its a bug then we will fix it. IMO a serious performance
> regression would qualify, within reason (e.g. major rewrites of the
> assembler would not be ok).
> 
> > Wholesale replacement of implementations of algorithms should not be
> > happening in LTS releases.
> 
> This I agree with.

Both of Matt's paragraphs match up with my understanding/sentiment.

-Ben


Re: Backports to 1.1.1 and what is allowed

2020-06-19 Thread Benjamin Kaduk
On Sat, Jun 20, 2020 at 08:11:16AM +1000, Tim Hudson wrote:
> The general concept is to only fix serious bugs in stable releases.
> Increasing performance is not fixing a bug - it is a feature.

Is remediating a significant performance regression fixing a bug?

-Ben


Re: Backports to 1.1.1 and what is allowed

2020-06-16 Thread Benjamin Kaduk
On Tue, Jun 16, 2020 at 02:03:58PM +0100, Matt Caswell wrote:
> PR 11188 proposes to backport a series of s390x patches to the 1.1.1
> branch. IIUC it includes performance improvements as well as support for
> new hardware instructions.
> 
> I think we need to have a much clearer and more explicit policy about
> exactly what is allowed to be backported to a stable branch. For example
> PR #11968 was a significant recent PR that backported AES CTR-DRBG
> performance improvements to the 1.1.1 branch and was allowed (should it
> have been?).

I am happy to see 11968 landed; some informal testing that I hope to make
more formal soon seems to show a ~20% slowdown/regression for large RNG
requests going from 1.1.1d to 1.1.g, and 11968 provided substantially more
significant of a speedup (again, very informal testing, though).

In this case you could say that the "bug" is that we're only calling AES on
a block at a time despite having much more effective backends available for
multi-block calls.

> We have always said that the stable releases should only receive bug and
> security fixes. Performance improvements have been a bit of a grey area,
> e.g. a few lines of code that significantly improves the performance of
> a particular algorithm or codepath could reasonably be justified as
> "fixing a performance bug". OTOH bringing in whole new files of
> assembler seems to go way beyond that definition.

There's probably some semi-subtle distinction to make along the lines of
"making the algorithm more efficient" vs "using a new algorithm, that
happens to run faster", with the latter counting as a feature.

> However, when I tried to find a reference to the policy which says
> exactly what we are allowed to backport I could find one. Do we have
> such a thing?
> 
> In any case I think we should develop a much more explicit policy that
> will enable us to decide whether PRs such as 11188 are reasonable or
> not. See for example Ubuntu's Stable Release Updates policy:

I agree that having something written down will be very useful, even if
just as a fixed benchmark against which to think about how exceptional any
given "exception case" is where we want to deviate from it.

-Ben

> https://wiki.ubuntu.com/StableReleaseUpdates
> 
> 
> Matt


Re: addrev warning

2020-06-08 Thread Benjamin Kaduk
On Mon, Jun 08, 2020 at 12:38:57PM +0200, Richard Levitte wrote:
> Yes.
> 
> I propose telling git to shut up, i.e. FILTER_BRANCH_SQUELCH_WARNING=1
> Our use of git-filter-branch isn't one that will generate a gotcha, or
> we would have seen that a long time ago...  in other words, we treat
> it gently.

I just patched around it locally (in this manner) when it showed up in
debian unstable.  I agree that we're not using filter-branch in a way that
is likely to cause problems.

-Ben


Re: Alpha1

2020-04-21 Thread Benjamin Kaduk
On Tue, Apr 21, 2020 at 11:10:19AM +0100, Matt Caswell wrote:
> The 3.0 developers met via conference call this morning. All the
> functionality that we had planned for alpha 1 has now been merged, so we
> are now thinking that we will do the alpha 1 release on Thursday this
> week. That would imply a repo freeze tomorrow.
> 
> Thoughts/opinions/objections to this proposal?

Given that the list of required things for alpha 1 are done, it does seem
appropriate.  I know of a couple things that would be bug reports against
an alpha1 if produced right now, but ... what is an alpha for, if not to
trigger people to look and file bug reports? :)

-Ben


Re: Improving X.509 certificate validation errors

2020-03-25 Thread Benjamin Kaduk
Hi Martin,

Hopefully this response is still useful a few weeks later.

On Thu, Mar 05, 2020 at 04:10:10PM +0100, Martin Ukrop wrote:
> Hi,
> 
> I’m the lead of a university project investigating (and improving) the
> usability of certificate validation errors. Our goal is to simplify the
> ecosystem by consolidating the errors and their documentation in one place,
> providing replicable example certificates for all validation errors and by
> explaining better what the individual errors mean. The project is live at
> https://x509errors.org/
> 
> Now we are reaching out to library developers and users (you) to ask for
> feedback.
> 
> Currently, we base the system on OpenSSL errors (as it’s the most common).
> We have example certificates for 30+ OpenSSL errors and in-progress mapping
> for corresponding errors error for OpenSSL, GnuTLS, Botan and MbedTLS.
> In the future, we plan the possibility of web reorganization based on the
> other libraries (currently, the web is organized by OpenSSL), adding the
> error frequencies based on IP-wide scans and elaborating on the
> consequences of individual errors.
> Ultimately, we want to propose better (ideally user-tested) errors and
> their documentation. (Just recently, we made a survey among 180 developers
> regarding their error documentation preference with good reception).
> 
> As developers/users of TLS libraries, what do you think of the idea?
> * Which part(s) do you find the most/least useful?
> * Is there anything you see missing?
> * What are your thoughts on unifying the error taxonomy? (a very long-term
> goal, if at all possible)

I tihnk it's an interesting idea.  To me, perhaps the most valuable part
would be to accumulate a corpus of certificates/chains that are malformed
or fail to validate due to a wide variety of errors, almost akin to a
fuzzing corpus.  I'd also be curious (though I'm not entirely sure how
large a practical impact it would have) to perform a clustering analysis
across different X.509 implementations and see if different implementations
produce different distributions of errors.  (That is, we might expect each
implementation to have an error for "not valid yet", "expired", "missing
required ASN.1 field", etc.; each implementation will have a different
error string, of course, but if we group all certificates that produce the
same error with the same implementation together, we have a bunch of
different clusters.  Repeating the clustering across all implementations
lets us compare the different distributions, and examine certificates that
end up in a different cluster in different implementations.)
I also like the idea of getting a sense for which types of errors are "most
common", though it will probably require some care to construct the
sample population for the experiment so that the results have interprative
value.  Those results might (depending on what they are) be used as input
to "best practice" guides for (e.g.) making a local PKI.

> During spring, we would like to start creating pull requests improving the
> documentation and error messages in some of the libraries. Would you
> welcome such contributions?

It's probably best to make sure everyone agrees what is meant by
"improving" before doing much writing work; a github issue would be a fine
place to discuss that topic.  I expect that our current error messages are
suboptimal, though we will have to keep in mind during any attempt to
change them that in some cases there will be more value in keeping things
stable than in making them better messages in isolation -- as an
open-source project, it's hard for us to know with confidence which of our
behaviors people are relying on.

-Ben


Re: Deprecation

2020-02-14 Thread Benjamin Kaduk
On Fri, Feb 14, 2020 at 05:59:32PM +0100, Tomas Mraz wrote:
> On Fri, 2020-02-14 at 16:17 +, Salz, Rich wrote:
> > I think we should delay the deprecation of engine stuff to 4.0.
> > Personally I don't have sense of stability of provider API.
> >  
> > I share this concern.  This is the first release of the provider
> > infrastructure, and while it will be known to work for FIPS modules,
> > it’s hubris to think OpenSSL will get it completely right for other
> > uses.
> >  
> > All other deprecations point to existing API’s or, if they are brand
> > new, are not a lot of code/work to implement them.  The provider
> > interface is both brand new and significant work.  Deprecating and
> > saying “use providers” without at least one release cycle of 
> > providers, seems premature.
> 
> This is an argument for not removing engines in 4.0 (or earlier than
> one major release after the provider interface fully stabilizes and
> proves viable). However I do not think that it is argument for not
> deprecating it. Deprecation is declaration of intent and not a way to
> force people stop using the API immediately.
> 
> How is the provider API going to improve/stabilize, if the 3rd party
> engine suppliers will not get the the message that the engine API is
> eventually going away in future and start writing providers as
> replacement.

I have similar feelings to Tomas.

Note that the current policy's test for "is removal allowed" is a
two-pronged test, and if the next release after 3.0 is 4.0, we would not be
allowed to remove engines in 4.0, since providers (the "engine
replacement") will not have been available in a (previous) LTS release.

If we know that something will need to go away eventually, it seems like we
can use deprecation annotations to publicize that fact, even if the
eventual removal will be delayed for quite some time due to other
considerations.

-Ben


Re: Legacy provider

2020-01-15 Thread Benjamin Kaduk
On Thu, Jan 16, 2020 at 06:57:49AM +1000, Dr Paul Dale wrote:
> I’m not sure what more I can write.
> 
> I proposed the vote text around the time I sent the notification here: no 
> comments.
> I created the vote, early in the voting period, the clarification was sought 
> and made.
> All OMC members registered their vote and the vote closed early.
> 
> The criteria for being valid as per the bylaws 
>  was met.  As votes go, 
> this one was quick taking two days of the two weeks.
> 
> Abstentions are frequent in votes for a number of reasons.
> The reasons each person uses are not revealed and not asked for.

Thank you for the quick response; I understand there's not anything more to
be said.

-Ben


Re: Legacy provider

2020-01-15 Thread Benjamin Kaduk
Hi Pauli,

On Tue, Jan 14, 2020 at 09:34:40PM +1000, Dr Paul Dale wrote:
> The OMC vote is closed.
> 
> The vote text being:
> 
> The legacy provider should be disabled by default in 3.0
> 
> With the clarification that "disabled" in this context means "not loaded”.
> 
> The vote passed (two for, one against, four abstain)

It's good to have a decision here, but I'm kind of worried about the four
abstains -- it's easy for me to leap to a conclusion that the individuals
in question just didn't want to to spend the time to come to a considered
position, even though this issue has substantial potential impact for our
userbase.  I'm trying to not make faulty assumptions, so some greater
clarity on the circumstances would be helpful, if possible.

Thanks,

Ben


Re: Build failures on master?!

2019-12-21 Thread Benjamin Kaduk
On Sun, Dec 15, 2019 at 08:25:02PM +, Dr. Matthias St. Pierre wrote:
> Gentle reminder: it's almost a month since a started this thread, but the 
> external pyca and krb5 
> tests are still failing. Apart from complicating the review process, it also 
> happens that people

With apologies for being a week behind on mail, but I didn't see any
commits in the past week that look like they were targetted to fix external
tests, and I see (E.g.)
https://travis-ci.org/openssl/openssl/jobs/628003784?utm_medium=notification_source=github_status
succeeding (as well as my local build with the krb5 tests).  Are the
external tests still failing?

Thanks,

Ben


Re: Check NULL pointers or not...

2019-11-28 Thread Benjamin Kaduk
On Wed, Nov 27, 2019 at 10:38:41AM +0100, Richard Levitte wrote:
> Depending on who you're asking, you get different responses.
> 
> The topic is: should we check pointers coming from applications before
> using them or not?
> 
> There are two positions on this:
> 
> 1. Yes we should so we don't crash processes unnecessarily, and we
>should return an error if pointers are NULL (unless that's valid
>input, such as for the freeing functions).  There is a generic
>error reason code for it, ERR_R_PASSED_NULL_PARAMETER.
> 
> 2. No we should not, as we don't cater for user programming errors.
>Also, it allows us to catch our own errors early.
> 
> For a while, our pendulum was going with option 2, but lately, we seem
> to go back to option 1.
> 
> Both options have their pros and cons:
> 
> 1. cons: there's the very real possibility that users don't check for
>  errors as they should, and things go wrong further on,
>  sometimes in ways where the actual cause is very hard to
>  figure out.
>pros: programs will not spuriously crash because we didn't catch an
>  internal corner case in our tests.
> 
> 2. cons: programs may crash, sometimes due to our own mistakes,
>  sometimes due to user errors.
>pros: some very subtle bugs are found, either quicker or at all.
>  An actual case is PR 7630 [1], which was uncovered because
>  there was a crash due to a NULL being used (this was
>  ultimately due to users not checking errors).
> 
> As a middle ground, and perhaps a way to satify both camps, we could
> use ossl_assert() in our checks for input NULL, as follows:
> 
> if (!ossl_assert(ptr != NULL)) {
> ERR_raise(ERR_LIB_WHATEVER, ERR_R_PASSED_NULL_PARAMETER);
> return 0;
> }
> 
> This means that in a debug build, there may be crashes due to pointers
> being NULL, while in a release build, the caller gets an error.
> 
> Thoughts?

My recollection was that we had some reasonable support for this "middle
ground" approach.  I don't personally have strong enough feelings to
advocate a specific position, and will try to adhere to the group consensus
going forward.

-Ben


Re: Build failures on master?!

2019-11-21 Thread Benjamin Kaduk
On Mon, Nov 18, 2019 at 10:12:52PM +, Matt Caswell wrote:
> 
> 
> On 18/11/2019 21:48, Dr. Matthias St. Pierre wrote:
> > The last 19 commits on https://github.com/openssl/openssl/commits/master,
> > starting from Nov 14 have a red cross from the CIs. What's going on again?
> > My personal impression is that builds on master are failing 50% of the time.
> 
> The builds on master have been failing for *much* longer than that. They
> should be succeeded on the PRs, but its the extended tests that fail.
> 
> > 
> > It is really tedious to check pull requests for build errors just to find 
> > that those errors
> > are well known failures from master. In particular, the krb5 an pyca tests 
> > are notoriously
> > failing. Are there any plans to fix them or disable them?
> 
> The build failures mostly come from the external tests (or that was the
> case I haven't checked recently). The problem with these is that, on
> occasion we need the maintainers of these packages to help us track down
> a problem. Or in some cases we actually need them to make a change (for
> example because something changed on master which revealed that they
> were relying on some behaviour in master which they really shouldn't be
> relying on).
> 
> Once one of them is failing then multiple errors can build up and they
> get masked by the earlier failure.
> 
> I'll try and find some time to take another pass at the current
> failures. I suspect the answer might be to disable some of the external
> tests.

Please let me know if you want me to look at the krb5 tests (or pester
folks at upstream there to do so).  I'm in Singapore for the IETF this week
and have been behind on, well, everything.

-Ben


will there be a final "wrap-up" 1.1.0l release?

2019-08-28 Thread Benjamin Kaduk
Hi all,

I note that 1.1.0 goes EOL on 2019-09-11, and the current 1.1.0k release is
from the end of May.  On the normal 3-ish-month cycle for letter releases
(to "roll up" accumulated changes in the absence of a security release), we
would be having one soon.  Are there any plans to do a final letter release
to package up all the changes before 1.1.0 goes EOL?  (If we do, will there
also be "roll up" letter releases on the other branches?)

Thanks,

Ben


Re: Do we really want to have the legacy provider as opt-in only?

2019-07-19 Thread Benjamin Kaduk
On Mon, Jul 15, 2019 at 02:19:22PM +0100, Matt Caswell wrote:
> 
> 
> On 15/07/2019 13:58, Tomas Mraz wrote:
> 
> > 
> > I understand that for the current digest algos implemented in the
> > legacy provider the problem might not be as pressing as these
> > algorithms are not widely used however which other algorithms are going
> > to be moved into the legacy provider?
> 
> My guess is that the ones likely to give us the most problems would be DES, 
> DSA
> and RC4

To add a bit of anecdata, Debian and Fedora are removing DES support from
(MIT) krb5.  So far all we've seen as bug reports are that the kernel may
still have that enctype in its list to use for NFS (as well as other,
still-useful, ones), and so we need to ignore it instead of bailing.
But given that it provides only ca. $20 of protection, it's not especially
surprising that we aren't seeing much using it.

On the other hand, krb5 is not going around and disabling RC4, even though
RFC 8429 is a thing.

-Ben


Re: Do we really want to have the legacy provider as opt-in only?

2019-07-19 Thread Benjamin Kaduk
On Tue, Jul 16, 2019 at 03:06:28PM -0400, Viktor Dukhovni wrote:
> On Mon, Jul 15, 2019 at 02:27:44PM +, Salz, Rich wrote:
> 
> > >>DSA
> > > 
> > > What is the cryptographic weakness of DSA that you are avoiding?
> > 
> > It's a good question. I don't recall the specific reason why that was 
> > added to
> > the list. Perhaps others can comment.
> > 
> > The only weakness I know about is that if you re-use the nonce, the private
> > key is leaked. It's more brittle than RSA-PKCS, but not as flawed as RC4.
> > 
> > I think this should be removed from the "legacy" list unless someone can 
> > point out why it's like the others in the list.
> 
[...]
> 4.  As mentioned key disclosure is more likely than with RSA.

Huh, and it looks like we don't even implement deterministic DSA (RFC
6979) which is a partial mitigation.

-Ben


Re: Issues and pull requests are largely getting ignored

2019-03-26 Thread Benjamin Kaduk
On Tue, Mar 26, 2019 at 02:20:28PM +0100, Kurt Roeckx wrote:
> On Tue, Mar 26, 2019 at 09:53:22AM +, Matt Caswell wrote:
> > 
> > So the real problem there is a mismatch between the opening rate and the 
> > closing
> > rate, i.e. it is NOT that we are ignoring these issues. I see it more as a
> > resource issue - we are seeing issues raised at a greater rate than we have
> > resources to handle.
> 
> Or that the resources we do have are spend on the wrong thing.

One might, perhaps, try to measure distribution of time between
[pull-request/issue] submission and the first comment on it.
Consider
https://github.com/openssl/openssl/pull/8108
and
https://github.com/openssl/openssl/pull/7943 (both submitted by users that
are now project members), that have both been open for months and still
have no comments!

> > I can't do the same analysis for PRs because github doesn't detect properly 
> > when
> > we merge PRs (because we don't use the github facility for doing that). I 
> > would
> > expect to see somewhat similar numbers though.
> 
> The amount of open pull requests has been around 200 for the last
> few months, so it seems we can just keep up with that currently,
> but it used to be around 100.
> 
> So I think that in general we do not have enough resources to keep up
> with both the pull requests and issues.

Not if we want to keep up the pace of new development towards the goals for
the next major release, I agree.
(Not that I am doing much to contribute, myself, at this point -- my AD
role in the IETF keeps me quite busy.)

-Ben


Re: Thoughts on OSSL_ALGORITHM

2019-03-23 Thread Benjamin Kaduk
I also like the provider data approach.

-Ben

On Sat, Mar 23, 2019 at 09:11:23AM +1000, Dr Paul Dale wrote:
> I’ve no issue having a provider data field there.  It will be useful for more 
> than just this (S390 AES e.g. holds data differently to other 
> implementations).
> 
> I also don’t think forcing separate functions is a big problem — most 
> providers will only implement one or two algorithm families which will help 
> control the redundancy.
> 
> I don’t think we should be doing a string lookup every time one of these is 
> called.
> 
> 
> Of the three, the provider data feels clean and unique functions fast.
> 
> I’d like to avoid mandating another level of indirection (it’s slow), which 
> is a risk with provider data.
> 
> 
> My thought: add the provider data field.  Use that when it can be done 
> directly, use unique functions otherwise.
> The example with key and iv lengths would be a direct use.  Code that dives 
> through a function pointer or a switch statement would be an example of not.
> 
> 
> 
> Pauli
> -- 
> Dr Paul Dale | Cryptographer | Network Security & Encryption 
> Phone +61 7 3031 7217
> Oracle Australia
> 
> 
> 
> > On 23 Mar 2019, at 1:45 am, Matt Caswell  wrote:
> > 
> > Currently we have the OSSL_ALGORITHM type defined as follows:
> > 
> > struct ossl_algorithm_st {
> >const char *algorithm_name;  /* key */
> >const char *property_definition; /* key */
> >const OSSL_DISPATCH *implementation;
> > };
> > 
> > I'm wondering whether we should add an additional member to this structure: 
> > a
> > provider specific handle. i.e.
> > 
> > struct ossl_algorithm_st {
> >const char *algorithm_name;  /* key */
> >const char *property_definition; /* key */
> >const OSSL_DISPATCH *implementation;
> >void *handle;
> > };
> > 
> > The reason to do this is because providers are likely to want to share the 
> > same
> > implementation across multiple algorithms, e.g. the init/update/final 
> > functions
> > for aes-128-cbc are likely to look identical to aes-256-cbc with the only
> > difference being the key length. A provider could use the handle to point to
> > some provider side structure which describes the details of the algorithm 
> > (key
> > length, IV size etc). For example in the default provider we might have:
> > 
> > typedef struct default_alg_handle_st {
> >int nid;
> >size_t keylen;
> >size_t ivlen;
> > } DEFAULT_ALG_HANDLE;
> > 
> > DEFAULT_ALG_HANDLE aes256cbchandle = { NID_aes_256_cbc, 32, 16 };
> > DEFAULT_ALG_HANDLE aes128cbchandle = { NID_aes_128_cbc, 16, 16 };
> > 
> > static const OSSL_ALGORITHM deflt_ciphers[] = {
> >{ "AES-256-CBC", "default=yes", aes_cbc_functions,  },
> >{ "AES-128-CBC", "default=yes", aes_cbc_functions,  },
> >{ NULL, NULL, NULL }
> > };
> > 
> > Then when the "init" function is called (or possibly at newctx), the core 
> > passes
> > as an argument the provider specific handle associated with that algorithm.
> > 
> > An alternative is for the provider to pass the algorithm name instead, but 
> > this
> > potentially requires lots of strcmps to identify which algorithm we're 
> > dealing
> > with which doesn't sound particularly attractive.
> > 
> > A second alternative is for each algorithm to always have unique functions
> > (which perhaps call some common functions underneath). This sounds like 
> > lots of
> > unnecessary redundancy.
> > 
> > Thoughts?
> > 
> > Matt
> 


Re: [openssl-project] OpenSSL 3.0 and FIPS Update

2019-02-14 Thread Benjamin Kaduk
On Wed, Feb 13, 2019 at 03:28:30PM -0500, Michael Richardson wrote:
> 
> Matt Caswell  wrote:
> > Please see my blog post for an OpenSSL 3.0 and FIPS Update:
> 
> > https://www.openssl.org/blog/blog/2019/02/13/FIPS-update/
> 
> Thank you, it is very useful to have these plans made up front.
> I think your posts should probably explain what happened to 2.x, and if this
> represents a move towards semantic versioning. (I think it does...?)
> 
> In the various things linked, in particular:
>https://www.openssl.org/docs/OpenSSL300Design.html
> 
> I think that there is a missing box.  Specifically, the PERL API wrappers

Later on you seem to clarify that by "PERL API" you mean "Net::SSLeay" but
I just want to double-check.

> that are used in the test bench.  I believe that the "applications" are
> a serious problem as there are (in 1.1.1) still many things that are very
> difficult (sometimes, it seems, impossible) to do programmatically, and which
> the test cases actually simply shell out to the application to do.
> An example of this is adding certain extensions to a certificate when
> generating it, which is only really possible by loading pieces of
> configuration file in.

It might we worth raising github issues to point out these omissions.

> So what I'd like to see is to remove many of the applications from the core
> of OpenSSL, put them into a seperate package using better-documented API
> calls.  Let them evolve according their own time-scale, probably taking
> patches faster without disrupting the underlying libraries.

In the vein of your later comment about "20 years of twisty code", I'm
actually somewhat inclined to clave off the applications and leave them
alone, so that people needing bug-for-bug compatibility can keep it.  Then
we could rewrite the functionality in question with a more maintainable
structure in a way that would also function as examplar API usage.

> My observation is that the Perl testing system is used to drive the tests,
> but the tests do not actually use the Perl API wrapper for OpenSSL, but
> rather rely on the vast number of .c files in test/*.
> In other (more purely agile) projects, the test cases often serve as
> documentation as to how to use the API.  In OpenSSL, the test cases
> rely too much on the openssl "applications", and the API is hidden.

So, when I write new code that requires tests (which is basically all new
code, try as I may to convince myself otherwise), the first thing I try to
do is put in an API-only test that's a "standalone" executable (ignore
sslapitest.c which is not standalone but functionally is so) to exercise
narrowly the functionality in question.  Only if it's a big hassle,
requires additional (certificate) input and/or configuration files, etc.,
do I fail over to using the "application" as the testbed.  There's a
tradeoff, and while the current playing field often biases that tradeoff in
a direction I'd rather not go, changing the playing field is more work than
I have time to take on while a sitting IETF AD.

> This would involve adopting some or all of Net::SSLeay.
> While there would be some initial duplication of effort, I think that over
> time it would sort itself out.  Perl is no longer as cool as it used to be (I
> still like it) and maybe someone would argue for Python3 or something, and
> frankly I don't care which.

I don't think that OpenSSL itself should adopt Net::SSLeay, even though we
do seem to have one of the higher perl usage fractions of the open-source
projects I interact with.  We have enough on our hands with the core C
libraries to modernize.

> What I care about is that the test cases actually test the API, rather than
> depend upon 20 years of twisty code in the "applications".
> And that the applications are permitted to grow/change/adapt to people's
> needs, rather than living in a hard spot between developer needs and end
> user needs, pissing off both groups.

I agree that mixing user needs and developer needs is a recipe for sadness.
I'm not sure that actually tearing them apart from the current "apps" is
something that we can realistically do while preserving our goal of
cross-release stability.

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] Proposed vote text for the SSL_CB_HANDSHAKE_START change

2019-01-30 Thread Benjamin Kaduk
On Wed, Jan 30, 2019 at 09:02:30AM +0100, Kurt Roeckx wrote:
> On Tue, Jan 29, 2019 at 02:07:09PM +, Matt Caswell wrote:
> > So I plan to start the vote soon for merging PR#8096 and backporting it to
> > 1.1.1. This is a breaking change as previously discussed.
> > 
> > My proposed vote text is as follows. Please let me know asap of any 
> > feedback.
> > Otherwise I will start the vote soon.
> > 
> > "master and 1.1.1 will be updated to use SSL_CB_POST_HANDSHAKE_START and
> > SSL_CB_POST_HANDSHAKE_END to signal the start and end of a post handshake
> > message exchange in the info callback (replacing SSL_CB_HANDSHAKE_START and
> > SSL_CB_HANDSHAKE_END)."
> 
> So my proposal would be: Don't call the callback for post
> handshake messages. (It could use some rewording.)

That does have a fair bit of appeal to it, though perhaps not enough to
justify the breaking change in 1.1.1.

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] Proposed vote text for the SSL_CB_HANDSHAKE_START change

2019-01-29 Thread Benjamin Kaduk
On Tue, Jan 29, 2019 at 01:27:24PM -0600, David Benjamin wrote:
> On Tue, Jan 29, 2019 at 11:31 AM Kurt Roeckx  wrote:
> 
> > On Tue, Jan 29, 2019 at 02:07:09PM +, Matt Caswell wrote:
> > > So I plan to start the vote soon for merging PR#8096 and backporting it
> > to
> > > 1.1.1. This is a breaking change as previously discussed.
> > >
> > > My proposed vote text is as follows. Please let me know asap of any
> > feedback.
> > > Otherwise I will start the vote soon.
> > >
> > > "master and 1.1.1 will be updated to use SSL_CB_POST_HANDSHAKE_START and
> > > SSL_CB_POST_HANDSHAKE_END to signal the start and end of a post handshake
> > > message exchange in the info callback (replacing SSL_CB_HANDSHAKE_START
> > and
> > > SSL_CB_HANDSHAKE_END)."
> >
> 
> What exactly is a post-handshake message exchange? Do the NewSessionTicket
> sent by the server at the beginning count as the part of the handshake? Are
> they each separate post-handshake exchanges? Are all of them together one
> exchange? Conversely, what happens when you receive that NewSessionTicket
> as a client?

I don't think we should try to get into the business of demarcating the
start and end of post-handshake exchanges.  (In particular, the
NewSessionTickets are formally not grouped with anything, whether the
initial handshake or each other.)

> When you send a KeyUpdate with key_update_requested, is the reply you
> expect part of the exchange or separate? What if the peer coalesced them to
> avoid DoS problems? Conversely, if you receive a KeyUpdate with
> key_update_requested, is your reply part of the exchange? What if you
> coalesced them to avoid DoS problems?
> 
> What if I send a CertificateRequest, but the other side sends me a
> KeyUpdate with key_update_requested before responding with Certificate, so
> I respond with my own KeyUpdate? What and how many exchanges are there?
> 
> Is it important that both sides agree what an "exchange" is?
> 
> What I'm getting at here is that "post-handshake exchange" does not seem to
> be a meaningful construct to the protocol. I would thus advocate not
> signaling START/END things to begin with. That way, if TLS 1.4 comes by and
> shuffles around again, we don't repeat this adventure.

+1

> I think one clear conclusion from this incident is that this sort of
> low-level API should be avoided, or people will use them in finicky ways
> that break unexpectedly when you change things. Better defer such
> mechanisms to when concrete use cases come up, and then implement direct
> APIs for those use cases, like SSL_OP_NO_RENEGOTIATION. (If info_callback
> hadn't existed, OpenSSL would hopefully have learned sooner that
> SSL_OP_NO_RENEGOTIATION was important, added it earlier, and avoided
> today's TLS 1.3 KeyUpdate intolerance in the ecosystem.)
> 
> 
> > This will only cover the key update currently? Does that come with
> > some parameter telling which kind of handshake is happening? If
> > not, is it more useful to just say that a key update is happening?
> >
> 
> There's already a message callback. Why not just use that? It's unclear to
> me why anyone would want to know when KeyUpdates happen anyway, aside from
> low-level logging and debugging. The message callback works fairly well for
> such things.

+1

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] inline functions

2019-01-28 Thread Benjamin Kaduk
On Mon, Jan 28, 2019 at 07:10:55AM +0100, Richard Levitte wrote:
> On Mon, 28 Jan 2019 06:17:35 +0100,
> Dr Paul Dale wrote:
> > Richard wrote:
> > 
> > Not really, since they are static inline. This is by design, that for 
> > any file you want to use
> > a safestack in, you just start with a DEFINE_ line. The mistake we did 
> > was to leave a few
> > common ones in the safestack header file. (same thing for lhash) 
> > 
> > Which means we’ve a compatibility issue.  The functions are in a public 
> > header, they can be used
> > by any application.  We need to continue supporting such use.
> > Asking a user to add a DEFINE_ line is API breaking.
> > 
> > I would be pro making such a change but we’d need to accept the 
> > consequences.
> 
> We have to accept consequences either way, either:
> 
> 1. the surprise breakage if someone includes  but
>doesn't link with libcrypto, while compiling with
>-fkeep-inline-functions (explicitly or implicitly, depending on the
>compiler)

This one is only "surprising and new" the first time a user/project tries
to turn on -fkeep-inline-functions.

> 2. The controlled and documented change / breakage that they will have
>to either add those DEFINE lines where they need the functionality,
>or include another header file with common stack / lhash type
>implementations (with the caveat that they MUST link with libcrypto
>if they use those headers)

This one is "surprising and new" to everyone using the stuff (i.e., more
people).

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project

Re: [openssl-project] QUIC, again

2018-11-12 Thread Benjamin Kaduk
Between last time we discussed it and now, waiting seems to have been
prudent, as the TLS/QUIC interaction got significantly revamped.
The current QUIC drafts have TLS exporting key material and plaintext
handshake messages, with QUIC record protection used on the wire and not
TLS record protection.  There is a huge amount of interest in QUIC at the
IETF, and we will need to support it eventually.  But that may be best as
limited to exposing the needed APIs and not necessarily pulling in a full
QUIC implementation -- I haven't thought about that question very much.

I don't think I would have the team as a whole prioritize QUIC over FIPS,
though it may be worth someone taking an initial look at what would be
needed.

-Ben

On Mon, Nov 12, 2018 at 11:34:20AM +0100, Richard Levitte wrote:
> For those wanting to follow what's happening in QUIC space, this is a
> good place to start: https://datatracker.ietf.org/wg/quic/about/
> 
> In message <20181112.113323.260349601387601601.levi...@openssl.org> on Mon, 
> 12 Nov 2018 11:33:23 +0100 (CET), Richard Levitte  said:
> 
> > QUIC was mentioned a little more than a year ago.   Since then, it
> > seems that the drafts have moved forward with quite some speed:
> > 
> > https://tools.ietf.org/html/draft-ietf-quic-transport-16
> > https://tools.ietf.org/html/draft-ietf-quic-tls-16
> > https://tools.ietf.org/html/draft-ietf-quic-recovery-16
> > 
> > There seems to be an effort to have the next major HTTP version be
> > based on QUIC, at least if this blog is any indication:
> > 
> > https://daniel.haxx.se/blog/2018/11/11/http-3/
> > 
> > So the question is, should we start taking a closer look?  Last time,
> > it seems like the discussions were cautiously positive, but never
> > reached a conclusion.
> > 
> > Thoughts?  Anyone feeling enthusiastic and want to do something?
> > 
> > Cheers,
> > Richard
> > 
> > -- 
> > Richard Levitte levi...@openssl.org
> > OpenSSL Project http://www.openssl.org/~levitte/
> > 
> ___
> openssl-project mailing list
> openssl-project@openssl.org
> https://mta.openssl.org/mailman/listinfo/openssl-project
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] FYI: [postfix & TLS1.3 problems]

2018-10-11 Thread Benjamin Kaduk
I would guess that the misbehaving clients are early openssl betas
that receive the real TLS 1.3 version and then try to interpret
as whatever draft versino they actually implemnet.

-Ben

On Thu, Oct 11, 2018 at 01:18:03PM -0400, Viktor Dukhovni wrote:
> 
> Apparently, some SMTP clients set fallback_scsv when doing TLS 1.2
> with Postfix servers using OpenSSL 1.1.1.  Not yet clear whether
> they tried TLS 1.3 first and failed, or just sent the SCSV out of
> the blue...
> 
> See attached.  If this is a common problem, it might be useful to
> have a control that tolerates "downgrade" to TLS 1.2, without
> disabling TLS 1.3 support.  In many cases, and especially opportunitistic
> security, where STARTTLS can be stripped by an MiTM entirely, so
> we often can't even prevent downgrades to cleartext, TLS 1.2 is
> quite good enough.
> 
> -- 
>   Viktor.

> Date: Thu, 11 Oct 2018 12:53:38 -0400
> From: Viktor Dukhovni 
> To: postfix-us...@postfix.org
> Subject: Re: postfix & TLS1.3 problems
> User-Agent: Mutt/1.10.1 (2018-07-13)
> 
> On Thu, Oct 11, 2018 at 05:54:59PM +0200, A. Schulze wrote:
> 
> > today I noticed a significant amount of TLS failures in my postfix log.
> > 
> > Oct 11 17:43:35 mta postfix/smtpd[23847]: SSL_accept error from  
> > client.example[192.0.2.25]:34152: -1
> > 
> > I traced some sessions and found the problematic client is announcing  
> > the special cipher "TLS_FALLBACK_SCSV"
> > in a TLSv1.2 ClientHello message. Now, as my server support TLSv1.3,  
> > my SSL library (openssl-1.1.1) assume a downgrade attack an close the  
> > connection with an SSL error message "inappropriate fallback"
> > 
> > The core issue is a client with a nonconforming TLS implementation.
> 
> Any idea what software these clients are running?  Are they at all
> likely to fix this any time soon?
> 
> > To circumvent the problem I tried to disable TLS1.3 on my server by setting
> > smtpd_tls_protocols = !SSLv2,!SSLv3,!TLSv1.3
> > 
> > But that does not help.
> > The Client still fail an deliver the message by falling back to plain text 
> > :-/
> > 
> > The only option to force encrypted traffic again would be a library  
> > downgrade on my side.
> > Any other suggestions?
> 
> Support for OpenSSL 1.1.1 and TLS 1.3 is on the list of fixes slated
> for Postfix 3.4, and some may then be backported to patch levels
> of earlier releases.
> 
> In the meantime, try:
> 
> tls_ssl_options = 0x2000
> 
> which corresponds to SSL_OP_NO_TLSv1_3.  I am not aware of any
> method to accept the "downgrade" to TLS 1.2 without disabling TLS
> 1.3 for clients that do have correct implementations.
> 
> -- 
>   Viktor.

> ___
> openssl-project mailing list
> openssl-project@openssl.org
> https://mta.openssl.org/mailman/listinfo/openssl-project

___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] A proposal for an updated OpenSSL version scheme (v2)

2018-09-23 Thread Benjamin Kaduk
On Sat, Sep 22, 2018 at 01:02:29AM -0400, Viktor Dukhovni wrote:
> 
> 
> > On Sep 22, 2018, at 12:59 AM, Richard Levitte  wrote:
> > 
> > So in summary, do we agree on this, and that it's a good path forward?
> > 
> > - semantic versioning scheme good, we should adopt it.
> > - we need to agree on how to translate that in code.
> > - we need to stop fighting about history.
> 
> Yes.

+100.

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] A proposal for an updated OpenSSL version scheme (v2)

2018-09-23 Thread Benjamin Kaduk
On Sat, Sep 22, 2018 at 01:12:21AM -0400, Viktor Dukhovni wrote:
> 
> 
> > On Sep 22, 2018, at 12:50 AM, Tim Hudson  wrote:
> > 
> > The impact of the breaking change on anyone actually following our 
> > documented encoding cannot.
> > i.e. openssh as one example Richard pointed out.
> 
> The only use of OPENSSL_VERSION_NUMBER bits in OpenSSH (which is not yet 
> ported to
> 1.1.x upstream BTW, so hardly relevant really) is:

It seems that they have done the porting just in the past couple weeks:

482d23bcac upstream: hold our collective noses and use the openssl-1.1.x
48f54b9d12 adapt -portable to OpenSSL 1.1x API
86e0a9f3d2 upstream: use only openssl-1.1.x API here too
a3fd8074e2 upstream: missed a bit of openssl-1.0.x API in this unittest
cce8cbe0ed Fix openssl-1.1 fallout for --without-openssl.

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] coverity defect release criteria (Fwd: New Defects reported by Coverity Scan for openssl/openssl)

2018-09-09 Thread Benjamin Kaduk
On Sun, Sep 09, 2018 at 10:38:50PM +, Dr. Matthias St. Pierre wrote:
> preliminary status report:
> 
> *** CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
>   see https://github.com/openssl/openssl/pull/7156
>   
> *** CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
>   work in progress...   

I think this one may be a false positive -- it's worried that EVP_MD_size()
will return -1, but we've essentially already validated that the md is
valid by the time we get there.  I didn't do a full check, though.

-Ben

> *** CID 1439136:  Resource leaks  (RESOURCE_LEAK)
>   see https://github.com/openssl/openssl/pull/7155
> 
> *** CID 1439135:  Memory - illegal accesses  (INCOMPATIBLE_CAST)
>   todo
> 
> *** CID 1423323:  Null pointer dereferences  (FORWARD_NULL)
>   see https://github.com/openssl/openssl/pull/7158
> 
> *** CID 1201571:  Error handling issues  (CHECKED_RETURN)
>   todo
> 
> if anybody wants to fix one of the CIDs marked 'todo', no problem. Just drop 
> a note on the openssl-project list.
> 
> Matthias
> 
> 
> > -Ursprüngliche Nachricht-
> > Von: openssl-project  Im Auftrag von 
> > Benjamin Kaduk
> > Gesendet: Sonntag, 9. September 2018 18:04
> > An: openssl-project@openssl.org
> > Betreff: [openssl-project] coverity defect release criteria (Fwd: New 
> > Defects reported by Coverity Scan for openssl/openssl)
> > 
> > I see that Matthias has opened pull requests for a couple of these already;
> > are you planning to work through the rest of them as well?
> > 
> > -Ben
> > 
> > On Sun, Sep 09, 2018 at 09:28:12AM +, scan-ad...@coverity.com wrote:
> > > Hi,
> > >
> > > Please find the latest report on new defect(s) introduced to 
> > > openssl/openssl found with Coverity Scan.
> > >
> > > 6 new defect(s) introduced to openssl/openssl found with Coverity Scan.
> > >
> > >
> > > New defect(s) Reported-by: Coverity Scan
> > > Showing 6 of 6 defect(s)
> > >
> > >
> > > ** CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
> > >
> > >
> > > 
> > > *** CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
> > > /crypto/rsa/rsa_pss.c: 247 in RSA_padding_add_PKCS1_PSS_mgf1()
> > > 241 EM[emLen - 1] = 0xbc;
> > > 242
> > > 243 ret = 1;
> > > 244
> > > 245  err:
> > > 246 EVP_MD_CTX_free(ctx);
> > > >>> CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
> > > >>> "sLen" is passed to a parameter that cannot be negative.
> > > 247 OPENSSL_clear_free(salt, sLen);
> > > 248
> > > 249 return ret;
> > > 250
> > > 251 }
> > > 252
> > > 253 #if defined(_MSC_VER)
> > > 254 # pragma optimize("",on)
> > >
> > > ** CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
> > >
> > >
> > > 
> > > *** CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
> > > /crypto/sm2/sm2_pmeth.c: 277 in pkey_sm2_digest_custom()
> > > 271 }
> > > 272
> > > 273 /* get hashed prefix 'z' of tbs message */
> > > 274 if (!sm2_compute_z_digest(z, md, smctx->id, smctx->id_len, 
> > > ec))
> > > 275 return 0;
> > > 276
> > > >>> CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
> > > >>> "EVP_MD_size(md)" is passed to a parameter that cannot be 
> > > >>> negative.
> > > 277 return EVP_DigestUpdate(mctx, z, EVP_MD_size(md));
> > > 278 }
> > > 279
> > > 280 const EVP_PKEY_METHOD sm2_pkey_meth = {
> > > 281 EVP_PKEY_SM2,
> > > 282 0,
> > >
> > > ** CID 1439136:  Resource leaks  (RESOURCE_LEAK)
> > > /test/dhtest.c: 202 in dh_test()
> > >
> > >
> > > 
> > > *** CID 1439136:  Resource leaks  (RESOURCE_LEAK)
> > > /test/dhtest.c: 202 in dh_test()
> > > 196 BN_free(bp);
> > > 197 BN_free(bg);
> > > 198 BN_free(cpriv_key);
> > > 199  

[openssl-project] coverity defect release criteria (Fwd: New Defects reported by Coverity Scan for openssl/openssl)

2018-09-09 Thread Benjamin Kaduk
I see that Matthias has opened pull requests for a couple of these already;
are you planning to work through the rest of them as well?

-Ben

On Sun, Sep 09, 2018 at 09:28:12AM +, scan-ad...@coverity.com wrote:
> Hi,
> 
> Please find the latest report on new defect(s) introduced to openssl/openssl 
> found with Coverity Scan.
> 
> 6 new defect(s) introduced to openssl/openssl found with Coverity Scan.
> 
> 
> New defect(s) Reported-by: Coverity Scan
> Showing 6 of 6 defect(s)
> 
> 
> ** CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
> 
> 
> 
> *** CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
> /crypto/rsa/rsa_pss.c: 247 in RSA_padding_add_PKCS1_PSS_mgf1()
> 241 EM[emLen - 1] = 0xbc;
> 242 
> 243 ret = 1;
> 244 
> 245  err:
> 246 EVP_MD_CTX_free(ctx);
> >>> CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
> >>> "sLen" is passed to a parameter that cannot be negative.
> 247 OPENSSL_clear_free(salt, sLen);
> 248 
> 249 return ret;
> 250 
> 251 }
> 252 
> 253 #if defined(_MSC_VER)
> 254 # pragma optimize("",on)
> 
> ** CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
> 
> 
> 
> *** CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
> /crypto/sm2/sm2_pmeth.c: 277 in pkey_sm2_digest_custom()
> 271 }
> 272 
> 273 /* get hashed prefix 'z' of tbs message */
> 274 if (!sm2_compute_z_digest(z, md, smctx->id, smctx->id_len, ec))
> 275 return 0;
> 276 
> >>> CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
> >>> "EVP_MD_size(md)" is passed to a parameter that cannot be negative.
> 277 return EVP_DigestUpdate(mctx, z, EVP_MD_size(md));
> 278 }
> 279 
> 280 const EVP_PKEY_METHOD sm2_pkey_meth = {
> 281 EVP_PKEY_SM2,
> 282 0,
> 
> ** CID 1439136:  Resource leaks  (RESOURCE_LEAK)
> /test/dhtest.c: 202 in dh_test()
> 
> 
> 
> *** CID 1439136:  Resource leaks  (RESOURCE_LEAK)
> /test/dhtest.c: 202 in dh_test()
> 196 BN_free(bp);
> 197 BN_free(bg);
> 198 BN_free(cpriv_key);
> 199 BN_GENCB_free(_cb);
> 200 DH_free(dh);
> 201 
> >>> CID 1439136:  Resource leaks  (RESOURCE_LEAK)
> >>> Variable "priv_key" going out of scope leaks the storage it points to.
> 202 return ret;
> 203 }
> 204 
> 205 static int cb(int p, int n, BN_GENCB *arg)
> 206 {
> 207 return 1;
> 
> ** CID 1439135:  Memory - illegal accesses  (INCOMPATIBLE_CAST)
> 
> 
> 
> *** CID 1439135:  Memory - illegal accesses  (INCOMPATIBLE_CAST)
> /apps/speed.c: 3105 in speed_main()
> 3099 ERR_print_errors(bio_err);
> 3100 rsa_count = 1;
> 3101 } else {
> 3102 for (i = 0; i < loopargs_len; i++) {
> 3103 /* Perform EdDSA signature test */
> 3104 loopargs[i].siglen = test_ed_curves[testnum].siglen;
> >>> CID 1439135:  Memory - illegal accesses  (INCOMPATIBLE_CAST)
> >>> Pointer "[i].siglen" points to an object whose effective 
> >>> type is "unsigned int" (32 bits, unsigned) but is dereferenced as a wider 
> >>> "unsigned long" (64 bits, unsigned).  This may lead to memory corruption.
> 3105 st = EVP_DigestSign(loopargs[i].eddsa_ctx[testnum],
> 3106 loopargs[i].buf2, (size_t 
> *)[i].siglen,
> 3107 loopargs[i].buf, 20);
> 3108 if (st == 0)
> 3109 break;
> 3110 }
> 
> ** CID 1423323:  Null pointer dereferences  (FORWARD_NULL)
> 
> 
> 
> *** CID 1423323:  Null pointer dereferences  (FORWARD_NULL)
> /test/evp_extra_test.c: 894 in test_EVP_PKEY_check()
> 888 
> 889 if (!TEST_int_eq(EVP_PKEY_param_check(ctx), expected_param_check))
> 890 goto done;
> 891 
> 892 ctx2 = EVP_PKEY_CTX_new_id(0xdefaced, NULL);
> 893 /* assign the pkey directly, as an internal test */
> >>> CID 1423323:  Null pointer dereferences  (FORWARD_NULL)
> >>> Passing null pointer "pkey" to "EVP_PKEY_up_ref", which dereferences 
> >>> it.
> 894 EVP_PKEY_up_ref(pkey);
> 895 ctx2->pkey = pkey;
> 896 
> 897 if (!TEST_int_eq(EVP_PKEY_check(ctx2), 0xbeef))
> 898 goto done;
> 899 
> 
> ** CID 1201571:  Error handling issues  

Re: [openssl-project] Release Criteria Update

2018-09-06 Thread Benjamin Kaduk
On Wed, Sep 05, 2018 at 06:04:08PM -0500, Benjamin Kaduk wrote:
> On Wed, Sep 05, 2018 at 11:59:34PM +0100, Matt Caswell wrote:
> > Today's update is that we still have 6 open PRs for 1.1.1. 5 of these
> > are the same as yesterday. The 1 that was marked as "ready" yesterday
> > has now been merged, and a new PR addressing issue #7014 has been opened.
> > 
> > There are still 2 open issues for 1.1.1 but both of these are now being
> > addressed by one of the open PRs.
> > 
> > That means there are still 4 "critical path" PRs open:
> > 
> > #7115 Restore historical SSL_get_servername() behavior
> > 
> > Updates made following earlier review. Ready for another round of reviews??
> > Owner: Ben.
> 
> I believe it's ready for another round of reviews, yes.
> Do we think we want to wait for confirmation from @MSP-Greg?

I see that Matt has marked this one as Ready.
I'm going to be "on a plane" (not exactly, but effectively so) for the next
9-ish hours and am not confident that I'll be able to merge it until
tomorrow.  I also see that the original reporter is still not having
success; is anyone in a position to try to set up those Ruby EventMachine
tests (it's unclear if it needs to be on windows or not)?

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] Release Criteria Update

2018-09-05 Thread Benjamin Kaduk
On Wed, Sep 05, 2018 at 11:59:34PM +0100, Matt Caswell wrote:
> Today's update is that we still have 6 open PRs for 1.1.1. 5 of these
> are the same as yesterday. The 1 that was marked as "ready" yesterday
> has now been merged, and a new PR addressing issue #7014 has been opened.
> 
> There are still 2 open issues for 1.1.1 but both of these are now being
> addressed by one of the open PRs.
> 
> That means there are still 4 "critical path" PRs open:
> 
> #7115 Restore historical SSL_get_servername() behavior
> 
> Updates made following earlier review. Ready for another round of reviews??
> Owner: Ben.

I believe it's ready for another round of reviews, yes.
Do we think we want to wait for confirmation from @MSP-Greg?

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] Release Criteria Update

2018-09-04 Thread Benjamin Kaduk
On Tue, Sep 04, 2018 at 05:11:41PM +0100, Matt Caswell wrote:
> There are 2 open issues for 1.1.1. One of these is being addressed by
> PR#7073 above. The other one is:
> 
> #7014 TLSv1.2 SNI hostname works in 1.1.0h, not in 1.1.1 master (as of
> 18-Aug)
> 
> This one seems stuck!! No clear way forward as yet.
> 
> Ben - any views?

I'm thinking that the ABI stability argument is going to win me over and
we should continue to return the client's offered SNI in all cases until
1.2.0.  Hoping to get a patch out this morning (US pacific) -- yesterday
was a national holiday.

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] Late thoughts on the 1.1.1 release - are we fooling ourselves?

2018-08-17 Thread Benjamin Kaduk
On Fri, Aug 17, 2018 at 06:39:54PM +0200, Richard Levitte wrote:
> In message <20180817162909.ga10...@roeckx.be> on Fri, 17 Aug 2018 18:29:09 
> +0200, Kurt Roeckx  said:
> 
> kurt> On Fri, Aug 17, 2018 at 01:55:13PM +0200, Richard Levitte wrote:
> kurt> > Personally, I see this as a showstopper re a release on Tuesday
> kurt> 
> kurt> You mean a beta release?
> 
> ...
> 
> D'oh.  For some reason, I got mixed up and imagined a live release on
> Tuesday.
> 
> So ok, not a showstopper per se, even though I would have *liked* to
> see as much of the applicable issues fixed and PRs (and that includes
> those I'm talking about) applied as possible.
> 
> However, I think we need to go over the "Assessed" milestone stuff and
> make an actual assessment on them (i.e. labeling them correctly).  The
> final release will probably not be very far away, and I'd hate to have
> to call showstopper again by then.

I agree.  And, if we make a bunch of (bugfix) changes between a beta
release and the scheduled final release, it may be more appropriate to do
another beta than to do a final release with "many" (vaguely defined)
changes since the last beta.

-Ben

> See this thread as an early warning ;-)
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] Speaking of broken master, have a look at Travis

2018-07-24 Thread Benjamin Kaduk
On Tue, Jul 24, 2018 at 08:34:28PM +0200, Kurt Roeckx wrote:
> On Tue, Jul 24, 2018 at 07:54:58PM +0200, Richard Levitte wrote:
> > ...
> > go test: FAILED (ServerNameExtensionServer-TLS1)
> > go test: unexpected failure: local error 'read tcp4 
> > 127.0.0.1:41729->127.0.0.1:60574: read: connection reset by peer', child 
> > error 'signal: segmentation fault (core dumped)', stdout:
> 
> This is caused by https://github.com/openssl/openssl/pull/6378

Yup, Andy pointed it out.
I've tried to get a local setup with the boring tests, but need to put a
bit more time into it, it seems.  At least there's not an IESG telechat
this week...

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] To distribute just the repo file, or the result of 'make dist'?

2018-07-24 Thread Benjamin Kaduk
On Tue, Jul 24, 2018 at 02:28:40PM +0200, Kurt Roeckx wrote:
> On Tue, Jul 24, 2018 at 02:08:46PM +0200, Richard Levitte wrote:
> > 
> > The original intention (way back, I think we're even talking SSLeay
> > time here, but at the very least pre-1.0.0 time) was to make a tarball
> > that can be built directly with just a 'make' on any Unix box and
> > without requiring perl.
> 
> I don't see how that could work our current system. As far as I
> know, it's actually confired for a system, and it will not work
> properly on an other. It would just work on the same system as
> that we ran config on.
> 
> > 1.  Don't release pre-configured tarballs.  This is a very simple
> > thing to do, all we have to do is use 'make tar' to create
> > tarballs instead of 'make dist'.  We could remove the dist target
> > entirely while we're at it.
> 
> This makes most sense to me.

To me as well.

(As a side note, OpenAFS also has something called 'make dist' that is
functionally different, but also has deep historical roots and is also
something I'm trying to get rid of.)

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


[openssl-project] thread-unsafety in SNI handling with SSL_SESSION

2018-07-02 Thread Benjamin Kaduk
Hi folks,

https://github.com/openssl/openssl/pull/4519 introduced some thread-unsafe
behavior, and we had some discussion on that (closed) PR back in May, which
led to the creation of https://github.com/openssl/openssl/pull/6378 .  The
latter one has languished for a while, partly because I was slow in making
some needed fixups.  But it may be worth revisiting now, if anyone has some
time to spare.

Thanks,

Bne
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] Milestones and the 1.1.1 release

2018-06-26 Thread Benjamin Kaduk
On Tue, Jun 26, 2018 at 07:43:45PM +, Salz, Rich wrote:
> That's interesting.  Would we put a bugfix in 1.1.0, not put the fix in 1.1.1 
> until our first "a" release?
> 
> Or are you saying that if it's in 1.1.0, then we don't have to fix it until 
> after 1.1.1 comes out?  That seems justifiable to me.

I assume the latter -- we feel obligated to fix regressions from 1.1.0 to
1.1.1 before finalizing 1.1.1, but bugs that are present in 1.1.0 can be
present in the 1.1.1 initial release (to be fixed in 1.1.1a and 1.1.0next).
(This is what I do for OpenAFS.)

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] Milestones and the 1.1.1 release

2018-06-26 Thread Benjamin Kaduk
On Tue, Jun 26, 2018 at 04:56:26PM +0100, Matt Caswell wrote:
> I'm thinking that we should maybe re-asses the current milestones in github.
> 
> We currently use the following milestones:
> 
> Assessed - Anything against this milestone isn't relevant to the 1.1.1
> release (e.g. 1.0.2 specific issue)
> 
> 1.1.1 - This is relevant to the 1.1.1 release but may not be specific to
> it (e.g. an issue that affects both 1.1.1 and 1.1.0)
> 
> Post 1.1.1 - Feature request to be looked at once 1.1.1 is released
> 
> 
> I think we should re-asses everything currently against the 1.1.1
> milestone so that anything which isn't specific to that release gets
> moved to the "Assessed" milestone.
> 
> At the moment its difficult to see the "wood for the trees" between
> issues which are newly introduced and those that are long standing. In
> terms of getting the 1.1.1 release out the door we should focus on the
> former.
> 
> Thoughts?

If the choice is between your proposal and the current state, your proposal
seems better.  I don't want to start a bikeshed, so I'll try to leave the
discussion on the immediate topic which relates to getting 1.1.1 out the
door and not add in other things I'd like to see changed (but do not really
seem to be blocking 1.1.1).

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] GitHub labels

2018-06-21 Thread Benjamin Kaduk
On Wed, Jun 20, 2018 at 10:29:37PM +0200, Richard Levitte wrote:
> In message  on Wed, 20 Jun 
> 2018 19:59:02 +, "Dr. Matthias St. Pierre"  
> said:
> 
> Matthias.St.Pierre> III) VERSION NUMBER LABELS
> Matthias.St.Pierre> 
> Matthias.St.Pierre> It seems like the version number labels '1.0.2',
> Matthias.St.Pierre> '1.1.0', '1.1.1', 'after 1.1.1' currently serve
> Matthias.St.Pierre> two differente purposes: 
> Matthias.St.Pierre> 
> Matthias.St.Pierre> 1. Indicate the intention to which branches a pull
> Matthias.St.Pierre>request will be backported 
> Matthias.St.Pierre>    Approval holds for all labeled branches.
> Matthias.St.Pierre>    
> Matthias.St.Pierre> 2. As surrogate milestones
> 
> ... and the other way around, it seems silly to use a "1.0.2"
> milestone, since 1.0.2 was released a long time ago.  I'd argue that
> all old milestones should really be removed, and only future versions
> should have milestones.

I would argue that old milestones should *not* be removed!  There seems to
be some archival value in being able to see the contents of the milestone
even after it is completed.

> Matthias.St.Pierre> ad 1):
> Matthias.St.Pierre> Using the version number labels as indication of merge 
> intention makes sense.
> Matthias.St.Pierre> But then the 'master' label and (currently) the '1.1.1' 
> label are superfluous.
> 
> I'd suggest keeping the 1.1.1 label, as we will have use for it.
> 
> Matthias.St.Pierre> If the pull request targets the 'master' branch, why does 
> it need a 'master' label?
> Matthias.St.Pierre> The github search index allows to search for 
> 'base:' which is a much
> Matthias.St.Pierre> more reliable way of determining the target branch:
> 
> I'm learning something new, I had no clue of the 'base:' feature.
> 
> However, it sometimes happens that I do a PR based on, for example,
> OpenSSL_1_1_0-stable, simply because that's where the issue was found,
> but with the intent to cherry pick into newer lines of development
> (master, and OpenSSL_1_1_1-stable soon).  That gives those labels
> their potential for showing intent.
> 
> Matthias.St.Pierre> ad 2): 
> Matthias.St.Pierre> The label 'after 1.1.1' is a surrogate milestone
> Matthias.St.Pierre> and IMHO it would be better to use the 'Post
> Matthias.St.Pierre> 1.1.1' milestone instead of the label. 
> 
> I agree with you, but this was debated during the last F2F, and ideas
> differ.  I don't quite remember if we came to a real decision, though.
> 
> Matthias.St.Pierre> One could go even further and ask what sense does
> Matthias.St.Pierre> it make to have such an unspecific milestone as
> Matthias.St.Pierre> 'Post 1.1.1'? Wouldn't it be better to leave such
> Matthias.St.Pierre> pull requests unassigned?
> 
> No, because we need to differentiate between PRs and issues we haven't
> looked at yet and those where we have made a decision where they
> should go.  And perhaps that's an argument to keep using the label, as
> it's more visible in the pull request summary.
> 
> Matthias.St.Pierre> IMHO it would make sense to use the version labels
> Matthias.St.Pierre> only to indicate merge intention and otherwise use
> Matthias.St.Pierre> milestones.
> 
> I personally agree.

I think that could work.

What's still unclear to me in the current scheme is how I'm supposed to
indicate something that is (intentionally) API/ABI-breaking and must be
postponed to the next major release.  Bear in mind that we still don't know
of the release after 1.1.1 will be such a thing or not...

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] To use or not use the iconv API, and to use or not use other libraries

2018-06-07 Thread Benjamin Kaduk
On Thu, Jun 07, 2018 at 05:55:27PM +0200, Andy Polyakov wrote:
> > Regarding general use of other libraries, please think carefully before 
> > voting, 'cause this *is* tricky. If you have a look, you will see that we 
> > *currently* depend on certain standard libraries, such as, for example, 
> > libdl.
> 
> One has to recognize that each dependency has to be justified. Sometimes
> you don't have a choice. For example if you want to talk network on
> Solaris, you have to link with -lsocket -lnsl. It's just part of the
> game. But in cases one has a choice, well, one has a choice to *make*.
> And key question is how do you anchor it. It's only natural to have as
> little dependencies as possible, so question is what would justify extra
> dependency.

Taking off on a bit of a tangent, how much justification did we go
through when adding pthreads as a dependency.  I have not been
following very much (Kurt would know more), but apparently in Debian
there are some issues regarding (statically linked?) applications
and libraries that use libcrypto but do not explicitly link with
-pthread.  "Issues" here being, IIRC, crashes at runtime.

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] Is Mac a supported platform?

2018-06-01 Thread Benjamin Kaduk
On Fri, Jun 01, 2018 at 06:52:21PM +, Salz, Rich wrote:
> Our INSTALL doesn’t mention it.  We have config’s for it.  I think we should 
> say we support it and update the various docs.  Thoughts?

The PR associated with the thread around
https://mta.openssl.org/pipermail/openssl-project/2018-January/68.html
seems likely to be relevant, though I don't think I have a link
handy.

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project

Re: [openssl-project] Help deciding on PR 6341 (facilitate reading PKCS#12 objects in OSSL_STORE)

2018-06-01 Thread Benjamin Kaduk
On Fri, Jun 01, 2018 at 12:23:39PM +, Salz, Rich wrote:
> >I think that the gist of the difference of opinion is whether it's OK
> to use locale dependent functions such as mbstowcs in libcrypto or
> not.
>   
> 
> Thanks for the summary.
> 
> I am against use locale-dependent functions in libcrypto. 

I think it's pretty clear (at least to me), that such functions do
not belong in the normal path.  I'd be open to considering them as a
fallback attempt to read existing data (as opposed to generating new
encrypted data), but find Andy's argument about nonpredictability
(combined with David Woodhouse's enumeration of the various cases
and the minimal utility of such conversions) to be fairly
compelling.  That is, I am also against the use of functions that
depend on the current process's locale in libcrypto.  (I phrase this
slightly differently, in that functions which take an explicit
locale to use might still be okay, but are not really portable
enough for us to use, AIUI.)

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] build/test before merging

2018-05-23 Thread Benjamin Kaduk
On Wed, May 23, 2018 at 03:12:30PM +, Dr. Matthias St. Pierre wrote:
> > So do you guys use the ghmerge script or own procedures?  I'm curious.
> 
> At the beginnning, I tried to use ghmerge but it was not flexible
> enough for my needs. In particular, it only gives me the choice
> between squashing everything or leaving everything as it is. Most
> notably, it does not support partial squashing by interactive
> rebasing. Or alternatively: pausing + letting me fix something +
> resuming. What I also dislike is that it uses a lot of GitHub API

Sorry for partially hijacking the thread, but this reminds me that
several people have started using the "git commit --fixup" tooling,
which is in general helpful for the reviewer (to know what the
squashing intention is).

But I am curious if we currently do and/or should have a commit hook
on git.openssl.org to reject commits that start with "!fixup".

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] build/test before merging

2018-05-22 Thread Benjamin Kaduk
On Wed, May 23, 2018 at 12:43:58AM +, Salz, Rich wrote:
> > I do the same, but I am reluctant having a script doing it for me using 
> some fixed recipe...
> 
> >I'm happy doing the build/test manually before merging, too.
>   
> 
> So do you guys use the ghmerge script or own procedures?  I'm curious.

My own procedures (the addrev script and push by hand).

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] OpenSSL 1.1.1 library(OpenSSL 1.1.0 compile) Postfix to Postfix test

2018-04-28 Thread Benjamin Kaduk
On Tue, Apr 24, 2018 at 10:21:28AM -0400, Viktor Dukhovni wrote:
> 
> 
> > On Apr 24, 2018, at 9:29 AM, Benjamin Kaduk <ka...@mit.edu> wrote:
> > 
> > To be clear, the current draft explicitly says "Servers SHOULD issue
> > new tickets with every connection."  This is not a MUST, but is
> > perhaps strong enough guidance to merit overriding the existing
> > ticket callback semantics.
> 
> Fine advice for browsers, but not terribly useful for Postfix.
> Multiple processes read and write the session cache in parallel,
> and single-use tickets won't work without serialization and
> multiple cache slots for the same destination.
> 
> The Postfix SMTP server needs to be able to issue tickets only
> as-needed on the server.  The TLS 1.2 model works just fine for
> SMTP and STEKs are already properly rotated.

I'm not trying to say that Postfix or even SMTP in general needs to
adopt the TLS 1.3 (Web) model; I'm only trying to consider the
OpenSSL 1.1.1 library default, which I think ought to honor the
SHOULD in the spec.

> I think that the previous behaviour of the callback needs to
> continue to apply, if the callback does not return re-issue,
> no new ticket should be returned.  The callback has access
> to the SSL handle and can determine the protocol version
> if it so chooses.

and will not automatically update to TLS 1.3 semantics by default.
But maybe that's okay.

> The built-in ticket callback can always re-issue if that's
> the preferred default.

I think that is the preferred default, and would not object to
implementing the default in the built-in ticket callback if you
insist on Postfix not having to change its callback to get its
preferred behavior.

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] OpenSSL 1.1.1 library(OpenSSL 1.1.0 compile) Postfix to Postfix test

2018-04-24 Thread Benjamin Kaduk
On Mon, Apr 23, 2018 at 09:34:18PM -0400, Viktor Dukhovni wrote:
> 
> 
> > On Apr 22, 2018, at 9:49 PM, Viktor Dukhovni  
> > wrote:
> > 
> > - Client-side diagnostics -
> 
> On the server side I see that even when the ticket callback returns "0" to 
> accept and not re-issue the ticket, a new ticket is requested anyway.  I'd 
> like to be able to control this, and not issue new tickets when the present 
> ticket is acceptable.  If this requires new API entry points, I can condition 
> them on a suitable min library version.  But ideally the callback return 
> value will be honoured, I don't yet see why we would not do that.

To be clear, the current draft explicitly says "Servers SHOULD issue
new tickets with every connection."  This is not a MUST, but is
perhaps strong enough guidance to merit overriding the existing
ticket callback semantics.

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] FW: April Crypto Bulletin from Cryptosense

2018-04-06 Thread Benjamin Kaduk
On Fri, Apr 06, 2018 at 04:23:02PM +0200, Andy Polyakov wrote:
> > This is one reason why keeping around old assembly code can have a cost. :(
> > 
> > https://github.com/openssl/openssl/pull/5320
> 
> There is nothing I can add to what I've already said. To quote myself.
> "None of what I say means that everything *has to* be kept, but as
> already said, some of them serve meaningful purpose..."
> 
> Well, I also said that "I'm *not* saying that bit-rot is not a concern,
> only that it's not really assembly-specific." And I can probably add
> something here, in addition to already mentioned example of legacy code
> relying on formally undefined or implementation-specific behaviour. It's
> not actually that uncommon that *new* C code is committed[!!!]
> "bit-rotten". So one can *just as well* say that supporting another
> operating system has a cost, and so does using another compiler... Why
> not get "angry" about that? What's the difference really? Relevant

Yes, supporting another operating system has a cost!
At risk of drawing Richard's ire, if we did not intend to support
(e.g.) VMS, we might have been able to get away with not writing our
own custom build system in favor of some "industry standard".
Supporting non-POSIX systems (e.g., Windows) also adds overhead in
how we implement many of our interfaces (file handling, thread
handling, locking, randomness, etc.).

I personally prefer a more conservative/restrictive approach than
the historical trend, and probably also more conservative than the
average of the team.  This is presumably shaped by my personal
experiences and career trajectory, and I understand that others'
path are different and so they will have different, but still valid,
preferences.  We as a team are charged with weighing the tradeoff of
supporting an additional platform against the burden of supporting
it and the risks against our ability to continue supporting it.  For
example, in this modern world where properly supporting a platform
basically does require some assembly code, for crypto-relevant
timing considerations, if only one person understands and will
support that assembly code, that is a risk.  Perhaps it's enough of
a risk to make officially supporting that platform a bad idea;
perhaps not -- it's just one factor that we must, as a whole, weigh
and consider.
Removing platform-specific assembly when not needed for security
would seem to reduce the risk, and presumably improve the
maintainability of the software as a whole.  But I don't see a good
way to not have these decisions all be made on a case-by-case basis.

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] About PR 5702, etc.

2018-03-29 Thread Benjamin Kaduk
On Thu, Mar 29, 2018 at 12:15:39PM +0200, Richard Levitte wrote:
> In message <4e32b364-3ed3-9101-158c-09338f96e...@openssl.org> on Thu, 29 Mar 
> 2018 11:06:46 +0100, Matt Caswell  said:
> 
> matt> How about this for the vote text:
> matt> 
> matt> "Feature changes in 1.1.1 directly related to TLSv1.3 will be allowed
> matt> during the beta as long as at least 3 OMC members approve the change"
> 
> I can get behind that.

Yup, that sounds good to me as well.

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] Code Repo

2018-03-20 Thread Benjamin Kaduk
On Wed, Mar 21, 2018 at 12:27:13AM +1000, Tim Hudson wrote:
> We have been holding off on post-1.1.1 feature development for a long time
> now - on the grounds that TLSv1.3 was just around the corner etc and the
> release was close - and then we formed a release plan which we pushed back
> a week.

I expect TLS 1.3 to be sent to the RFC Editor in the next day.

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] DRBGs, threads and locking

2018-03-13 Thread Benjamin Kaduk
On Wed, Mar 14, 2018 at 01:27:47AM +0100, Kurt Roeckx wrote:
> My solution is to just have 1 master DRBG, and a public and
> private DRBG per thread. The only lock that then is needed is when
> the public or private DRBG needs to reseed. All the rest of the
> code can stay just as it is, but we might want to change some
> places to use the (thread local) private DRBG, which is what #4665
> is about.
[...]
> So the suggestion was to still have a per SSL public DRBG, but
> then the problem is that that SSL object might have moved to a
> different thread between creating and being used and so that the
> parent DRBG might actually belong to a different thread. One
> solution there is that we just take the current thread's public
> DRBG as parent instead of the original threads public DRBG.

This should be fine from a thread-safety point of view.  I don't
know whether it could potentially affect the standards compliance,
for the intermediate DRBG to potentially change over time (even
though it still chains to a common grandparent/master DRBG).

Per-SSL DRBGs (especially if split to public and private) seem
excessive to me, so architecture described in the quoted text seems
like the best option, to me.

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] External contributors and the next release

2018-03-06 Thread Benjamin Kaduk
On Wed, Mar 07, 2018 at 03:01:03PM +1000, Tim Hudson wrote:
> If you are blocked on review please drop a note (like the one you just did)
> to the group.
> Some of us review the specifically blocked things when such notes are sent.
> 
> #3082 is already closed and merged - did you mean another PR?

Yup, I meant #3802, sorry.
(tmshort is my team lead at work)

> #3958 approved (in case Richard doesn't get back to it)
> #1130 approved
> #3958 approved

Thanks!

-Ben

> Tim.
> 
> 
> 
> On Wed, Mar 7, 2018 at 2:40 PM, Benjamin Kaduk <ka...@mit.edu> wrote:
> 
> > On Wed, Mar 07, 2018 at 01:20:41AM +, Salz, Rich wrote:
> > > I think we should make sure to set aside time to review as many of the
> > non-project pull requests as possible.  I think it is important to show a
> > commitment to the larger community.
> >
> > I agree.  I started looking at this last week, when we had something
> > like 170 open issues+pull requests for the 1.1.1 milestone.  We
> > still have 165, and that excludes a couple things that I would like
> > to see get in but are not part of the milestone (like #3082 and
> > #1130).  I also have several changes open myself that are small and
> > could easily go in, though none of them are especially critical.
> > (Richard, could you reconfirm on #3958, though, please?)
> >
> > > One way to do this is to say that we will extend the BETA date by a
> > week, and in that week no new code from the team, only third-party existing
> > pull requests will be considered
> >
> > I'm open to extending the date, though am interested to see what we
> > can do if everybody chips in this week.  (Though, I myself will only
> > be able to start on Friday, as my next couple days are fully
> > booked.)
> >
> > -Ben
> > ___
> > openssl-project mailing list
> > openssl-project@openssl.org
> > https://mta.openssl.org/mailman/listinfo/openssl-project
> >

> ___
> openssl-project mailing list
> openssl-project@openssl.org
> https://mta.openssl.org/mailman/listinfo/openssl-project

___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] External contributors and the next release

2018-03-06 Thread Benjamin Kaduk
On Wed, Mar 07, 2018 at 01:20:41AM +, Salz, Rich wrote:
> I think we should make sure to set aside time to review as many of the 
> non-project pull requests as possible.  I think it is important to show a 
> commitment to the larger community.

I agree.  I started looking at this last week, when we had something
like 170 open issues+pull requests for the 1.1.1 milestone.  We
still have 165, and that excludes a couple things that I would like
to see get in but are not part of the milestone (like #3082 and
#1130).  I also have several changes open myself that are small and
could easily go in, though none of them are especially critical.
(Richard, could you reconfirm on #3958, though, please?)

> One way to do this is to say that we will extend the BETA date by a week, and 
> in that week no new code from the team, only third-party existing pull 
> requests will be considered

I'm open to extending the date, though am interested to see what we
can do if everybody chips in this week.  (Though, I myself will only
be able to start on Friday, as my next couple days are fully
booked.)

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] Next release is beta1

2018-03-04 Thread Benjamin Kaduk
On Sun, Mar 04, 2018 at 05:30:32PM +0100, Kurt Roeckx wrote:
> On Sun, Mar 04, 2018 at 02:44:01PM +, Salz, Rich wrote:
> > I also intend to merge the config file .include PR (5351), and I want us to 
> > decide about 4848.
> 
> I have to agree that I want to resolv 4848 (reading config file to
> select things like supported ciphers.)

So far my personal opinion on this one is that I'd rather wait until
1.2 and actually change the SSL_CTX_new() behavior, as opposed to
having to add a new API that not much software would be using.  (To
be clear, I think that changing SSL_CTX_new() to read a systemwide
config file is inconsistent with our API stability policy for dot
releases.)  This is perhaps complicated by the interplay with #2397,
which also wants to extend SSL_CTX_new() for sharing session caches
between SSL_CTXes.  (This behavior inherently requires a new API.)

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


[openssl-project] Potentially adding TLS record header to TLS 1.3 AAD

2018-02-24 Thread Benjamin Kaduk
Hi all,

There's a pull request open against the TLS 1.3 spec to include the
record header in the AAD for record protection
(https://github.com/tlswg/tls13-spec/pull/1158).  We're somewhat on
the fence about this, with the main advantage seeming to be for DTLS
and not plain TLS, but it would probably still be useful to have
some sense for how hard it would be to implement.  Matt, do you have
any thoughts off the top of your head?

Thanks,

Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] release tools now in the 'tools' repository

2018-02-03 Thread Benjamin Kaduk
On Sat, Feb 03, 2018 at 11:40:42PM +, Salz, Rich wrote:
> We have cleaned up and posted the release tools as part of the tools 
> repository.  Thanks to Richard Levitte for a great deal of feedback and 
> review.
> 
> I had thought someone opened an issue for this, but I can’t find it; anyone 
> else remember?

Someone was asking for xz compressed releases and the non-visibility
of the tools came up there:
https://github.com/openssl/openssl/issues/3786

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project

Re: [openssl-project] Local kid does good

2018-01-30 Thread Benjamin Kaduk
On Tue, Jan 30, 2018 at 04:14:52PM +, Matt Caswell wrote:
> 
> 
> On 30/01/18 16:13, Salz, Rich wrote:
> > One of our own, Ben Kaduk, was just picked to be the Security Area
> > co-Director in the IETF!
> 
> Awesome! Well done Ben!

Thanks!

It does seem likely to imply that I will be spending less time on
code (reviews), though.

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


[openssl-project] travis builds failing with aligment errors?

2018-01-30 Thread Benjamin Kaduk
It seems that we've started getting issues with a single build
configuration, e.g.,
https://travis-ci.org/openssl/openssl/jobs/335110257

Lots of complaints about alignment, like:

crypto/modes/gcm128.c:1090:36: runtime error: load of misaligned
address 0x02350ce5 for type 'const size_t' (aka 'const unsigned
long'), which requires 8 byte alignment
0x02350ce5: note: pointer points here
 68 1f ea 3b 14 00 00  0c 00 02 00 00 00 00 00  0c a3 35 89 7d a7 5e 9e  87 fa 
d7 fd 8b c7 34 8a  8d
 ^ 
I didn't see anything particularly special about that configuration
on a quick once-over; any ideas?

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] Style guide updates

2018-01-27 Thread Benjamin Kaduk
On Fri, Jan 26, 2018 at 01:26:58PM +, Salz, Rich wrote:
> Some things I think we should add to the style guide.  Let’s discuss here.
> 
> No space after sizeof, use parens.  (But see ssl/record/rec_layer_{d1,s3}.c )
> 
> Multiline conditionals, such as in an if, should be broken before the logical 
> connector and indented an extra tabstop.  For example:
> 
> while (this_is_true
>   && that_is_not_false) {
> frobit();
> more_stuff();
> }

To state it explictly, in the absence of the "extra" indentation
this would be formatted

while (this_is_true
   && that_is_not_false) {
frobit();
more_stuff();
}

which is, as was noted, quite readable for the while() case, and
only if() has trouble.

> When dealing with long lines, try to avoid breaking across a function call. 
> Don’t do this:
> If (some_long_text && foo(a,
>  b, c) && bar()) {

And for this specific case, I am used to the rule being that if you
break mid-function-call, you indent to the paren for the function
call, as in

if (some_long_text && foo(a,
  b, c) && bar()) {

which rather inherently incentivizes not breaking across the
function call.

I read the rest of the thread and don't really have more to add to
it.

-Ben

> Instead do this:
> If (some_long_text
> && foo(a, b, c,)
> && bar())
> 
> What else needs to be updated?
> 

> ___
> openssl-project mailing list
> openssl-project@openssl.org
> https://mta.openssl.org/mailman/listinfo/openssl-project

___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project

Re: [openssl-project] Issues review

2018-01-23 Thread Benjamin Kaduk
On Tue, Jan 23, 2018 at 06:11:50PM +, Matt Caswell wrote:
> 
> 
> On 23/01/18 18:05, Benjamin Kaduk wrote:
> > On Tue, Jan 23, 2018 at 05:51:41PM +, Matt Caswell wrote:
> >>
> >>
> >> On 23/01/18 17:49, Matt Caswell wrote:
> >>> I completed my first pass review of all issues. I still need to look at
> >>> PRs. I have put all PRs against a milestone using the following criteria:
> >>
> >> I have put all *issues* against a milestone not PR!!
> > 
> > Do we still need to review the issues assigned to milestones that
> > have already happened (e.g., 1.1.0, post-1.1.0)?
> 
> There no issues against post-1.1.0 (there are PRs - but that will be
> fixed when I do the PR review).
> 
> 1.1.0 and 1.0.2 are still supported so issues against those milestones
> are still relevant. They are *not* relevant to the 1.1.1 release
> timetable though (which is why I started this exercise). Consider an
> issues against the 1.1.0 milestone to mean, relevant to the next 1.1.0
> letter release.

That's great if that's the intent, but I don't think that the
current application of those tags is consistent with the above
description.  For example, #1418 is a somewhat abstract question of
what it means for acertificate to be self-signed, yet has the 1.0.2
milestone, when (to me) 1.2.0 would seem more appropriate.

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] Issues review

2018-01-23 Thread Benjamin Kaduk
On Tue, Jan 23, 2018 at 05:51:41PM +, Matt Caswell wrote:
> 
> 
> On 23/01/18 17:49, Matt Caswell wrote:
> > I completed my first pass review of all issues. I still need to look at
> > PRs. I have put all PRs against a milestone using the following criteria:
> 
> I have put all *issues* against a milestone not PR!!

Do we still need to review the issues assigned to milestones that
have already happened (e.g., 1.1.0, post-1.1.0)?

-Ben
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project