Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Introducing passwordbasedauthentication based on lcrypt and sha512 (backport #484) #487

Merged
merged 13 commits into from
Apr 30, 2021
Merged

Introducing passwordbasedauthentication based on lcrypt and sha512 (backport #484) #487

merged 13 commits into from
Apr 30, 2021

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Apr 29, 2021

This is an automatic backport of pull request #484 done by Mergify.
Cherry-pick of 1c6aeb5 has failed:

On branch mergify/bp/gvm-libs-21.04/pr-484
Your branch is up to date with 'origin/gvm-libs-21.04'.

You are currently cherry-picking commit 1c6aeb5.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   .ccls
	modified:   CMakeLists.txt
	modified:   util/CMakeLists.txt
	new file:   util/passwordbasedauthentication.c
	new file:   util/passwordbasedauthentication.h
	new file:   util/passwordbasedauthentication_tests.c

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   CHANGELOG.md

To fix up this pull request, you can check it out locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

  • @Mergifyio refresh will re-evaluate the rules
  • @Mergifyio rebase will rebase this PR on its base branch
  • @Mergifyio update will merge the base branch into this PR
  • @Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

  • look at your merge queues
  • generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.io/

nichtsfrei and others added 12 commits April 29, 2021 14:40
@nichtsfrei
Introducing passwordbasedauthentication based on lcrypt and sha512
c8e7c6c 
Introducing passwordbasedauthentication to handle upgrade from MD5 to
sha512 within gvmd.

The decision to write new implementation based on lcrypt and sha512
rather than updating authutils is based on the decision to follow phc
comliance as closely as feasible to allow easier versioning and updating
of hashes for password based logins.

To allow easy handling of correct but dated hashes a new state:
RECOMMEND_UPDATE
is introduced for passwords ontop of valid/invalid.

Therefore a direct dependency of passwordbasedauthentication.c to
authutils.c is chosen to have easier access to the deprecated
functionality.

To create the hash crypt_r is chosen to have a thread safe hashing
possibility, this comes with the costs of portability since not all
crypt libraries do support that. However libcrypt (used by
debian:testing) as well as UFC crypt (used by buster) do.

Due to the limitation of salt to 16 bytes the newly introduced pepper
usage is limited to 4 bytes. This needs to be respected in using
clients, e.g. gvmd and enforced there.

(cherry picked from commit 1c6aeb5)

# Conflicts:
#	CHANGELOG.md
@nichtsfrei
Update license
5852826 
(cherry picked from commit f727722)
@nichtsfrei
switch implementation from aututils.h to passwordbasedauthentication.c
11faabf 
In order to use passwordbasedauthentication.c directly dependable on
authutils.c it should not be installed via CMakeLists.txt for
authutils.h instead it should use passwordbasedauthentication.c

(cherry picked from commit ed611d3)
@nichtsfrei
Add CRYPT_LDFLAGS to gvm_util_shared
5e2c692 
(cherry picked from commit d67a617)
@nichtsfrei
Set password in pba_hash to const
ad6aed6 
Since pba_hash does not change the password it is set to const char

(cherry picked from commit b231fca)
@nichtsfrei
remove .ccls instead of setting macro on build time guess used library
2a312f8 
Remove .ccls since it is developer specific settings and should not be
within github.

Instead of setting a macro if the internal gensalt_r method or the
external should be used passwordbasedauthentication is guessing based on

ifndef crypt_gensalt_r

if crypt_gensalt_r is defined it is using the external salt function
    otherwise it is using it's own implementation.

(cherry picked from commit b1f8eec)
@nichtsfrei
remove implicit check for crypt_genssalt_r to explicit
17af92d 
The control of using the internal crypt_gensalt_r or the external by
the used lcrypt library is now defined and set within
util/CMakeLists.txt as EXTERNAL_CRYPT_GENSALT_R maco based on the
version of the crypt module; when higher than 3.1.1 than use external
otherwise internal implementation.

(cherry picked from commit e59a243)
@nichtsfrei
used calloc instead of malloc for data->initialized = 0
ec6bf34 
(cherry picked from commit c0afaef)
@nichtsfrei
Apply suggestions from code review
866779f 
Co-authored-by: Björn Ricks <bjoern.ricks@gmail.com>
(cherry picked from commit 116c51d)
@nichtsfrei
Verify before memcp if either strlen or CRYPT_OUTPUT_SIZE is smaller
5d53d5a 
Before doing a memcpy verify the length of hash; when shorter then
CRYPT_OUTPUT_SIZE then use strlen otherwise CRYPT_OUTPUT_SIZE

(cherry picked from commit 65cf632)
@nichtsfrei
util/CMakeLists.txt check if CRYPT_M_VERSION is DEFINED
9b76406 
util/CMakeLists.txt check if CRYPT_M_VERSION is DEFINED before
verifiying if the version is greater than 3.1.1 to prevent:

Unknown arguments specified

while building.

(cherry picked from commit 7ea4005)
@nichtsfrei
Replace memcpy in favor of strncpy
0d2b323 
(cherry picked from commit 1115330)
@mergify mergify bot added the conflicts This backport pull request has merge conflicts that need to be resolved label Apr 29, 2021
@bjoernricks bjoernricks enabled auto-merge April 30, 2021 06:01
@bjoernricks bjoernricks merged commit 0a1e41e into gvm-libs-21.04 Apr 30, 2021
@mergify mergify bot deleted the mergify/bp/gvm-libs-21.04/pr-484 branch April 30, 2021 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjoernricks bjoernricks bjoernricks approved these changes

@ArnoStiefvater ArnoStiefvater Awaiting requested review from ArnoStiefvater

@jjnicola jjnicola Awaiting requested review from jjnicola

@timopollmeier timopollmeier Awaiting requested review from timopollmeier

Assignees
No one assigned
Labels
conflicts This backport pull request has merge conflicts that need to be resolved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bjoernricks @nichtsfrei