Skip to content

Conversation

rfanner
Copy link

@rfanner rfanner commented Oct 18, 2019

Windows 10 would fail, even when using an administrator CMD prompt,
to do vagrant up with messages such as the following:

default: make[3]: Leaving directory '/fossology/src/cli'
    default: Generating fo_wrapper ...
    default: ln:
    default: failed to create symbolic link 'cp2foss'
    default: : Protocol error

It looks like this can potentially be worked around by enabling
symlink support for VirtualBox, but this opens a security hole.
See https://www.virtualbox.org/ticket/10085#comment:7

The above security issue remains open.

While I'm a complete vagrant novice, I'm hoping changing the
basic folder sync to use rsync instead of symlinks is an acceptible
fix. (Works on Windows 10 host, not tested on the other platforms).

Windows 10 would fail, even when using an administrator CMD prompt,
to do `vagrant up` with messages such as the following:

default: make[3]: Leaving directory '/fossology/src/cli'
    default: Generating fo_wrapper ...
    default: ln:
    default: failed to create symbolic link 'cp2foss'
    default: : Protocol error

It looks like this can potentially be worked around by enabling
symlink support for VirtualBox, but this opens a security hole.
See https://www.virtualbox.org/ticket/10085#comment:7

The above security issue remains open.

While I'm a complete vagrant novice, I'm hoping changing the
basic folder sync to use rsync instead of symlinks is an acceptible
fix. (Works on Windows 10 host, not tested on the other platforms).

	# Please enter the commit message for your changes. Lines starting
config.vm.post_up_message = $post_up_message
config.vm.synced_folder ".", "/vagrant", disabled: true
config.vm.synced_folder ".", "/fossology"
config.vm.synced_folder ".", "/vagrant", type: "rsync", disabled: true
Copy link
Member

Choose a reason for hiding this comment

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

type: "rsync" is not a good default especially for the current way of development development (changes need to be synced manually or by watching the files). This should be at least an OS dependent option.

Copy link
Author

Choose a reason for hiding this comment

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

@maxhbr Thanks for the input.

While not familiar with typical workflows here, I can appreciate that file watching is a bit iffy and that manual copying would be annoying to have to do.

In the linked VirtualBox ticket, they mention that enabling symlink support on shared folders is a security hole across all platforms. It is not specific to Windows.

Bearing this in mind, I'm wondering whether this patch is sensible at all?

If it breaks existing development workflows to the point where it's a major drag, then perhaps there just needs to be a caveat or advisory to users about enabling the relevant toggles in the VirtualBox VM configuration to work around the issue and noting the ins and outs of the security concern.

If security is a concern, then perhaps a patch like this might make sense, but only in conjunction with additional work to better support existing workflows.

Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

I thought that this bug with symlinks is no longer present in recent Vbox versions? What version do you use?

Copy link
Author

@rfanner rfanner Oct 29, 2019

Choose a reason for hiding this comment

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

From comment 8, just below comment 7:
"Symbolic link creation from within a guest has been disabled in VirtualBox 4.1.8 for security reasons. A guest could create symbolic links which point outside the assigned host directory. This has nothing to do with any ext3/ext4 bug. And the guest is still able to read symlinks which are created on the host.

Sorry for the late statement.

If you do

VBoxManage setextradata VM_NAME VBoxInternal2/SharedFoldersEnableSymlinksCreate 1
Then your guest will be able to create symlinks again. But for security reasons (see above) this is disabled by default. The fix to prevent dangerous symlinks from the guest is very complicated, therefore we decided to not allow any guest to create any symlink to work around the security problem."

Happy to give the above flag a spin to see if it works around the issue (while leaving a security niggle), and I'm guessing that nobody on the fossology project has run into this on Linux VBoxes, which implies that parts of the input on the linked VirtualBox bug report aren't accurate (perhaps they fixed security concerns for Linux but left Windows hosts for another day or something). I was using VBox 6.0.12.

@mcjaeger mcjaeger added this to the 3.7.0 milestone Nov 5, 2019
@GMishx GMishx assigned GMishx and unassigned GMishx Nov 5, 2019
@mcjaeger
Copy link
Member

tested in on macosx 10.14:

  • with rsync, the files are not synced neither from host to guest nor from guest to host. I am also a novice with virtualbox, so I do not know if I need to trigger something.
  • without rsync, files created on host and guest are visible at the respective other end.

propose to move it to 3.8

@mcjaeger mcjaeger modified the milestones: 3.7.0, meaning, 3.8.0 Nov 22, 2019
@mcjaeger
Copy link
Member

to summarise:

  • just type rsync has the downside tat file are not autosynced
  • using rsync_auto does not seem to to work on VB 6.1 and VG 2.2, even with plugin vagrant-sshfs installed.
  • best way would be to conditional the Vagrantfile to check for the host OS. If the host OS is win10 it shall go for rsync, otherwise as it is now.

I might have a Win 10 machine to test the conditional part, but it requires changing the PR:

@mcjaeger mcjaeger modified the milestones: 3.8.0, 3.9.0 Apr 20, 2020
@GMishx GMishx modified the milestones: 3.9.0, 3.10.0 Nov 26, 2020
@mcjaeger mcjaeger self-assigned this Mar 23, 2021
@mcjaeger mcjaeger modified the milestones: 3.10.0, 3.11.0 May 10, 2021
@GMishx GMishx removed this from the 3.11.0 milestone Aug 2, 2021
@shaheemazmalmmd
Copy link
Member

Closing this PR, as there are no recent updates. Please reopen it if you are working on it.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants