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.

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

samba: 4.7.10 -> 4.7.12 #55890

Merged
merged 1 commit into from Feb 24, 2019
Merged

samba: 4.7.10 -> 4.7.12 #55890

merged 1 commit into from Feb 24, 2019

Conversation

andrew-d
Copy link
Contributor

@andrew-d andrew-d commented Feb 16, 2019 •
edited

Motivation for this change

Security update; see #55392

EDIT:

I don't know if this is going to be fixed with the most recent force-push, but I'm seeing a bunch of the following errors:

anonymous function at [...]pkgs/applications/networking/remote/x2goclient/default.nix:1:1 called without required argument 'nxproxy', at [...]/lib/customisation.nix:69:12

This doesn't seem at all related to the Samba changes in this PR, so I have no idea what's going on 🤷‍♂️

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

cc? @ckauhaus

@andrew-d andrew-d force-pushed the andrew/samba-4.7.12 branch 3 times, most recently from 2ff4c26 to fb65a2e Compare February 16, 2019 10:21
@andrew-d andrew-d mentioned this pull request Feb 16, 2019
Merged
10 tasks
@srhb
Copy link
Contributor

srhb commented Feb 16, 2019

@GrahamcOfBorg eval

bachp
bachp approved these changes Feb 21, 2019
View reviewed changes
Copy link
Member

@bachp bachp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sucessfully tested with Windows 10 for several days

@xeji
Copy link
Contributor

xeji commented Feb 23, 2019

@GrahamcOfBorg build samba sambaFull

@xeji
Copy link
Contributor

xeji commented Feb 23, 2019

Darwin build should be fixed before we merge this.

@andrew-d
Copy link
Contributor Author

@xeji - Any idea what the error is? Both builds appear to fail just after the patching script interpreter paths message, but I don't see any error messages that say why that is...

@xeji
Copy link
Contributor

xeji commented Feb 24, 2019

It also failed on x64_64-linux this time.
Looking at the full logs, there are several compiler errors both for Darwin and x64_64-linux.
Not sure about the cause.

@vcunat
Copy link
Member

vcunat commented Feb 24, 2019

For the sambaFull problem see: #55893 (comment)

@vcunat
Copy link
Member

vcunat commented Feb 24, 2019

samba itself builds fine for me.

@andrew-d
Copy link
Contributor Author

@vcunat - I just force-pushed a change that fixes the sambaFull build locally

@vcunat
Copy link
Member

vcunat commented Feb 24, 2019

So the plan is to have sambaFull contain the "insecure" combination and hope for admin not to utilize it or something. Well, sambaFull isn't used by default anywhere, so I expect that will be OK, though I can't say I understand stuff around running a samba service.

@vcunat
Copy link
Member

vcunat commented Feb 24, 2019

@GrahamcOfBorg build samba sambaFull

@vcunat vcunat self-assigned this Feb 24, 2019
vcunat
vcunat approved these changes Feb 24, 2019
View reviewed changes
Copy link
Member

@vcunat vcunat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as for 18.09, tested on two platforms, builds of two packages and the nixos test.

@vcunat
Copy link
Member

vcunat commented Feb 24, 2019

Notes:

  • we have no meta.maintainers, even though the package/service seems relatively important (I imagine);
  • release notes for 4.7.11 suggested that the 4.7 branch is about its EOL, so it might be a good time to upgrade, possibly before 19.03 fork-off, though security patches will probably still be available, so I expect it's not too urgent.

@vcunat vcunat merged commit 2acd000 into NixOS:master Feb 24, 2019
vcunat added a commit that referenced this pull request Feb 24, 2019
@vcunat
Merge #55890: samba: 4.7.10 -> 4.7.12 (security)
d4fcb9b
@andrew-d andrew-d deleted the andrew/samba-4.7.12 branch February 25, 2019 07:25
@andrew-d
Copy link
Contributor Author

@vcunat - Thank you for the help here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bachp bachp bachp approved these changes

@vcunat vcunat vcunat approved these changes

Assignees

@vcunat vcunat

Labels
1.severity: security 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@andrew-d @srhb @xeji @vcunat @bachp @GrahamcOfBorg